chandlerc added a comment.

This really confused me. We shouldn't be seeing this kind of difference in the 
new PM.

But I think I figured it out.

Both PMs have to run *some* inliner at -O0. This is because we need to inline 
`always_inline` functions. The new PM has a *super* simple (and fast 
compile-time wise) inliner designed for this. It's really good at its job. The 
old PM runs the normal inliner. This thing isn't simple at all. It does complex 
stuff to incrementally simplify code while inlining because that can have a 
huge effect on the overall efficiency of the compiler *when doing full 
optimization passes*. But we're not doing that.

The result is that because all of these tests check code that is emitted by 
`always_inline` functions in our builtin headers, and then *inlined* into the 
test functions, the inliner in the old pass manager did a bunch of "cleanup" of 
the code that the new PM doesn't do.

Honestly, I think the new PM's behavior is correct for -O0, but I think it 
makes these tests less easy to understand. I think we should instead change the 
command running here, and when we do I think we will find a set of checks that 
work for both PMs.

When we are willing to do something like run `opt` in addition to clang (the 
`convergent.cl` case), instead of just doing `-instnamer` we should also do 
`-instcombine`. I suspect this will remove the differences.

When we're just directly using `%clang_cc1` I suspect that rather than checking 
the completely unoptimized code we should check with `-O1` to make the IR much 
more stable and easy to inspect. It should also result in identical results 
from the two PMs.



================
Comment at: clang/test/CodeGenOpenCL/convergent.cl:2
+// RUN: %clang_cc1 -triple spir-unknown-unknown -emit-llvm %s -o - 
-fno-experimental-new-pass-manager | opt -instnamer -S | FileCheck 
-enable-var-scope %s --check-prefixes=CHECK,CHECK-LEGACY
+// RUN: %clang_cc1 -triple spir-unknown-unknown -emit-llvm %s -o - 
-fexperimental-new-pass-manager | opt -instnamer -S | FileCheck 
-enable-var-scope %s --check-prefixes=CHECK,CHECK-NEWPM
 
----------------
Probably want to use `opt -passes=instnamer` or some such to use the new PM in 
the `opt` invocation as well...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63174/new/

https://reviews.llvm.org/D63174



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to