[llvm] 8650f05 - [InstCombine] fix miscompile when casting int->FP->int

[Sanjay Patel via llvm-commits]

Author: Sanjay Patel
Date: 2022-05-07T08:46:25-04:00
New Revision: 8650f05c97624b76fed3230bf0ea26a6106bcc3d

URL: https://github.com/llvm/llvm-project/commit/8650f05c97624b76fed3230bf0ea26a6106bcc3d
DIFF: https://github.com/llvm/llvm-project/commit/8650f05c97624b76fed3230bf0ea26a6106bcc3d.diff

LOG: [InstCombine] fix miscompile when casting int->FP->int

As shown in https://github.com/llvm/llvm-project/issues/55150 -
the existing fold may be wrong when converting to a signed value.
This is a quick fix to avoid the miscompile.

I added tests/comments for all of the signed/unsigned combinations
at either side of the boundary width, and tried to confirm with Alive2:
https://alive2.llvm.org/ce/z/3p9DSu

There are already some TODO items in the test file that suggest
possible refinements, so the regression with ui->FP->si is probably ok.
It seems unlikely that we'd see these kind of edge cases with
non-byte-width integer types in real code. The potential miscompile
went undetected for several years.

This and 747c6a0c734e fixes #55150.

Differential Revision: https://reviews.llvm.org/D124692

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
    llvm/test/Transforms/InstCombine/sitofp.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
index 4fbbdd5d6dbc..dc3f32dc90cf 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
@@ -1977,7 +1977,7 @@ Instruction *InstCombinerImpl::foldItoFPtoI(CastInst &FI) {
     // If the destination type is narrow, that means the intermediate FP value
     // must be large enough to hold the source value exactly.
     // For example, (uint8_t)((float)(uint32_t 16777217) is undefined behavior.
-    int OutputSize = (int)DestType->getScalarSizeInBits() - IsOutputSigned;
+    int OutputSize = (int)DestType->getScalarSizeInBits();
     if (OutputSize > OpI->getType()->getFPMantissaWidth())
       return nullptr;
   }

diff  --git a/llvm/test/Transforms/InstCombine/sitofp.ll b/llvm/test/Transforms/InstCombine/sitofp.ll
index 59a80c3a4de9..2427979260f9 100644
--- a/llvm/test/Transforms/InstCombine/sitofp.ll
+++ b/llvm/test/Transforms/InstCombine/sitofp.ll
@@ -190,11 +190,13 @@ define i32 @test17(i26 %A) {
   ret i32 %C
 }
 
-; This can fold because the 54-bit output is signed and we discard the sign bit.
+; This can't fold because the 54-bit output is big enough to hold an input
+; that was rounded when converted to double.
 
 define i54 @test18(i64 %A) {
 ; CHECK-LABEL: @test18(
-; CHECK-NEXT:    [[C:%.*]] = trunc i64 [[A:%.*]] to i54
+; CHECK-NEXT:    [[B:%.*]] = sitofp i64 [[A:%.*]] to double
+; CHECK-NEXT:    [[C:%.*]] = fptosi double [[B]] to i54
 ; CHECK-NEXT:    ret i54 [[C]]
 ;
   %B = sitofp i64 %A to double
@@ -246,6 +248,8 @@ define i25 @low_masked_input(i25 %A) {
   ret i25 %C
 }
 
+; Output is small enough to ensure exact cast (overflow produces poison).
+
 define i11 @s32_half_s11(i32 %x) {
 ; CHECK-LABEL: @s32_half_s11(
 ; CHECK-NEXT:    [[R:%.*]] = trunc i32 [[X:%.*]] to i11
@@ -256,6 +260,8 @@ define i11 @s32_half_s11(i32 %x) {
   ret i11 %r
 }
 
+; Output is small enough to ensure exact cast (overflow produces poison).
+
 define i11 @s32_half_u11(i32 %x) {
 ; CHECK-LABEL: @s32_half_u11(
 ; CHECK-NEXT:    [[R:%.*]] = trunc i32 [[X:%.*]] to i11
@@ -266,6 +272,8 @@ define i11 @s32_half_u11(i32 %x) {
   ret i11 %r
 }
 
+; Output is small enough to ensure exact cast (overflow produces poison).
+
 define i11 @u32_half_s11(i32 %x) {
 ; CHECK-LABEL: @u32_half_s11(
 ; CHECK-NEXT:    [[R:%.*]] = trunc i32 [[X:%.*]] to i11
@@ -276,6 +284,8 @@ define i11 @u32_half_s11(i32 %x) {
   ret i11 %r
 }
 
+; Output is small enough to ensure exact cast (overflow produces poison).
+
 define i11 @u32_half_u11(i32 %x) {
 ; CHECK-LABEL: @u32_half_u11(
 ; CHECK-NEXT:    [[R:%.*]] = trunc i32 [[X:%.*]] to i11
@@ -286,9 +296,12 @@ define i11 @u32_half_u11(i32 %x) {
   ret i11 %r
 }
 
+; Too many bits in output to ensure exact cast.
+
 define i12 @s32_half_s12(i32 %x) {
 ; CHECK-LABEL: @s32_half_s12(
-; CHECK-NEXT:    [[R:%.*]] = trunc i32 [[X:%.*]] to i12
+; CHECK-NEXT:    [[H:%.*]] = sitofp i32 [[X:%.*]] to half
+; CHECK-NEXT:    [[R:%.*]] = fptosi half [[H]] to i12
 ; CHECK-NEXT:    ret i12 [[R]]
 ;
   %h = sitofp i32 %x to half
@@ -296,6 +309,8 @@ define i12 @s32_half_s12(i32 %x) {
   ret i12 %r
 }
 
+; Too many bits in output to ensure exact cast.
+
 define i12 @s32_half_u12(i32 %x) {
 ; CHECK-LABEL: @s32_half_u12(
 ; CHECK-NEXT:    [[H:%.*]] = sitofp i32 [[X:%.*]] to half
@@ -307,9 +322,12 @@ define i12 @s32_half_u12(i32 %x) {
   ret i12 %r
 }
 
+; TODO: This is safe to convert to trunc.
+
 define i12 @u32_half_s12(i32 %x) {
 ; CHECK-LABEL: @u32_half_s12(
-; CHECK-NEXT:    [[R:%.*]] = trunc i32 [[X:%.*]] to i12
+; CHECK-NEXT:    [[H:%.*]] = uitofp i32 [[X:%.*]] to half
+; CHECK-NEXT:    [[R:%.*]] = fptosi half [[H]] to i12
 ; CHECK-NEXT:    ret i12 [[R]]
 ;
   %h = uitofp i32 %x to half
@@ -317,6 +335,8 @@ define i12 @u32_half_s12(i32 %x) {
   ret i12 %r
 }
 
+; Too many bits in output to ensure exact cast.
+
 define i12 @u32_half_u12(i32 %x) {
 ; CHECK-LABEL: @u32_half_u12(
 ; CHECK-NEXT:    [[H:%.*]] = uitofp i32 [[X:%.*]] to half