This expands the loop idiom recognition pass to recognize the unordered
atomic memset pattern. This pattern is the same pattern as a regular memset
except that the stores are unordered atomic, and limited to register size.
Details
- Reviewers
anna skatkov apilipenko
Diff Detail
- Repository
- rL LLVM
- Build Status
Buildable 18765 Build 18765: arc lint + arc unit
Event Timeline
lib/IR/IRBuilder.cpp | ||
---|---|---|
106 ↗ | (On Diff #124777) | Given there's some constraints for unordered memset, does it make sense to add the constraints as asserts here (ptrAlign relation to ElementSize)? Any other constraints? |
lib/Transforms/Scalar/LoopIdiomRecognize.cpp | ||
944 | Stylistic comment: Could you switch around the if else so that smaller body is first? | |
test/Transforms/LoopIdiom/unordered-atomic-memset-noarch.ll | ||
6 | where's this max element size of 0? |
lib/IR/IRBuilder.cpp | ||
---|---|---|
106 ↗ | (On Diff #124777) | Easy enough. Good point. |
test/Transforms/LoopIdiom/unordered-atomic-memset-noarch.ll | ||
6 | I should clarify this better in the test comment... This is a noarch test, so a call to TTI->getAtomicMemIntrinsicMaxElementSize() will return 0, thus preventing the transform. |
lib/Transforms/Scalar/LoopIdiomRecognize.cpp | ||
---|---|---|
924 | Why do we not check that the "ordinary memset" was substituting a store? This is the behaviour before your change as well (we dont check for store), but somehow it struck me as weird. |
lib/Transforms/Scalar/LoopIdiomRecognize.cpp | ||
---|---|---|
924 | processLoopStridedStore() kind of has two modes of operation.
I made the atomic case explicitly check for a store instruction here because the "loop of atomic memsets" hasn't been implemented yet. |
LGTM
lib/Transforms/Scalar/LoopIdiomRecognize.cpp | ||
---|---|---|
924 | makes sense. thanks for the clarification. |
Why do we not check that the "ordinary memset" was substituting a store?
In other words, shouldn't there be a check for SI
This is the behaviour before your change as well (we dont check for store), but somehow it struck me as weird.