[PATCH] D49229: [InstCombine] Fold redundant masking operations of shifted value
Alexandros Lamprineas via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 23 07:10:39 PDT 2018
labrinea added inline comments.
================
Comment at: lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2024
return replaceInstUsesWith(I, V);
+ { // Fold redundant masking.
+ const APInt *AndC, *ShAndC, *ShftAmt;
----------------
You could add a more descriptive comment like:
```// Fold redundant masking.
// Let C3 = C1 lsh C2
// (X & C1) | ((X lsh C2) & C3) -> (X & C1) | ((X & C1) lsh C2)
// Also handles the commutative cases.
```
================
Comment at: lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2027
+ Value *X1, *X2;
+ if (match(&I, m_Or(m_c_And(m_Value(X1), m_APInt(AndC)),
+ m_c_And(m_OneUse(m_Shift(m_Value(X2), m_APInt(ShftAmt))),
----------------
You could change that into `m_c_Or` to cover the commutative case too. Then you can get rid of the code duplication.
================
Comment at: lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2031
+ (X1 == X2)) {
+ BinaryOperator *And = cast<BinaryOperator>(I.getOperand(0));
+ BinaryOperator *ShtAnd = cast<BinaryOperator>(I.getOperand(1));
----------------
You could replace the lines 2031-2035 with
```
BinaryOperator *Op0 = cast<BinaryOperator>(I.getOperand(0));
BinaryOperator *Op1 = cast<BinaryOperator>(I.getOperand(1));
BinaryOperator *And, *Shift;
if ((Shift = dyn_cast<BinaryOperator>(Op0->getOperand(0))))
And = Op1;
else if ((Shift = dyn_cast<BinaryOperator>(Op0->getOperand(1))))
And = Op1;
else if ((Shift = dyn_cast<BinaryOperator>(Op1->getOperand(0))))
And = Op0;
else
And = Op0;
```
and then remove the commutative pattern match from below (lines 2049-2070).
================
Comment at: test/Transforms/InstCombine/FoldRedundantShiftedMasks.ll:4
+
+; https://reviews.llvm.org/D48278
+; Fold redundant masking operations of shifted value
----------------
The reference to Phabricator is confusing. Please remove it. It'll become irrelevant with time anyway.
https://reviews.llvm.org/D49229
More information about the llvm-commits
mailing list