[PATCH] D79369: [InstCombine] "X - (X / C) * C == 0" to "X & C-1 == 0"
Roman Lebedev via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue May 5 14:03:51 PDT 2020
lebedev.ri added inline comments.
================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp:1309-1310
+ // ((X / C1) << C2) + X => X % C1
+ // where C1 = 1 << C2
+ const APInt *C1, *C2;
----------------
```
// ((X / C1) << C2) + X => X % -C1
// where -C1 = 1 << C2
```
================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp:1311-1312
+ // where C1 = 1 << C2
+ const APInt *C1, *C2;
+ if (match(LHS, m_Shl(m_SDiv(m_Value(A), m_APInt(C1)), m_APInt(C2)))) {
+ if ((RHS == A) && C1->abs().isPowerOf2() && C1->isNegative()) {
----------------
Do we care about non-splat vector support?
This will only work for scalars/splat vectors.
================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp:1317
+ Constant *NewRHS = ConstantInt::get(RHS->getType(), C1->abs());
+ return replaceInstUsesWith(I, Builder.CreateSRem(RHS, NewRHS, "srem"));
+ }
----------------
Just return the new `srem`.
================
Comment at: llvm/test/Transforms/InstCombine/icmp-div-constant.ll:41
+define i1 @is_rem32_pos_decomposed_i8(i8 %x) {
+; CHECK-LABEL: @is_rem32_pos_decomposed_i8(
----------------
Please pull into a new file. Some comments:
* there should be no `icmp` here, this isn't the pattern we are folding (although yes, that is the larger pattern)
* there shouldn't be `nsw` flags since we don't use them
* there shouldn't be `mul`, we are looking for `shl`
* there shouldn't be `sub`, we are looking for `add`
And this needs more tests, in particular:
* splat vector test
* non-splat vector test
* vector splat with undef lane test: there's two constants here, so two tests where either one of them contains `undef` lane, and a test where both of them contain undef lane
* one test with extra uses (see `@use` in this file) on every of the values - we don't care whether there are extra uses
* Negative tests - what are preconditions on the constants?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79369/new/
https://reviews.llvm.org/D79369
More information about the llvm-commits
mailing list