[InstCombine] likely incorrect optimization of sitofp/fptosi roundtrip

[regehr]

here's a test case that lives in llvm/test/Transforms/InstCombine/sitofp.ll:

; This can fold because the 54-bit output is signed and we discard the sign bit.

define i54 @test18(i64 %A) {
; CHECK-LABEL: @test18(
; CHECK-NEXT:    [[C:%.*]] = trunc i64 [[A:%.*]] to i54
; CHECK-NEXT:    ret i54 [[C]]
;
  %B = sitofp i64 %A to double
  %C = fptosi double %B to i54
  ret i54 %C
}

it is also performed by the x64 and arm64 backends (https://godbolt.org/z/fT15zMncK). since it is capable of changing the value that is returned, we believe it to be incorrect in the absence of fast math flags.

the alive2 compiler explorer instance times out on it, but this is the output:

regehr@john-home:~$ ~/alive2/build/alive-tv --smt-to=0 foo.ll

----------------------------------------
define i54 @test18(i64 %A) {
%0:
  %B = sitofp i64 %A to double, exceptions=ignore
  %C = fptosi double %B to i54, exceptions=ignore
  ret i54 %C
}
=>
define i54 @test18(i64 %A) {
%0:
  %C = trunc i64 %A to i54
  ret i54 %C
}
Transformation doesn't verify!

ERROR: Value mismatch

Example:
i64 %A = #xffdfffffffffffff (18437736874454810623, -9007199254740993)

Source:
double %B = #xc340000000000000 (-9007199254740992)
i54 %C = #x20000000000000 (9007199254740992, -9007199254740992)

Target:
i54 %C = #x1fffffffffffff (9007199254740991)
Source value: #x20000000000000 (9007199254740992, -9007199254740992)
Target value: #x1fffffffffffff (9007199254740991)

Summary:
  0 correct transformations
  1 incorrect transformations
  0 failed-to-prove transformations
  0 Alive2 errors
regehr@john-home:~$ 

cc @aqjune @nunoplopes @ryan-berger @nbushehri @zhengyang92

[regehr]

(I should have added that you don't need to believe alive2 about the change in output, we verified this independently, splitting the roundtrip across functions to defeat the backend optimizations)

Johns-MacBook-Pro:~ regehr$ cat foo.c
#include <stdio.h>

unsigned long src(unsigned long);
unsigned long srcX(unsigned long);
unsigned long tgt(unsigned long);

int main(void) {
  unsigned long val = 18437736874454810623UL;
  unsigned long x = src(val);
  unsigned long z = srcX(val);
  unsigned long y = tgt(val);
  printf("%lx %lx %lx %lx\n", x, z, y, val);
  return 0;
}

Johns-MacBook-Pro:~ regehr$ cat foo.ll
define i64 @src(i64 %A) {
  %B = sitofp i64 %A to double
  %C = fptosi double %B to i54
  %D = zext i54 %C to i64
  ret i64 %D
}

define i54 @srcY(double %A) {
  %C = fptosi double %A to i54
  ret i54 %C
}

define i64 @srcX(i64 %A) {
  %B = sitofp i64 %A to double
  %C = call i54 @srcY(double %B)
  %D = zext i54 %C to i64
  ret i64 %D
}

define i64 @tgt(i64 %A) {
  %C = trunc i64 %A to i54
  %D = zext i54 %C to i64
  ret i64 %D
}
Johns-MacBook-Pro:~ regehr$ llc foo.ll && clang foo.s foo.c && ./a.out
1fffffffffffff 20000000000000 1fffffffffffff ffdfffffffffffff
Johns-MacBook-Pro:~ regehr$ 

[efriedma-quic]

Looks like an off-by-one error. LLVM doesn't do the optimization for i55, and I think the transformation is correct for i53.

[regehr]

Looks like an off-by-one error. LLVM doesn't do the optimization for i55, and I think the transformation is correct for i53.

this sounds right to me

[rotateright]

The instcombine bug appears to go back to before LLVM 3.9:
b9a0fa4

[rotateright]

https://reviews.llvm.org/D124692

[rotateright]

The same fold was added to DAGCombiner around the same time, so that's why it shows up in codegen too:
3e0023b

I can push another patch for that once we have an approved patch for instcombine.

[rotateright]

https://reviews.llvm.org/D124771