Hoist the shared dense load as a let in stage_strided_loads#8964
Hoist the shared dense load as a let in stage_strided_loads#8964
Conversation
This will make it possible in future to transpose it as one thing.
alexreinking
left a comment
alexreinking
left a comment
There was a problem hiding this comment.
Looks good with a couple clarifying questions and one comment-wording nit.
| Func f; | ||
| Var x; | ||
| f(x) = buf(16 * x) + buf(16 * x + 15); | ||
| f(x) = buf(17 * x) + buf(17 * x + 15); |
There was a problem hiding this comment.
I made it so that hoisting a dense load now occurs in the case where the stride equals the vector length, so that square transposes will be handled in the next PR.
src/StageStridedLoads.cpp
Outdated
| replacer.replacements.emplace(std::make_pair(alloc, l), shuf); | ||
|
|
||
| // We can't lift the shared load further out than the scope over | ||
| // which the loads definition occur. If k.scope is null, the loads |
There was a problem hiding this comment.
Grammar here is awkward. "over which the load's definition occurs"? "the load is valid everywhere"?
| return result; | ||
| } | ||
|
|
||
| bool can_hoist_shared_load(const IRNode *n, const std::string &buf, const Expr &idx) { |
There was a problem hiding this comment.
Is there any risk of hoisting past an extern stage with a side-effect?
There was a problem hiding this comment.
I think there might be. I'll try to construct a failure
src/StageStridedLoads.cpp
Outdated
| // within this stmt, and there are no stores to the given buffer in the | ||
| // stmt. |
There was a problem hiding this comment.
Is there a test that covers the "don't hoist past a store" mistake?
|
Failure is #8928 |
Also address other review comments
2 participants
Previously each strided load did the same dense load redundantly, and we relied on LLVM to dedup. Pulling it out as a let makes it possible in #8925 future to transpose it as one thing.