Skip to content

Conversation

@steven-johnson
Copy link
Contributor

This is a sketch of a possible way to address Issue #3306: Input<Func> and Output<Func> have a new method, dimensions_and_alignment(), which returns a struct that lets you get/set the min/extent/stride/host-alignment; these values are entirely ignored if the given Input/Output isn't AOT-compiled as a Buffer.

This works fine and I feel it's robust, but I'm not sure if this API is one I'm happy to support forever. That said, it would make it a lot easier to make composable Generators per the details in the issue.

This is a sketch of a possible way to address Issue #3306: `Input<Func>` and `Output<Func>` have a new method, `dimensions_and_alignment()`, which returns a struct that lets you get/set the min/extent/stride/host-alignment; these values are entirely ignored if the given Input/Output isn't AOT-compiled as a Buffer.

This works fine and I feel it's robust, but I'm not sure if this API is one I'm happy to support forever. That said, it would make it a lot easier to make composable Generators per the details in the issue.
@steven-johnson
Copy link
Contributor Author

Attn @inazarenko

@steven-johnson
Copy link
Contributor Author

Any comments on this at all?

@abadams
Copy link
Member

abadams commented Oct 18, 2018

Can we not just support output.dim(2).set_stride(...) and co. on Output<Func> ?

@steven-johnson
Copy link
Contributor Author

Can we not just support output.dim(2).set_stride(...) and co. on Output ?

No, because the Output<Func> might not have any realization if the Generator has been inlined into another Generator.

@abadams
Copy link
Member

abadams commented Oct 18, 2018

Why can't output.dim(1).set_stride mean the same thing as output.dimensions_and_alignment().dim(1).set_stride? I.e. it gets ignored if there's no realization.

@steven-johnson
Copy link
Contributor Author

gets ignored if there's no realization

The thing I'm concerned about is that it's easy to use dim(0).extent(), etc mistakenly, and then get weird or incorrect results that are hard to debug. Let me see if I can reliably ensure that using these inappropriately will produce a hard+clear failure at compile time.

@dsharletg
Copy link
Contributor

Maybe a bit pie in the sky, but I'd love to try to reduce/eliminate the inconsistency between internal and output stages in general, which might implicitly make this problem better.

For example, it seems like we should be able to use bound and reorder_storage to implement interleaved output buffers, the way you would for an internal stage. The fact that we have a different language for describing this for internal and external stages is unfortunate IMO and a source of confusion for myself at least.

Basically, I think it would be awesome to get rid of set_min/extent_stride completely, and use reorder_storage, align_bounds, and bound on output stages instead.

@abadams
Copy link
Member

abadams commented Oct 18, 2018

+1

@steven-johnson
Copy link
Contributor Author

I think it would be awesome to get rid of set_min/extent_stride completely, and use reorder_storage, align_bounds, and bound on output stages instead.

SGTM. (Do we have the ability to fully replicate everything we need already, or do we need to enrich the scheduling language a bit?)

That said: will all of these work on Funcs that may be inlined? (e.g. if I call align_storage on a Func with no storage?)

@inazarenko
Copy link
Contributor

That said: will all of these work on Funcs that may be inlined?

In particular, Funcs wrapped around buffers in the implementation of Input<Func> are always inlined.

@steven-johnson steven-johnson added the skip_buildbots Do not run buildbots on this PR. Must add before opening PR as we scan labels immediately. label May 26, 2020
@alexreinking alexreinking added this to the Eventually milestone Oct 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip_buildbots Do not run buildbots on this PR. Must add before opening PR as we scan labels immediately.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants