[llvm] 8815ce0 - Remove "Rewrite Symbols" from codegen pipeline

Arthur Eubanks via llvm-commits llvm-commits at lists.llvm.org
Mon May 31 08:32:47 PDT 2021


Author: Arthur Eubanks
Date: 2021-05-31T08:32:36-07:00
New Revision: 8815ce03e8575cb27e951b5f5a11384421730e20

URL: https://github.com/llvm/llvm-project/commit/8815ce03e8575cb27e951b5f5a11384421730e20
DIFF: https://github.com/llvm/llvm-project/commit/8815ce03e8575cb27e951b5f5a11384421730e20.diff

LOG: Remove "Rewrite Symbols" from codegen pipeline

It breaks up the function pass manager in the codegen pipeline.

With empty parameters, it looks at the -mllvm flag -rewrite-map-file.
This is likely not in use.

Add a check that we only have one function pass manager in the codegen
pipeline.

Some tests relied on the fact that we had a module pass somewhere in the
codegen pipeline.

addr-label.ll crashes on ARM due to this change. This is because a
ARMConstantPoolConstant containing a BasicBlock to represent a
blockaddress may hold an invalid pointer to a BasicBlock if the
blockaddress is invalidated by its BasicBlock getting removed. In that
case all referencing blockaddresses are RAUW a constant int. Making
ARMConstantPoolConstant::CVal a WeakVH fixes the crash, but I'm not sure
that's the right fix. As a workaround, create a barrier right before
ISel so that IR optimizations can't happen while a
ARMConstantPoolConstant has been created.

Reviewed By: rnk, MaskRay, compnerd

Differential Revision: https://reviews.llvm.org/D99707

Added: 
    

