[PATCH] D125557: [APInt] Remove all uses of zextOrSelf, sextOrSelf and truncOrSelf

[Jay Foad via Phabricator via llvm-commits]

foad marked 2 inline comments as done.
foad added inline comments.


================
Comment at: llvm/lib/Analysis/ConstantFolding.cpp:2884
     if (IntrinsicID == Intrinsic::smul_fix_sat) {
-      APInt Max = APInt::getSignedMaxValue(Width).sextOrSelf(ExtendedWidth);
-      APInt Min = APInt::getSignedMinValue(Width).sextOrSelf(ExtendedWidth);
+      APInt Max = APInt::getSignedMaxValue(Width).sext(ExtendedWidth);
+      APInt Min = APInt::getSignedMinValue(Width).sext(ExtendedWidth);
----------------
lattner wrote:
> I think this can be a zext given the top bit will be zero
Sure the first one could be zext, but the second one can't be, so it feels conceptually simpler (to me) to keep them both as sext.


================
Comment at: llvm/lib/IR/ConstantRange.cpp:724
     auto BW = getBitWidth();
-    APInt Min = APInt::getMinValue(BW).zextOrSelf(ResultBitWidth);
-    APInt Max = APInt::getMaxValue(BW).zextOrSelf(ResultBitWidth);
+    APInt Min = APInt::getMinValue(BW);
+    APInt Max = APInt::getMaxValue(BW);
----------------
efriedma wrote:
> efriedma wrote:
> > Making the bitwidth of the result here not equal to ResultBitWidth seems suspect.
> > 
> > I think there should just be an `if (ResultBitWidth < BW) return getFull(ResultBitWidth);` here.  Then a simple conversion just works.
> Actually, looking at D27294 again, maybe it is actually making the result bitwidth intentionally inflate like this.
> 
> This could use a comment explaining what it's doing, in any case.
I agree it could use a comment but I don't feel qualified to write it - I am just trying to preserve the current behaviour.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125557