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

[spatel]

Summary

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 the border 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.

[craig.topper]

Does DAGCombiner have the same bug are will you be patching it too?

[spatel]

In D124692#3483317, @craig.topper wrote:

Does DAGCombiner have the same bug are will you be patching it too?

Sure - that part is in DAGCombiner::FoldIntToFPToInt() and looks similar.

[spatel]

Ping. The SDAG sibling was pushed a few days ago with:
D124771 ( 747c6a0c734e618db )

[efriedma]

LGTM

llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp

Instruction *InstCombinerImpl::foldItoFPtoI(CastInst &FI) {
   // This means this is also safe for a signed input and unsigned output, since
   // a negative input would lead to undefined behavior.
   if (!isKnownExactCastIntToFP(*OpI)) {
     // The first cast may not round exactly based on the source integer width
     // and FP width, but the overflow UB rules can still allow this to fold.
     // 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;
   }
 
   if (DestType->getScalarSizeInBits() > XType->getScalarSizeInBits()) {
     bool IsInputSigned = isa<SIToFPInst>(OpI);
     if (IsInputSigned && IsOutputSigned)
       return new SExtInst(X, DestType);

llvm/test/Transforms/InstCombine/sitofp.ll

 ; CHECK-NEXT:    [[C:%.*]] = fptoui float [[B]] to i32
 ; CHECK-NEXT:    ret i32 [[C]]
 ;
   %B = sitofp i26 %A to float
   %C = fptoui float %B to i32
   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
   %C = fptosi double %B to i54
   ret i54 %C
 }
 
 ; This can't fold because the 55-bit output won't fit the mantissa

 ; CHECK-NEXT:    ret i25 [[C]]
 ;
   %m = and i25 %A, -2
   %B = uitofp i25 %m to float
   %C = fptoui float %B to i25
   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
 ; CHECK-NEXT:    ret i11 [[R]]
 ;
   %h = sitofp i32 %x to half
   %r = fptosi half %h to i11
   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
 ; CHECK-NEXT:    ret i11 [[R]]
 ;
   %h = sitofp i32 %x to half
   %r = fptoui half %h to i11
   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
 ; CHECK-NEXT:    ret i11 [[R]]
 ;
   %h = uitofp i32 %x to half
   %r = fptosi half %h to i11
   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
 ; CHECK-NEXT:    ret i11 [[R]]
 ;
   %h = uitofp i32 %x to half
   %r = fptoui half %h to i11
   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
   %r = fptosi half %h to i12
   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
 ; CHECK-NEXT:    [[R:%.*]] = fptoui half [[H]] to i12
 ; CHECK-NEXT:    ret i12 [[R]]
 ;
   %h = sitofp i32 %x to half
   %r = fptoui half %h to i12
   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
   %r = fptosi half %h to i12
   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
 ; CHECK-NEXT:    [[R:%.*]] = fptoui half [[H]] to i12
 ; CHECK-NEXT:    ret i12 [[R]]
 ;
   %h = uitofp i32 %x to half
   %r = fptoui half %h to i12
   ret i12 %r
 }