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

[Nikita Popov via Phabricator via llvm-commits]

nikic 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:
> > 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.


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

https://reviews.llvm.org/D74484