foad added inline comments.

================
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);
----------------
foad wrote:
> 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.
@efriedma do you have any objection to the patch as-is?


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

Reply via email to