[PATCH] D63580: [clang][NewPM] Do not eliminate available_externally durng `-O2 -flto` runs

[Leonard Chan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
leonardchan marked an inline comment as done.
Closed by commit rG97dc622ab3f7: [clang][NewPM] Do not eliminate 
available_externally durng `-O2 -flto` runs (authored by leonardchan).

Changed prior to commit:
  https://reviews.llvm.org/D63580?vs=205699=205881#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63580

Files:
  clang/test/CodeGen/available-externally-suppress.c
  llvm/lib/Passes/PassBuilder.cpp
  llvm/test/Other/available-externally-lto.ll


Index: llvm/test/Other/available-externally-lto.ll
===
--- /dev/null
+++ llvm/test/Other/available-externally-lto.ll
@@ -0,0 +1,23 @@
+; Ensure that we don't emit available_externally functions at -O2, unless
+; -flto is present in which case we should preserve them for link-time inlining
+; decisions.
+; RUN: opt < %s -S -passes='default' | FileCheck %s
+; RUN: opt < %s -S -passes='lto-pre-link' | FileCheck %s --check-prefix=LTO
+
+@x = common local_unnamed_addr global i32 0, align 4
+
+define void @test() local_unnamed_addr #0 {
+entry:
+  tail call void @f0(i32 17)
+  ret void
+}
+
+; CHECK: declare void @f0(i32)
+; LTO: define available_externally void @f0(i32 %y)
+define available_externally void @f0(i32 %y) local_unnamed_addr #0 {
+entry:
+  store i32 %y, i32* @x, align 4
+  ret void
+}
+
+attributes #0 = { noinline }
Index: llvm/lib/Passes/PassBuilder.cpp
===
--- llvm/lib/Passes/PassBuilder.cpp
+++ llvm/lib/Passes/PassBuilder.cpp
@@ -812,8 +812,10 @@
   // available externally globals. Eventually they will be suppressed during
   // codegen, but eliminating here enables more opportunity for GlobalDCE as it
   // may make globals referenced by available external functions dead and saves
-  // running remaining passes on the eliminated functions.
-  MPM.addPass(EliminateAvailableExternallyPass());
+  // running remaining passes on the eliminated functions. These should be
+  // preserved during prelinking for link-time inlining decisions.
+  if (!LTOPreLink)
+MPM.addPass(EliminateAvailableExternallyPass());
 
   if (EnableOrderFileInstrumentation)
 MPM.addPass(InstrOrderFilePass());
Index: clang/test/CodeGen/available-externally-suppress.c
===
--- clang/test/CodeGen/available-externally-suppress.c
+++ clang/test/CodeGen/available-externally-suppress.c
@@ -1,6 +1,9 @@
-// RUN: %clang_cc1 -emit-llvm -o - -triple x86_64-apple-darwin10 %s | 
FileCheck %s
-// RUN: %clang_cc1 -O2 -fno-inline -emit-llvm -o - -triple 
x86_64-apple-darwin10 %s | FileCheck %s
-// RUN: %clang_cc1 -flto -O2 -fno-inline -emit-llvm -o - -triple 
x86_64-apple-darwin10 %s | FileCheck %s -check-prefix=LTO
+// RUN: %clang_cc1 -fno-experimental-new-pass-manager -emit-llvm -o - -triple 
x86_64-apple-darwin10 %s | FileCheck %s
+// RUN: %clang_cc1 -fno-experimental-new-pass-manager -O2 -fno-inline 
-emit-llvm -o - -triple x86_64-apple-darwin10 %s | FileCheck %s
+// RUN: %clang_cc1 -fno-experimental-new-pass-manager -flto -O2 -fno-inline 
-emit-llvm -o - -triple x86_64-apple-darwin10 %s | FileCheck %s 
-check-prefix=LTO
+// RUN: %clang_cc1 -fexperimental-new-pass-manager -emit-llvm -o - -triple 
x86_64-apple-darwin10 %s | FileCheck %s
+// RUN: %clang_cc1 -fexperimental-new-pass-manager -O2 -fno-inline -emit-llvm 
-o - -triple x86_64-apple-darwin10 %s | FileCheck %s
+// RUN: %clang_cc1 -fexperimental-new-pass-manager -flto -O2 -fno-inline 
-emit-llvm -o - -triple x86_64-apple-darwin10 %s | FileCheck %s 
-check-prefix=LTO
 
 // Ensure that we don't emit available_externally functions at -O0.
 // Also should not emit them at -O2, unless -flto is present in which case


