Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[RISCV] Miscompile with -O3 #90380

Closed
dtcxzyw opened this issue Apr 28, 2024 · 1 comment · Fixed by #90382
Closed

[RISCV] Miscompile with -O3 #90380

dtcxzyw opened this issue Apr 28, 2024 · 1 comment · Fixed by #90382
Assignees
@dtcxzyw
Labels
llvm:codegen miscompilation

Comments

@dtcxzyw
Copy link
Member

dtcxzyw commented Apr 28, 2024

Reduced test case:

#include "csmith.h"
uint8_t a = 55;
int32_t b;
int32_t c[4];
uint32_t e[4];
void f() {
  int32_t *o = &c[1];
  *o = 223;
  if (safe_mod_func_uint8_t_u_u(safe_sub_func_uint8_t_u_u(b, 1), a |= *o)) {
    e[0] = ++a;
  }
}
int main() {
  b = 0;
  f();
  printf("%d\n", a);
  return 0;
}

> bin/clang -O0 --target=riscv64-linux-gnu test.c -I/usr/include/csmith -w
> qemu-riscv64 -L /usr/riscv64-linux-gnu/ a.out
255
> bin/clang -O3 --target=riscv64-linux-gnu test.c -I/usr/include/csmith -w
> qemu-riscv64 -L /usr/riscv64-linux-gnu/ a.out
224

llvm version: d6c4ebb
@dtcxzyw dtcxzyw self-assigned this Apr 28, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 28, 2024

@llvm/issue-subscribers-backend-risc-v

Author: Yingwei Zheng (dtcxzyw)

Reduced test case: ``` #include "csmith.h" uint8_t a = 55; int32_t b; int32_t c[4]; uint32_t e[4]; void f() { int32_t *o = &c[1]; *o = 223; if (safe_mod_func_uint8_t_u_u(safe_sub_func_uint8_t_u_u(b, 1), a |= *o)) { e[0] = ++a; } } int main() { b = 0; f(); printf("%d\n", a); return 0; } ``` ``` > bin/clang -O0 --target=riscv64-linux-gnu test.c -I/usr/include/csmith -w > qemu-riscv64 -L /usr/riscv64-linux-gnu/ a.out 255 > bin/clang -O3 --target=riscv64-linux-gnu test.c -I/usr/include/csmith -w > qemu-riscv64 -L /usr/riscv64-linux-gnu/ a.out 224 ``` llvm version: d6c4ebb

@dtcxzyw dtcxzyw mentioned this issue Apr 28, 2024
Merged
dtcxzyw added a commit that referenced this issue Apr 29, 2024
@dtcxzyw
[CGP] Drop poison-generating flags after hoisting (#90382)
ab12bba 
See the following case:
```
define i8 @SRC1(i8 %x) {
entry:
  %cmp = icmp eq i8 %x, -1
  br i1 %cmp, label %exit, label %if.then

if.then:
  %inc = add nuw nsw i8 %x, 1
  br label %exit

exit:
  %retval = phi i8 [ %inc, %if.then ], [ -1, %entry ]
  ret i8 %retval
}

define i8 @tgt1(i8 %x) {
entry:
  %inc = add nuw nsw i8 %x, 1
  %0 = icmp eq i8 %inc, 0
  br i1 %0, label %exit, label %if.then

if.then:                                          ; preds = %entry
  br label %exit

exit:                                             ; preds = %if.then, %entry
  %retval = phi i8 [ %inc, %if.then ], [ -1, %entry ]
  ret i8 %retval
}
```
`optimizeBranch` converts `icmp eq X, -1` into cmp to zero on RISC-V and
hoists the add into the entry block. Poison-generating flags should be
dropped as they don't still hold.

Proof: https://alive2.llvm.org/ce/z/sP7mvK
Fixes #90380
llvmbot pushed a commit to llvmbot/llvm-project that referenced this issue Apr 29, 2024
@dtcxzyw @llvmbot
[CGP] Drop poison-generating flags after hoisting (llvm#90382)
237863e 
See the following case:
```
define i8 @SRC1(i8 %x) {
entry:
  %cmp = icmp eq i8 %x, -1
  br i1 %cmp, label %exit, label %if.then

if.then:
  %inc = add nuw nsw i8 %x, 1
  br label %exit

exit:
  %retval = phi i8 [ %inc, %if.then ], [ -1, %entry ]
  ret i8 %retval
}

define i8 @tgt1(i8 %x) {
entry:
  %inc = add nuw nsw i8 %x, 1
  %0 = icmp eq i8 %inc, 0
  br i1 %0, label %exit, label %if.then

if.then:                                          ; preds = %entry
  br label %exit

exit:                                             ; preds = %if.then, %entry
  %retval = phi i8 [ %inc, %if.then ], [ -1, %entry ]
  ret i8 %retval
}
```
`optimizeBranch` converts `icmp eq X, -1` into cmp to zero on RISC-V and
hoists the add into the entry block. Poison-generating flags should be
dropped as they don't still hold.

Proof: https://alive2.llvm.org/ce/z/sP7mvK
Fixes llvm#90380

(cherry picked from commit ab12bba)
tstellar pushed a commit to llvmbot/llvm-project that referenced this issue Apr 30, 2024
@dtcxzyw @tstellar
[CGP] Drop poison-generating flags after hoisting (llvm#90382)
6350acd 
See the following case:
```
define i8 @SRC1(i8 %x) {
entry:
  %cmp = icmp eq i8 %x, -1
  br i1 %cmp, label %exit, label %if.then

if.then:
  %inc = add nuw nsw i8 %x, 1
  br label %exit

exit:
  %retval = phi i8 [ %inc, %if.then ], [ -1, %entry ]
  ret i8 %retval
}

define i8 @tgt1(i8 %x) {
entry:
  %inc = add nuw nsw i8 %x, 1
  %0 = icmp eq i8 %inc, 0
  br i1 %0, label %exit, label %if.then

if.then:                                          ; preds = %entry
  br label %exit

exit:                                             ; preds = %if.then, %entry
  %retval = phi i8 [ %inc, %if.then ], [ -1, %entry ]
  ret i8 %retval
}
```
`optimizeBranch` converts `icmp eq X, -1` into cmp to zero on RISC-V and
hoists the add into the entry block. Poison-generating flags should be
dropped as they don't still hold.

Proof: https://alive2.llvm.org/ce/z/sP7mvK
Fixes llvm#90380

(cherry picked from commit ab12bba)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dtcxzyw dtcxzyw

Labels
llvm:codegen miscompilation
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[CGP] Drop poison-generating flags after hoisting dtcxzyw/llvm-project
3 participants
@EugeneZelenko @dtcxzyw @llvmbot