Skip to content

[RISCV] Miscompile at -O3 #84200

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.

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

Closed
dtcxzyw opened this issue Mar 6, 2024 · 4 comments · Fixed by #84232
Closed

[RISCV] Miscompile at -O3 #84200

dtcxzyw opened this issue Mar 6, 2024 · 4 comments · Fixed by #84232

Comments

@dtcxzyw
Copy link
Member

dtcxzyw commented Mar 6, 2024

Reduced test case: https://godbolt.org/z/1sxbE8eco

#include <stdint.h>
#include <stdio.h>
int32_t a, f;
uint16_t e = 10;
static uint8_t f1(uint8_t c, uint8_t d) { return d == 0 ? 0 : c % d; }
static uint32_t f2(uint32_t c, uint32_t d) { return d == 0 ? c : 0; }
static int32_t *h(int32_t l) {
  int32_t *n = &a;
  return n;
}
static uint64_t g(int16_t k) {
  uint16_t m[] = {0, 3};
  int32_t *j = h(0);
  *j = f1(f2(1 && m[1], 0), e) == 0;
  return m[f];
}
int main() {
  g(f);
  printf("%d\n", a);
}
> clang -O0 test.c && ./a.out
0
> clang -O3 test.c --target=riscv64-linux-gnu
> qemu-riscv64 -L /usr/riscv64-linux-gnu/ ./a.out
-10

Clang version: 6e27dd4

@llvmbot
Copy link
Member

llvmbot commented Mar 6, 2024

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

Author: Yingwei Zheng (dtcxzyw)

Reduced test case: https://godbolt.org/z/1sxbE8eco ``` #include <stdint.h> #include <stdio.h> int32_t a, f; uint16_t e = 10; static uint8_t f1(uint8_t c, uint8_t d) { return d == 0 ? 0 : c % d; } static uint32_t f2(uint32_t c, uint32_t d) { return d == 0 ? c : 0; } static int32_t *h(int32_t l) { int32_t *n = &a; return n; } static uint64_t g(int16_t k) { uint16_t m[] = {0, 3}; int32_t *j = h(0); *j = f1(f2(1 && m[1], 0), e) == 0; return m[f]; } int main() { g(f); printf("%d\n", a); } ```
&gt; clang -O0 test.c &amp;&amp; ./a.out
0
&gt; clang -O3 test.c --target=riscv64-linux-gnu
&gt; qemu-riscv64 -L /usr/riscv64-linux-gnu/ ./a.out
-10

@topperc
Copy link
Collaborator

topperc commented Mar 6, 2024

Looks like TypePromotion. I'll take this.

@topperc
Copy link
Collaborator

topperc commented Mar 6, 2024

Still fails if I revert the most recent change to TypePromotion

commit 0813b90ff5d195d8a40c280f6b745f1cc43e087a
Author: Craig Topper <[email protected]>
Date:   Fri Mar 1 09:17:14 2024

    [TypePromotion] Support positive addition amounts in isSafeWrap. (#81690)

@topperc
Copy link
Collaborator

topperc commented Mar 6, 2024

The bug is in SelectionDAG but exposed by TypePromotion.

We convert some selects into AND/OR in LowerSelect. Select blocks poison propagation, but AND/OR does not. We need to insert a freeze to keep poison from the true/false operand from affecting the AND/OR.

@topperc topperc assigned topperc and unassigned dtcxzyw Mar 6, 2024
topperc added a commit to topperc/llvm-project that referenced this issue Mar 6, 2024
Select blocks poison, but AND/OR do not. We need to insert a freeze
to block poison propagation.

This creates suboptimal codegen which I will try to fix with other
patches. We should prioritize the correctness fix.

Fixes llvm#84200.
topperc added a commit that referenced this issue Mar 7, 2024
Select blocks poison, but AND/OR do not. We need to insert a freeze
to block poison propagation.

This creates suboptimal codegen which I will try to fix with other
patches. I'm prioritizing the correctness fix since we have 2 bug reports.

Fixes #84200 and #84350
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants