[llvm] r219542 - Return undef on FP <-> Int conversions that overflow (PR21330).
Quentin Colombet
qcolombet at apple.com
Fri Oct 17 17:17:06 PDT 2014
Hi Sanjay,
I do not think this change is correct.
I have attached a test case with (before.ll) and without (after.ll) that change and the related C file (test.c).
To reproduce, compile with O1 or more and run the produced executable. The correct behavior is do not assert :).
Please revert or fix it.
> On Oct 10, 2014, at 4:00 PM, Sanjay Patel <spatel at rotateright.com> wrote:
>
> Author: spatel
> Date: Fri Oct 10 18:00:21 2014
> New Revision: 219542
>
> URL: http://llvm.org/viewvc/llvm-project?rev=219542&view=rev
> Log:
> Return undef on FP <-> Int conversions that overflow (PR21330).
>
> The LLVM Lang Ref states for signed/unsigned int to float conversions:
> "If the value cannot fit in the floating point value, the results are undefined."
>
> And for FP to signed/unsigned int:
> "If the value cannot fit in ty2, the results are undefined."
>
> This matches the C definitions.
Yes, this matches the C definitions, but the rules of propagating the undef value are not strictly the same in C and LLVM IR AFAICT.
Here is an example:
In (pseudo) C,
a = undef // a is undef;
if (something)
b = val;
else
b = a; // b is undef, but it should be the same value as a.
assert (something || b == a);
The related (pseudo) IR would be:
a = undef
b = select i1 something, val, a
cmpVal = cmp eq b, a
assertVal = or something, cmpVal
call assert(assertVal)
Now, the actual IR will be, because of constant propagation:
b = select i1 something, val, undef // <— we lose the information that the next two undef are the same value.
cmpVal = cmp eq b, undef
assertVal = or something, cmpVal
call assert(assertVal)
This is still fine as long as undef are expanded into the same constant.
However, the LangRef states that the following transformation is valid:
%C = select %X, %Y, undef
Safe:
%C = %Y
Unsafe:
%C = undef
Therefore, we end up with the following IR:
b = val // <— now, the value of b may not be a when ‘something' is false, which is incorrect.
cmpVal = cmp eq b, undef
assertVal = or something, cmpVal
call assert(assertVal)
Thanks,
-Quentin
>
> The existing behavior pins to infinity or a max int value, but that may just
> lead to more confusion as seen in:
> http://llvm.org/bugs/show_bug.cgi?id=21130
>
> Returning undef will hopefully lead to a less silent failure.
>
> Differential Revision: http://reviews.llvm.org/D5603
>
>
> Modified:
> llvm/trunk/lib/IR/ConstantFold.cpp
> llvm/trunk/test/Transforms/InstCombine/cast.ll
>
> Modified: llvm/trunk/lib/IR/ConstantFold.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/ConstantFold.cpp?rev=219542&r1=219541&r2=219542&view=diff
> ==============================================================================
> --- llvm/trunk/lib/IR/ConstantFold.cpp (original)
> +++ llvm/trunk/lib/IR/ConstantFold.cpp Fri Oct 10 18:00:21 2014
> @@ -593,8 +593,13 @@ Constant *llvm::ConstantFoldCastInstruct
> bool ignored;
> uint64_t x[2];
> uint32_t DestBitWidth = cast<IntegerType>(DestTy)->getBitWidth();
> - (void) V.convertToInteger(x, DestBitWidth, opc==Instruction::FPToSI,
> - APFloat::rmTowardZero, &ignored);
> + if (APFloat::opInvalidOp ==
> + V.convertToInteger(x, DestBitWidth, opc==Instruction::FPToSI,
> + APFloat::rmTowardZero, &ignored)) {
> + // Undefined behavior invoked - the destination type can't represent
> + // the input constant.
> + return UndefValue::get(DestTy);
> + }
> APInt Val(DestBitWidth, x);
> return ConstantInt::get(FPC->getContext(), Val);
> }
> @@ -653,9 +658,13 @@ Constant *llvm::ConstantFoldCastInstruct
> APInt api = CI->getValue();
> APFloat apf(DestTy->getFltSemantics(),
> APInt::getNullValue(DestTy->getPrimitiveSizeInBits()));
> - (void)apf.convertFromAPInt(api,
> - opc==Instruction::SIToFP,
> - APFloat::rmNearestTiesToEven);
> + if (APFloat::opOverflow &
> + apf.convertFromAPInt(api, opc==Instruction::SIToFP,
> + APFloat::rmNearestTiesToEven)) {
> + // Undefined behavior invoked - the destination type can't represent
> + // the input constant.
> + return UndefValue::get(DestTy);
> + }
> return ConstantFP::get(V->getContext(), apf);
> }
> return nullptr;
>
> Modified: llvm/trunk/test/Transforms/InstCombine/cast.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/cast.ll?rev=219542&r1=219541&r2=219542&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/InstCombine/cast.ll (original)
> +++ llvm/trunk/test/Transforms/InstCombine/cast.ll Fri Oct 10 18:00:21 2014
> @@ -1050,3 +1050,37 @@ define i8 @test85(i32 %a) {
> ; CHECK: [[SHR:%.*]] = lshr exact i32 [[ADD]], 23
> ; CHECK: [[CST:%.*]] = trunc i32 [[SHR]] to i8
> }
> +
> +; Overflow on a float to int or int to float conversion is undefined (PR21130).
> +
> +define i8 @overflow_fptosi() {
> + %i = fptosi double 1.56e+02 to i8
> + ret i8 %i
> +; CHECK-LABEL: @overflow_fptosi(
> +; CHECK-NEXT: ret i8 undef
> +}
> +
> +define i8 @overflow_fptoui() {
> + %i = fptoui double 2.56e+02 to i8
> + ret i8 %i
> +; CHECK-LABEL: @overflow_fptoui(
> +; CHECK-NEXT: ret i8 undef
> +}
> +
> +; The maximum float is approximately 2 ** 128 which is 3.4E38.
> +; The constant below is 4E38. Use a 130 bit integer to hold that
> +; number; 129-bits for the value + 1 bit for the sign.
> +define float @overflow_uitofp() {
> + %i = uitofp i130 400000000000000000000000000000000000000 to float
> + ret float %i
> +; CHECK-LABEL: @overflow_uitofp(
> +; CHECK-NEXT: ret float undef
> +}
> +
> +define float @overflow_sitofp() {
> + %i = sitofp i130 400000000000000000000000000000000000000 to float
> + ret float %i
> +; CHECK-LABEL: @overflow_sitofp(
> +; CHECK-NEXT: ret float undef
> +}
> +
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141017/8ddf8cc0/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: after.ll
Type: application/octet-stream
Size: 2340 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141017/8ddf8cc0/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141017/8ddf8cc0/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: before.ll
Type: application/octet-stream
Size: 2417 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141017/8ddf8cc0/attachment-0001.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141017/8ddf8cc0/attachment-0002.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: test.c
Type: application/octet-stream
Size: 436 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141017/8ddf8cc0/attachment-0002.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141017/8ddf8cc0/attachment-0003.html>
More information about the llvm-commits
mailing list