[PATCH] D143720: [InstCombine] extend "simplifyUsingControlFlow" supporting zext/sext/trunc for different sizes

[Kohei Asano via Phabricator via llvm-commits]

khei4 added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp:1307
   Value *Cond;
-  SmallDenseMap<ConstantInt *, BasicBlock *, 8> SuccForValue;
+  SmallDenseMap<ConstantInt *, BasicBlock *, 8> SuccValue, SuccValueTrunc,
+      SuccValueZExt, SuccValueSExt;
----------------
inclyc wrote:
> khei4 wrote:
> > IMHO, duplicating `SuccForValue` map for each casts seems a little redundant. I like collecting incoming phi values in the front and checking correspondences to unify those maps. Then we can also early return if the values don't correspond.
> Thanks for the feedback! 
> 
> >  I like collecting incoming phi values in the front and checking correspondences to unify those maps.
> 
> Can you elaborate your solution? Here are my worries:
> 
> Here we are going to find a operation that maps branch condition in to corresponding phi values. Not an operation that maps phi values back to branch condition.
> 
> I think I have to left "sext"/"zext"ed values here because we must know the result of the branch condition after "s/zext".  That's different from truncating from PHI values. i.e. s/zext information is not recoverable. Truncating phi values to branch condition does not imply we can z/sext the branch condition back to "phi values".
Sorry for my vague comments. The idea was unifying later inverting checking and these casting checking. Finding SExt or ZExt is similar to decide an inversion 

> I think I have to left "sext"/"zext"ed values here because we must know the result of the branch condition after "s/zext".

I just thought it'll be neat to reverse the roll between Succ/Cond and Pred/Phi. Then we don't need to hold candidate values. But I feel this might be too detailed and optional, this requires the dominant condition reversed. (I just tried to write actually, but ugly wip https://github.com/llvm/llvm-project/compare/main...khei4:llvm-project:wip/khei4-sext


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143720