[PATCH] D49229: [InstCombine] Fold redundant masking operations of shifted value

[Sam Parker via Phabricator via llvm-commits]

samparker added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2024
     return replaceInstUsesWith(I, V);
+  { // Fold redundant masking.
+    const APInt *AndC, *ShAndC, *ShftAmt;
----------------
labrinea wrote:
> 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.
> ```
Looks like the convention here is to perform more specific folding after the calls to SimplifyAssociativeOrCommutative, etc.. And there is already a case for '(A & C)|(B & D)'.


================
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))),
----------------
labrinea wrote:
> You could change that into `m_c_Or` to cover the commutative case too. Then you can get rid of the code duplication.
We already know I is an Or.


https://reviews.llvm.org/D49229