Index: llvm/test/Other/available-externally-lto.ll
===
--- /dev/null
+++ llvm/test/Other/available-externally-lto.ll
@@ -0,0 +1,23 @@
+; Ensure that we don't emit available_externally functions at -O2, unless
+; -flto is present in which case we should preserve them for link-time inlining
+; decisions.
+; RUN: opt < %s -S -passes='default' | FileCheck %s
+; RUN: opt < %s -S -passes='lto-pre-link' | FileCheck %s --check-prefix=LTO
+
+@x = common local_unnamed_addr global i32 0, align 4
+
+define void @test() local_unnamed_addr #0 {
+entry:
+  tail call void @f0(i32 17)
+  ret void
+}
+
+; CHECK: declare void @f0(i32)
+; LTO: define available_externally void @f0(i32 %y)
+define available_externally void @f0(i32 %y) local_unnamed_addr #0 {
+entry:
+  store i32 %y, i32* @x, align 4
+  ret void
+}
+
+attributes #0 = { noinline }
Index: llvm/lib/Passes/PassBuilder.cpp
===
--- llvm/lib/Passes/PassBuilder.cpp
+++ llvm/lib/Passes/PassBuilder.cpp
@@ -812,8 

[PATCH] D63580: [clang][NewPM] Do not eliminate available_externally durng `-O2 -flto` runs

[Chandler Carruth via Phabricator via cfe-commits]

chandlerc accepted this revision.
chandlerc added a comment.
This revision is now accepted and ready to land.

LGTM, again, really nice. Tiny tweak below.




Comment at: llvm/test/Other/available-externally-lto.ll:1
+; Ensure that we don't emit available_externally functions at -O2, unless 
-flto is present in which case we should preserve them for link-time inlining 
decisions.
+; RUN: opt < %s -S -passes='default' | FileCheck %s

Wrap to 80-columns?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63580



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

[PATCH] D63580: [clang][NewPM] Do not eliminate available_externally durng `-O2 -flto` runs

[Leonard Chan via Phabricator via cfe-commits]

leonardchan created this revision.
leonardchan added reviewers: chandlerc, phosek, echristo, serge-sans-paille.
leonardchan added a project: clang.
Herald added subscribers: llvm-commits, dexonsmith, steven_wu, hiraditya, 
inglorion, mehdi_amini.
Herald added a project: LLVM.

This fixes `CodeGen/available-externally-suppress.c` when the new pass manager 
is turned on by default. `available_externally` was not emitted during `-O2 
-flto` runs when it should still be retained for link time inlining purposes. 
This can be fixed by checking that we aren't LTOPrelinking when adding the 
`EliminateAvailableExternallyPass`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63580

Files:
  clang/test/CodeGen/available-externally-suppress.c
  llvm/lib/Passes/PassBuilder.cpp
  llvm/test/Other/available-externally-lto.ll


Index: llvm/test/Other/available-externally-lto.ll
===
--- /dev/null
+++ llvm/test/Other/available-externally-lto.ll
@@ -0,0 +1,21 @@
+; Ensure that we don't emit available_externally functions at -O2, unless 
-flto is present in which case we should preserve them for link-time inlining 
decisions.
+; RUN: opt < %s -S -passes='default' | FileCheck %s
+; RUN: opt < %s -S -passes='lto-pre-link' | FileCheck %s --check-prefix=LTO
+
+@x = common local_unnamed_addr global i32 0, align 4
+
+define void @test() local_unnamed_addr #0 {
+entry:
+  tail call void @f0(i32 17)
+  ret void
+}
+
+; CHECK: declare void @f0(i32)
+; LTO: define available_externally void @f0(i32 %y)
+define available_externally void @f0(i32 %y) local_unnamed_addr #0 {
+entry:
+  store i32 %y, i32* @x, align 4
+  ret void
+}
+
+attributes #0 = { noinline }
Index: llvm/lib/Passes/PassBuilder.cpp
===
--- llvm/lib/Passes/PassBuilder.cpp
+++ llvm/lib/Passes/PassBuilder.cpp
@@ -812,8 +812,10 @@
   // available externally globals. Eventually they will be suppressed during
   // codegen, but eliminating here enables more opportunity for GlobalDCE as it
   // may make globals referenced by available external functions dead and saves