Modified: 
    llvm/lib/CodeGen/TargetPassConfig.cpp
    llvm/lib/Target/ARM/ARMTargetMachine.cpp
    llvm/lib/Target/ARM/CMakeLists.txt
    llvm/test/CodeGen/AArch64/O0-pipeline.ll
    llvm/test/CodeGen/AArch64/O3-pipeline.ll
    llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
    llvm/test/CodeGen/ARM/O3-pipeline.ll
    llvm/test/CodeGen/Generic/addr-label.ll
    llvm/test/CodeGen/X86/O0-pipeline.ll
    llvm/test/CodeGen/X86/opt-pipeline.ll
    llvm/test/CodeGen/X86/select_meta.ll
    llvm/test/Other/2010-05-06-Printer.ll
    llvm/utils/gn/secondary/llvm/lib/Target/ARM/BUILD.gn

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/TargetPassConfig.cpp b/llvm/lib/CodeGen/TargetPassConfig.cpp
index ed36384efdd94..2a820911a2c97 100644
--- a/llvm/lib/CodeGen/TargetPassConfig.cpp
+++ b/llvm/lib/CodeGen/TargetPassConfig.cpp
@@ -939,7 +939,6 @@ void TargetPassConfig::addPassesToHandleExceptions() {
 void TargetPassConfig::addCodeGenPrepare() {
   if (getOptLevel() != CodeGenOpt::None && !DisableCGP)
     addPass(createCodeGenPreparePass());
-  addPass(createRewriteSymbolsPass());
 }
 
 /// Add common passes that perform LLVM IR to IR transforms in preparation for

diff  --git a/llvm/lib/Target/ARM/ARMTargetMachine.cpp b/llvm/lib/Target/ARM/ARMTargetMachine.cpp
index 033b98a82715e..ae7ea7c2f415e 100644
--- a/llvm/lib/Target/ARM/ARMTargetMachine.cpp
+++ b/llvm/lib/Target/ARM/ARMTargetMachine.cpp
@@ -47,6 +47,7 @@
 #include "llvm/Target/TargetLoweringObjectFile.h"
 #include "llvm/Target/TargetOptions.h"
 #include "llvm/Transforms/CFGuard.h"
+#include "llvm/Transforms/IPO.h"
 #include "llvm/Transforms/Scalar.h"
 #include <cassert>
 #include <memory>
@@ -462,6 +463,14 @@ bool ARMPassConfig::addPreISel() {
   if (TM->getOptLevel() != CodeGenOpt::None) {
     addPass(createHardwareLoopsPass());
     addPass(createMVETailPredicationPass());
+    // FIXME: IR passes can delete address-taken basic blocks, deleting
+    // corresponding blockaddresses. ARMConstantPoolConstant holds references to
+    // address-taken basic blocks which can be invalidated if the function
+    // containing the blockaddress has already been codegen'd and the basic
+    // block is removed. Work around this by forcing all IR passes to run before
+    // any ISel takes place. We should have a more principled way of handling
+    // this. See D99707 for more details.
+    addPass(createBarrierNoopPass());
   }
 
   return false;

diff  --git a/llvm/lib/Target/ARM/CMakeLists.txt b/llvm/lib/Target/ARM/CMakeLists.txt
index 89abc579460b6..a3ec9e936d977 100644
--- a/llvm/lib/Target/ARM/CMakeLists.txt
+++ b/llvm/lib/Target/ARM/CMakeLists.txt
@@ -74,6 +74,7 @@ add_llvm_target(ARMCodeGen
   AsmPrinter
   CodeGen
   Core
+  IPO
   MC
   Scalar
   SelectionDAG

diff  --git a/llvm/test/CodeGen/AArch64/O0-pipeline.ll b/llvm/test/CodeGen/AArch64/O0-pipeline.ll
index 3e0fd31d98414..ca89aa6968e65 100644
--- a/llvm/test/CodeGen/AArch64/O0-pipeline.ll
+++ b/llvm/test/CodeGen/AArch64/O0-pipeline.ll
@@ -25,8 +25,6 @@
 ; CHECK-NEXT:       Scalarize Masked Memory Intrinsics
 ; CHECK-NEXT:       Expand reduction intrinsics
 ; CHECK-NEXT:       AArch64 Stack Tagging
-; CHECK-NEXT:     Rewrite Symbols
-; CHECK-NEXT:     FunctionPass Manager
 ; CHECK-NEXT:       Exception handling preparation
 ; CHECK-NEXT:       Safe Stack instrumentation pass
 ; CHECK-NEXT:       Insert stack protectors

diff  --git a/llvm/test/CodeGen/AArch64/O3-pipeline.ll b/llvm/test/CodeGen/AArch64/O3-pipeline.ll
index c9334024dabbf..5b59b2ee40249 100644
--- a/llvm/test/CodeGen/AArch64/O3-pipeline.ll
+++ b/llvm/test/CodeGen/AArch64/O3-pipeline.ll
@@ -77,8 +77,6 @@
 ; CHECK-NEXT:       Interleaved Access Pass
 ; CHECK-NEXT:       Natural Loop Information
 ; CHECK-NEXT:       CodeGen Prepare
-; CHECK-NEXT:     Rewrite Symbols
-; CHECK-NEXT:     FunctionPass Manager
 ; CHECK-NEXT:       Dominator Tree Construction
 ; CHECK-NEXT:       Exception handling preparation
 ; CHECK-NEXT:     AArch64 Promote Constant

diff  --git a/llvm/test/CodeGen/AMDGPU/llc-pipeline.ll b/llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
index 8ac236420538e..698891aaec144 100644
--- a/llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
+++ b/llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
@@ -62,8 +62,7 @@
 ; GCN-O0-NEXT:       FunctionPass Manager
 ; GCN-O0-NEXT:         AMDGPU Lower Kernel Arguments
 ; GCN-O0-NEXT:       Analysis if a function is memory bound
-; GCN-O0-NEXT:     Rewrite Symbols
-; GCN-O0-NEXT:     FunctionPass Manager
+; GCN-O0-NEXT:       FunctionPass Manager
 ; GCN-O0-NEXT:       Lazy Value Information Analysis
 ; GCN-O0-NEXT:       Lower SwitchInst's to branches
 ; GCN-O0-NEXT:       Lower invoke and unwind, for unwindless code generators
@@ -104,8 +103,6 @@
 ; GCN-O0-NEXT:       Natural Loop Information
 ; GCN-O0-NEXT:       LCSSA Verifier
 ; GCN-O0-NEXT:       Loop-Closed SSA Form Pass
-; GCN-O0-NEXT:     CallGraph Construction
-; GCN-O0-NEXT:     Call Graph SCC Pass Manager
 ; GCN-O0-NEXT:       DummyCGSCCPass
 ; GCN-O0-NEXT:       FunctionPass Manager
 ; GCN-O0-NEXT:         Safe Stack instrumentation pass
@@ -243,8 +240,6 @@
 ; GCN-O1-NEXT:         Dominator Tree Construction
 ; GCN-O1-NEXT:         Natural Loop Information
 ; GCN-O1-NEXT:         CodeGen Prepare
-; GCN-O1-NEXT:     Rewrite Symbols
-; GCN-O1-NEXT:     FunctionPass Manager
 ; GCN-O1-NEXT:       Lazy Value Information Analysis
 ; GCN-O1-NEXT:       Lower SwitchInst's to branches
 ; GCN-O1-NEXT:       Lower invoke and unwind, for unwindless code generators
@@ -285,8 +280,6 @@
 ; GCN-O1-NEXT:       Natural Loop Information
 ; GCN-O1-NEXT:       LCSSA Verifier
 ; GCN-O1-NEXT:       Loop-Closed SSA Form Pass
-; GCN-O1-NEXT:     CallGraph Construction
-; GCN-O1-NEXT:     Call Graph SCC Pass Manager
 ; GCN-O1-NEXT:       DummyCGSCCPass
 ; GCN-O1-NEXT:       FunctionPass Manager
 ; GCN-O1-NEXT:         Safe Stack instrumentation pass
@@ -512,8 +505,6 @@
 ; GCN-O1-OPTS-NEXT:         Dominator Tree Construction
 ; GCN-O1-OPTS-NEXT:         Natural Loop Information
 ; GCN-O1-OPTS-NEXT:         CodeGen Prepare
-; GCN-O1-OPTS-NEXT:     Rewrite Symbols
-; GCN-O1-OPTS-NEXT:     FunctionPass Manager
 ; GCN-O1-OPTS-NEXT:       Dominator Tree Construction
 ; GCN-O1-OPTS-NEXT:       Basic Alias Analysis (stateless AA impl)
 ; GCN-O1-OPTS-NEXT:       Function Alias Analysis Results
@@ -560,8 +551,6 @@
 ; GCN-O1-OPTS-NEXT:       Natural Loop Information
 ; GCN-O1-OPTS-NEXT:       LCSSA Verifier
 ; GCN-O1-OPTS-NEXT:       Loop-Closed SSA Form Pass
-; GCN-O1-OPTS-NEXT:     CallGraph Construction
-; GCN-O1-OPTS-NEXT:     Call Graph SCC Pass Manager
 ; GCN-O1-OPTS-NEXT:       DummyCGSCCPass
 ; GCN-O1-OPTS-NEXT:       FunctionPass Manager
 ; GCN-O1-OPTS-NEXT:         Safe Stack instrumentation pass
@@ -795,8 +784,6 @@
 ; GCN-O2-NEXT:         Dominator Tree Construction
 ; GCN-O2-NEXT:         Natural Loop Information
 ; GCN-O2-NEXT:         CodeGen Prepare
-; GCN-O2-NEXT:     Rewrite Symbols
-; GCN-O2-NEXT:     FunctionPass Manager
 ; GCN-O2-NEXT:       Dominator Tree Construction
 ; GCN-O2-NEXT:       Basic Alias Analysis (stateless AA impl)
 ; GCN-O2-NEXT:       Function Alias Analysis Results
@@ -843,8 +830,6 @@
 ; GCN-O2-NEXT:       Natural Loop Information
 ; GCN-O2-NEXT:       LCSSA Verifier
 ; GCN-O2-NEXT:       Loop-Closed SSA Form Pass
-; GCN-O2-NEXT:     CallGraph Construction
-; GCN-O2-NEXT:     Call Graph SCC Pass Manager
 ; GCN-O2-NEXT:       DummyCGSCCPass
 ; GCN-O2-NEXT:       FunctionPass Manager
 ; GCN-O2-NEXT:         Safe Stack instrumentation pass
@@ -1092,8 +1077,6 @@
 ; GCN-O3-NEXT:         Dominator Tree Construction
 ; GCN-O3-NEXT:         Natural Loop Information
 ; GCN-O3-NEXT:         CodeGen Prepare
-; GCN-O3-NEXT:     Rewrite Symbols
-; GCN-O3-NEXT:     FunctionPass Manager
 ; GCN-O3-NEXT:       Dominator Tree Construction
 ; GCN-O3-NEXT:       Basic Alias Analysis (stateless AA impl)
 ; GCN-O3-NEXT:       Function Alias Analysis Results
@@ -1140,8 +1123,6 @@
 ; GCN-O3-NEXT:       Natural Loop Information
 ; GCN-O3-NEXT:       LCSSA Verifier
 ; GCN-O3-NEXT:       Loop-Closed SSA Form Pass
-; GCN-O3-NEXT:     CallGraph Construction
-; GCN-O3-NEXT:     Call Graph SCC Pass Manager
 ; GCN-O3-NEXT:       DummyCGSCCPass
 ; GCN-O3-NEXT:       FunctionPass Manager
 ; GCN-O3-NEXT:         Safe Stack instrumentation pass

diff  --git a/llvm/test/CodeGen/ARM/O3-pipeline.ll b/llvm/test/CodeGen/ARM/O3-pipeline.ll
index 07459aa93bb91..405d68c77d80c 100644
--- a/llvm/test/CodeGen/ARM/O3-pipeline.ll
+++ b/llvm/test/CodeGen/ARM/O3-pipeline.ll
@@ -50,8 +50,6 @@
 ; CHECK-NEXT:      Dominator Tree Construction
 ; CHECK-NEXT:      Natural Loop Information
 ; CHECK-NEXT:      CodeGen Prepare
-; CHECK-NEXT:    Rewrite Symbols
-; CHECK-NEXT:    FunctionPass Manager
 ; CHECK-NEXT:      Dominator Tree Construction
 ; CHECK-NEXT:      Exception handling preparation
 ; CHECK-NEXT:      Merge internal globals
@@ -64,9 +62,12 @@
 ; CHECK-NEXT:      Scalar Evolution Analysis
 ; CHECK-NEXT:      Loop Pass Manager
 ; CHECK-NEXT:        Transform predicated vector loops to use MVE tail predication
+; CHECK-NEXT:      A No-Op Barrier Pass
+; CHECK-NEXT:      FunctionPass Manager
 ; CHECK-NEXT:      Safe Stack instrumentation pass
 ; CHECK-NEXT:      Insert stack protectors
 ; CHECK-NEXT:      Module Verifier
+; CHECK-NEXT:      Dominator Tree Construction
 ; CHECK-NEXT:      Basic Alias Analysis (stateless AA impl)
 ; CHECK-NEXT:      Function Alias Analysis Results
 ; CHECK-NEXT:      Natural Loop Information

diff  --git a/llvm/test/CodeGen/Generic/addr-label.ll b/llvm/test/CodeGen/Generic/addr-label.ll
index 0dbe5021bbf08..e64f56564b633 100644
--- a/llvm/test/CodeGen/Generic/addr-label.ll
+++ b/llvm/test/CodeGen/Generic/addr-label.ll
@@ -1,3 +1,4 @@
+; RUN: llc %s -o - -mtriple=thumbv7-apple-darwin10
 ; RUN: llc %s -o -
 
 ;; Reference to a label that gets deleted.
@@ -16,7 +17,7 @@ ret:
 }
 
 
-;; Issues with referring to a label that gets RAUW'd later.
+; Issues with referring to a label that gets RAUW'd later.
 define i32 @test2a() nounwind {
 entry:
         %target = bitcast i8* blockaddress(@test2b, %test_label) to i8*

diff  --git a/llvm/test/CodeGen/X86/O0-pipeline.ll b/llvm/test/CodeGen/X86/O0-pipeline.ll
index 559739a5e00e6..82d9d1ed0e6c7 100644
--- a/llvm/test/CodeGen/X86/O0-pipeline.ll
+++ b/llvm/test/CodeGen/X86/O0-pipeline.ll
@@ -30,8 +30,6 @@
 ; CHECK-NEXT:       Scalarize Masked Memory Intrinsics
 ; CHECK-NEXT:       Expand reduction intrinsics
 ; CHECK-NEXT:       Expand indirectbr instructions
-; CHECK-NEXT:     Rewrite Symbols
-; CHECK-NEXT:     FunctionPass Manager
 ; CHECK-NEXT:       Exception handling preparation
 ; CHECK-NEXT:       Safe Stack instrumentation pass
 ; CHECK-NEXT:       Insert stack protectors

diff  --git a/llvm/test/CodeGen/X86/opt-pipeline.ll b/llvm/test/CodeGen/X86/opt-pipeline.ll
index 2f649e957c0fc..da25176ca7975 100644
--- a/llvm/test/CodeGen/X86/opt-pipeline.ll
+++ b/llvm/test/CodeGen/X86/opt-pipeline.ll
@@ -6,6 +6,8 @@
 ; RUN:   | grep -v 'Verify generated machine code' | FileCheck %s
 ; RUN: llc -mtriple=x86_64-- -O3 -debug-pass=Structure < %s -o /dev/null 2>&1 \
 ; RUN:   | grep -v 'Verify generated machine code' | FileCheck %s
+; RUN: llc -mtriple=x86_64-- -O3 -debug-pass=Structure < %s -o /dev/null 2>&1 \
+; RUN:   | FileCheck %s --check-prefix=FPM
 
 ; REQUIRES: asserts
 
@@ -62,8 +64,6 @@
 ; CHECK-NEXT:       Expand indirectbr instructions
 ; CHECK-NEXT:       Natural Loop Information
 ; CHECK-NEXT:       CodeGen Prepare
-; CHECK-NEXT:     Rewrite Symbols
-; CHECK-NEXT:     FunctionPass Manager
 ; CHECK-NEXT:       Dominator Tree Construction
 ; CHECK-NEXT:       Exception handling preparation
 ; CHECK-NEXT:       Safe Stack instrumentation pass
@@ -204,6 +204,12 @@
 ; CHECK-NEXT:       X86 Assembly Printer
 ; CHECK-NEXT:       Free MachineFunction
 
+; We should only have one function pass manager.
+; In the past, module passes have accidentally been added into the middle of
+; the codegen pipeline, implicitly creating new function pass managers.
+; FPM: FunctionPass Manager
+; FPM-NOT: FunctionPass Manager
+
 define void @f() {
   ret void
 }

diff  --git a/llvm/test/CodeGen/X86/select_meta.ll b/llvm/test/CodeGen/X86/select_meta.ll
index 2c73f767e3753..e89008406f1ca 100644
--- a/llvm/test/CodeGen/X86/select_meta.ll
+++ b/llvm/test/CodeGen/X86/select_meta.ll
@@ -1,4 +1,4 @@
-; RUN: llc -mtriple=x86_64-unknown-unknown -print-after-all < %s 2>&1 | FileCheck %s
+; RUN: opt -mtriple=x86_64-unknown-unknown -codegenprepare -S < %s 2>&1 | FileCheck %s
 
 ; Function Attrs: norecurse nounwind readnone uwtable
 define i32 @foo(i32, i32, i32) {

diff  --git a/llvm/test/Other/2010-05-06-Printer.ll b/llvm/test/Other/2010-05-06-Printer.ll
index decd977c3d212..fd73791d475ed 100644
--- a/llvm/test/Other/2010-05-06-Printer.ll
+++ b/llvm/test/Other/2010-05-06-Printer.ll
@@ -10,10 +10,8 @@ define void @foo(){
   ret void
 }
 
-;ALL-NOT: IR Dump After {{.*}}; ModuleID =
 ;ALL: define void @tester()
 ;ALL: define void @foo()
-;ALL: ModuleID =
 
 ;FOO: IR Dump After
 ;FOO-NEXT: define void @foo()

diff  --git a/llvm/utils/gn/secondary/llvm/lib/Target/ARM/BUILD.gn b/llvm/utils/gn/secondary/llvm/lib/Target/ARM/BUILD.gn
index 2847ac1bd3087..5d9156e653be3 100644
--- a/llvm/utils/gn/secondary/llvm/lib/Target/ARM/BUILD.gn
+++ b/llvm/utils/gn/secondary/llvm/lib/Target/ARM/BUILD.gn
@@ -59,6 +59,7 @@ static_library("LLVMARMCodeGen") {
     "//llvm/lib/Target",
     "//llvm/lib/Transforms/CFGuard",
     "//llvm/lib/Transforms/Utils",
+    "//llvm/lib/Transforms/IPO",
   ]
   include_dirs = [ "." ]
   sources = [


        


More information about the llvm-commits mailing list