[PATCH] D125557: [APInt] Remove all uses of zextOrSelf, sextOrSelf and truncOrSelf
Jay Foad via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Sat May 14 10:06:04 PDT 2022
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
More information about the cfe-commits
mailing list