[PATCH] D74484: [AggressiveInstCombine] Add support for ICmp instr that feeds a select intsr's condition operand.

[Ayman Musa via Phabricator via llvm-commits]

aymanmus marked an inline comment as done.
aymanmus added inline comments.


================
Comment at: llvm/lib/Transforms/AggressiveInstCombine/TruncInstCombine.cpp:356
+      Type *SrcTy = cast<CastInst>(I)->getSrcTy();
+      return Ty->getIntegerBitWidth() >= SrcTy->getIntegerBitWidth();
+    }
----------------
aymanmus wrote:
> nikic wrote:
> > aymanmus wrote:
> > > nikic wrote:
> > > > Doesn't the sext/zext case have to check the IsSigned flag as well? I don't think it's as simple as IsSigned => sext and !IsSigned => zext (e.g. it's fine if you have an equality comparison and both use same extension type), but same correctness checks are needed here.
> > > The IsSigned is not relevant in this part of the code.
> > > IsSigned only indicates whether the use of the constant (in case the given Value is a constant) was in a signed or unsigned context. If the Value is not a constant, we don't care about IsSigned.
> > > I'll change the name of the operand to avoid such confusion.
> > Not sure I get it. Converting `(zext i16 %x to i32) ult (sext i16 %y to i32)` to `%x ult %y` is not legal (https://rise4fun.com/Alive/wVZy), and I don't think anything prevents that from happening with your current code.
> No I understand.
> You're right.
> I gave it a thought and came up with a bit different approach to deal with all the limitations.
> I hope this new function (CmpCanBeShrinked) handles all the needed cases.
Now I understand***


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

https://reviews.llvm.org/D74484