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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
