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.

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

[NewGVN] wrong code at Os and above with -mllvm -enable-newgvn #56149

Open
zhendongsu opened this issue Jun 21, 2022 · 3 comments
Open

[NewGVN] wrong code at Os and above with -mllvm -enable-newgvn #56149

zhendongsu opened this issue Jun 21, 2022 · 3 comments
Labels
llvm:GVN GVN and NewGVN stages (Global value numbering) miscompilation

Comments

@zhendongsu
Copy link

zhendongsu commented Jun 21, 2022

This appears to affect clang 5.* and later, so a quite long-latent regression.

% clangtk -v
clang version 15.0.0 (https://github.com/llvm/llvm-project.git 4d2eda2bb3156cee63ea486be34b01164b178e10)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /local/suz-local/opfuzz/bin
Found candidate GCC installation: /usr/lib/gcc/i686-linux-gnu/8
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/10
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/11
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/6
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/6.5.0
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/7
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/7.5.0
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/8
Selected GCC installation: /usr/lib/gcc/x86_64-linux-gnu/11
Candidate multilib: .;@m64
Selected multilib: .;@m64
% 
% clangtk -O0 small.c; ./a.out
% clangtk -O3 small.c; ./a.out
% 
% clangtk -O3 -mllvm -enable-newgvn small.c
% timeout -s 9 5 ./a.out
Killed
% 
% cat small.c
int printf(const char *, ...);
int a;
short b;
int main() {
  short d = 0;
  a = 9;
  for (; a > 5; a--)
    ;
  short e = a;
  void *f();
  if (e < 7) {
  L:
    d = a;
    a = ~(d & (&f || a));
  }
  while (a >= d)
    ;
  while (a == d)
    printf("0");
  if (b <= a)
    goto L;
  return 0;
}

Compiler Explorer: https://godbolt.org/z/sT1G5seEP

@zhendongsu zhendongsu changed the title wrong code at Os and above with -mllvm -enable-newgvn [NewGVN] wrong code at Os and above with -mllvm -enable-newgvn Jun 21, 2022
@fhahn
Copy link
Contributor

fhahn commented Jun 21, 2022

Thanks for filing NewGVN issues. Unfortunately there are quite a lot of known issues in NewGVN, so I am not sure if it makes sense to keep fuzzing it for now, as I expect most of the new issues will be duplicates of the existing ones.

@EugeneZelenko EugeneZelenko added llvm:GVN GVN and NewGVN stages (Global value numbering) and removed new issue labels Jun 21, 2022
@zhendongsu
Copy link
Author

Thanks for filing NewGVN issues. Unfortunately there are quite a lot of known issues in NewGVN, so I am not sure if it makes sense to keep fuzzing it for now, as I expect most of the new issues will be duplicates of the existing ones.

Okay, Florian, thanks for the note; let me stop testing NewGVN for now then.

@nunoplopes
Copy link
Member

I confirm it's a bug in NewGVN. Just not sure why Alive2 is not catching it 😕

It's propagating an equality to the else branch; pretty weird:

 %while.cond9.preheader:
   %2 = load i32, ptr @a, align 4
   %cmp1126 = icmp eq i32 %2, %conv6
   br i1 %cmp1126, label %while.body13.preheader, label %while.end14

 %while.end14:
-   %.lcssa = phi i32 [ %2, %while.cond9.preheader ], [ %.lcssa27, %while.end14.loopexit ]
   %4 = load i16, ptr @b, align 2
   %conv15 = sext i16 %4 to i32
-  %cmp16.not = icmp slt i32 %.lcssa, %conv15
+  %cmp16.not = icmp slt i32 %conv6, %conv15
   br i1 %cmp16.not, label %if.end19, label %L

I don't see any UB that could make Alive2 ignore the bug; I'll investigate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm:GVN GVN and NewGVN stages (Global value numbering) miscompilation
Projects
None yet
Development

No branches or pull requests

4 participants