Exp/rnn #1
Open
Exp/rnn #1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This PR temporarily introduces a Halide hack to make progress. After a quick exchange with @abadams this seems unnecessary as the use case for sequential dependencies probably only requires an RDom instead of a Var for the `t` for-loop index. Punting for now.
This commit adds a `for t in 0:T { ... }` syntax to the TC language.
Because it introduces an extension to the grammar and type changes,
modifications need to propagate all the way to `tc2halide`.
Support for loops is only implemented in the parser and semantic checker.
Emitting proper HalideIR and ScheduleTree will be implemented in follow-up
commits.
The commit should be reviewed in this order:
1. add support to the lexer
2. add the `For` compound type in `tree_views.h`
3. support `parseStmt` in the parser (in particular see how we
`parseRangeConstraint` first and reuse its index)
4. perform semantic checks that guarantee only a single nested
`For` looFor` is allowed (in parsee how we `checkRangeConstraint`
after checking all comprehensions)
5. update `tc2halide`
6. add new tests
This commit would crash if trying to emit HalideIR with `for` loops
but in order to compile we still needs to update `tc2halide`.
Disclaimer: this is WIP and does not work yet. In particular the ScheduleTree generated is incorrect (not yet properly scoped under for loop). Additionally, the example `For4InvalidHalide` seems to hit deeper structural issues of the high-level HalideIR that traditional polyhedral dependence analysis has no issue with (cc @abadams). This commit tests that the for-loop language construct properly propagates to the Halide bounds inference. This commit also explicitly adds checks that only a single RangeConstraint s`traiindex in min:max`) is used for each index. This requirement is added because it would be very surprising to mix explicit ranges coming from for loops with where clauses in a comprehension. In particular, the current behavior in Halide inference is to only keep the last RangeConstraint which systematically discards the information specified in the loop. `tc2halide` even says: ```// TODO: What if subsequent updates have incompatible bounds // (e.g. an in-place stencil)?. The .bound directive will use the // bounds of the last stage for all stages. ``` As a consequence we add an explicit check that multiple bounds specifications may not coexist.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.