-  // running remaining passes on the eliminated functions.
-  MPM.addPass(EliminateAvailableExternallyPass());
+  // running remaining passes on the eliminated functions. These should be
+  // preserved during prelinking for link-time inlining decisions.
+  if (!LTOPreLink)
+MPM.addPass(EliminateAvailableExternallyPass());
 
   if (EnableOrderFileInstrumentation)
 MPM.addPass(InstrOrderFilePass());
Index: clang/test/CodeGen/available-externally-suppress.c
===
--- clang/test/CodeGen/available-externally-suppress.c
+++ clang/test/CodeGen/available-externally-suppress.c
@@ -1,6 +1,9 @@
-// RUN: %clang_cc1 -emit-llvm -o - -triple x86_64-apple-darwin10 %s | 
FileCheck %s
-// RUN: %clang_cc1 -O2 -fno-inline -emit-llvm -o - -triple 
x86_64-apple-darwin10 %s | FileCheck %s
-// RUN: %clang_cc1 -flto -O2 -fno-inline -emit-llvm -o - -triple 
x86_64-apple-darwin10 %s | FileCheck %s -check-prefix=LTO
+// RUN: %clang_cc1 -fno-experimental-new-pass-manager -emit-llvm -o - -triple 
x86_64-apple-darwin10 %s | FileCheck %s
+// RUN: %clang_cc1 -fno-experimental-new-pass-manager -O2 -fno-inline 
-emit-llvm -o - -triple x86_64-apple-darwin10 %s | FileCheck %s
+// RUN: %clang_cc1 -fno-experimental-new-pass-manager -flto -O2 -fno-inline 
-emit-llvm -o - -triple x86_64-apple-darwin10 %s | FileCheck %s 
-check-prefix=LTO
+// RUN: %clang_cc1 -fexperimental-new-pass-manager -emit-llvm -o - -triple 
x86_64-apple-darwin10 %s | FileCheck %s
+// RUN: %clang_cc1 -fexperimental-new-pass-manager -O2 -fno-inline -emit-llvm 
-o - -triple x86_64-apple-darwin10 %s | FileCheck %s
+// RUN: %clang_cc1 -fexperimental-new-pass-manager -flto -O2 -fno-inline 
-emit-llvm -o - -triple x86_64-apple-darwin10 %s | FileCheck %s 
-check-prefix=LTO
 
 // Ensure that we don't emit available_externally functions at -O0.
 // Also should not emit them at -O2, unless -flto is present in which case


Index: llvm/test/Other/available-externally-lto.ll
===
--- /dev/null
+++ llvm/test/Other/available-externally-lto.ll
@@ -0,0 +1,21 @@
+; Ensure that we don't emit available_externally functions at -O2, unless -flto is present in which case we should preserve them for link-time inlining decisions.
+; RUN: opt < %s -S -passes='default' | FileCheck %s
+; RUN: opt < %s -S -passes='lto-pre-link' | FileCheck %s --check-prefix=LTO
+
+@x = common local_unnamed_addr global i32 0, align 4
+
+define void @test() local_unnamed_addr #0 {
+entry:
+  tail call void @f0(i32 17)
+  ret void
+}
+
+; CHECK: declare void @f0(i32)
+; LTO: define available_externally void @f0(i32 %y)
+define available_externally void @f0(i32 %y) local_unnamed_addr #0 {
+entry:
+  store i32 %y, i32* @x, align 4
+  ret void
+}
+