Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
LGTM, with a couple of nits.
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp | ||
---|---|---|
9572–9575 | ||
9575 | This patch is fine - effectively considering Replicate recipe invariants as "uniform addresses" in addition to current isLiveIns (aka !hasDefiningRecipe). Above comment (independent of this patch) refers to vputils which introduced "onlyFirstLaneUsed" to handle the "AfterVectorization" part, and its isUniformAfterVectorization() should be renamed isUniform(). In fact, it currently checks isInvariant(), i.e., same value across all loop iterations. This could be improved to "uniform" - same value across lanes (per part) as in D148841, and possibly also to same value across lanes*parts (per vectorized iteration) - but the latter refers to UF. |
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp | ||
---|---|---|
9575 | Also, regarding isUniform - in addition to recipe-less LiveIns and uniform Replicate recipes, uniform values may also be provided by non-Replicate recipes placed in vec/pre-headers and by WidenGEP recipe (though broadcasted into a vector). |
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp | ||
---|---|---|
9572–9575 | Fixed, thanks! | |
9575 |
Let me do the renaming as follow-up. I think we shouldn't need any additional work to benefit from D148841 here; uniformity should be propagated to replicate recipes?
This will be addressed by fixing the TODO in isDefinedOutsideVectorRegions, will do as follow-up once I have a suitable test case. |
This is a bit fragile considering that isUniformAfterVectorization() in general also represents lane-varying values whose first lane need only be used, as in vector stores - see isUniformDecision .