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

[chenglin.bi via Phabricator via llvm-commits]

bcl5980 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,
----------------
spatel wrote:
> bcl5980 wrote:
> > spatel wrote:
> > > 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".
> > > 
> > > 
> > So which way are you prefer now? Not uniform code without freeze instruction or use freeze asap to make code easier?
> If there are no known regressions, let's go straight to the code with freeze. That should be the ideal form for most of these patterns.
I try a lot of ways to add freeze into the code today. But it looks it will make the code more complicate. 
Current code already fix all cases without freeze. What freeze can get benefit is convert logical-and to normal and, logical-or to normal or.
So the question becomes which pattern is better `freeze + and + or`,  `select + select`? Only if the first pattern is better we need to add freeze I think.
I'm sorry I haven't find a elegant way to do it. Can you give me some suggestions for that?


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

https://reviews.llvm.org/D139630