[PATCH] D139630: [InstCombine] Fold logic-and/logic-or by distributive laws part2

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp:2801
       bool FalseLogicAnd = isa<SelectInst>(FalseVal);
-      if (CondLogicAnd && FalseLogicAnd) {
-        // (A ? B : 0) ? 1 : (A ? D : 0) --> A ? (B ? 1 : D) : 0
+      auto AndFactorization = [&](Value *Common, Value *InnerCond,
+                                  Value *InnerVal,
----------------
Can we add some description of the different possibilities? 

I know that handling all of the patterns poison-safely is not uniform, but what if we use freeze to make it safe with regular logic ops instead:
https://alive2.llvm.org/ce/z/xb_moG

There is an existing code example for a similar transform below this block of code - search for "MayNeedFreeze".




================
Comment at: llvm/test/Transforms/InstCombine/select-factorize.ll:206
 
+; negative test: and is not last select condition
+
----------------
I am struggling to see all of the possible patterns, but this should fold:
https://alive2.llvm.org/ce/z/yXjD82

Does handling that case make the code for this patch simpler? 
It might be better to split up the changes either way, just so we can be sure that we are not missing anything.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139630/new/

https://reviews.llvm.org/D139630