Skip to content

Wrong code at -O2 on x86_64-linux_gnu since Clang-15 #70547

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
shao-hua-li opened this issue Oct 28, 2023 · 2 comments · Fixed by #71066
Closed

Wrong code at -O2 on x86_64-linux_gnu since Clang-15 #70547

shao-hua-li opened this issue Oct 28, 2023 · 2 comments · Fixed by #71066

Comments

@shao-hua-li
Copy link

clang at -O2 produced the wrong code.

Bisected to b22ffc7, which was committed by @aeubanks

Compiler explorer: https://godbolt.org/z/MxrnaqTqc

% cat a.c
int printf(const char *, ...);
int a;
static int b;
short(c)(short d, int e) { return d >> e; }
char *f(int d, char *e) {
  if ((d & 1) == 0)
    return e;
  switch (d) {
  case 7:
  case 49:
  case 1:
    return "";
  case 11:
    return "0";
  case 3:
    return "0";
  }
  return "1";
}
char g(int d) {
  char h = 0, i = *f(d, &h);
  return i;
}
char j(int d) {
  for (;;) {
    if (g(d + 7) + d)
      return 0;
    for (; b - 30; b+=0)
      ;
  }
}
void k() {
  int l = -8, m = c(l, 8);
  j(m);
}
int main() {
  k();
  printf("%X\n", a);
}
% 
% clang -O0 a.c && ./a.out
0
% clang -O2 a.c && ./a.out
%
@nikic
Copy link
Contributor

nikic commented Oct 31, 2023

Reduced test case:

; RUN: opt -S -passes=dse < %s
define void @test() {
entry:
  %a = alloca i8
  store i8 0, ptr %a
  %call = call ptr @f(ptr %a) memory(none) nounwind willreturn
  %v = load i8, ptr %call
  %cmp = icmp ne i8 %v, 1
  call void @llvm.assume(i1 %cmp)
  ret void
}

declare ptr @f(ptr)
declare void @llvm.assume(i1)

This ends up dropping the store.

@nikic
Copy link
Contributor

nikic commented Oct 31, 2023

Similar case as https://reviews.llvm.org/D153464.

nikic added a commit that referenced this issue Oct 31, 2023
fhahn added a commit to fhahn/llvm-project that referenced this issue Nov 2, 2023
…inter escapeness"

Unfortunately the commit (D123162) introduced a mis-compile
(llvm#70547), which wasn't fixed
by the alternative fix (c0de28b)

I think as long as the call considered as ephemeral is not removed, we
need to be conservative. To address the correctness issue quickly, I
think we should revert the patch (as this patch does, it doens't revert
cleanly)

This reverts commit 17fdacc.

Fixes llvm#70547
fhahn added a commit that referenced this issue Nov 2, 2023
#71066)

Unfortunately the commit (D123162) introduced a mis-compile
(#70547), which wasn't fixed
by the alternative fix (c0de28b)

I think as long as the call considered as ephemeral is not removed, we
need to be conservative. To address the correctness issue quickly, I
think we should revert the patch (as this patch does, it doens't revert
cleanly)

This reverts commit 17fdacc.

Fixes #70547
qihangkong pushed a commit to rvgpu/llvm that referenced this issue Apr 18, 2024
…… (#71066)

Unfortunately the commit (D123162) introduced a mis-compile
(llvm/llvm-project#70547), which wasn't fixed
by the alternative fix (c0de28b)

I think as long as the call considered as ephemeral is not removed, we
need to be conservative. To address the correctness issue quickly, I
think we should revert the patch (as this patch does, it doens't revert
cleanly)

This reverts commit 17fdacc.

Fixes llvm/llvm-project#70547
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.

4 participants