From 688e752207acc4308eae8a83b489366ec88793c9 Mon Sep 17 00:00:00 2001 From: Vlad Tsyrklevich Date: Tue, 10 Jul 2018 00:46:07 +0000 Subject: [PATCH] Revert "AMDGPU: Force inlining if LDS global address is used" This reverts commit r336587, it was causing test failures on the sanitizer bots. llvm-svn: 336623 --- llvm/lib/Target/AMDGPU/AMDGPUAlwaysInlinePass.cpp | 108 ++++----------------- llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp | 12 +-- llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.h | 1 - llvm/test/CodeGen/AMDGPU/early-inline.ll | 10 +- .../force-alwaysinline-lds-global-address.ll | 77 --------------- llvm/test/CodeGen/AMDGPU/stress-calls.ll | 2 +- 6 files changed, 28 insertions(+), 182 deletions(-) delete mode 100644 llvm/test/CodeGen/AMDGPU/force-alwaysinline-lds-global-address.ll diff --git a/llvm/lib/Target/AMDGPU/AMDGPUAlwaysInlinePass.cpp b/llvm/lib/Target/AMDGPU/AMDGPUAlwaysInlinePass.cpp index d4bbb2c..c274254 100644 --- a/llvm/lib/Target/AMDGPU/AMDGPUAlwaysInlinePass.cpp +++ b/llvm/lib/Target/AMDGPU/AMDGPUAlwaysInlinePass.cpp @@ -14,9 +14,6 @@ //===----------------------------------------------------------------------===// #include "AMDGPU.h" -#include "AMDGPUTargetMachine.h" -#include "Utils/AMDGPUBaseInfo.h" -#include "llvm/ADT/SmallPtrSet.h" #include "llvm/IR/Module.h" #include "llvm/Transforms/Utils/Cloning.h" @@ -33,18 +30,13 @@ static cl::opt StressCalls( class AMDGPUAlwaysInline : public ModulePass { bool GlobalOpt; - void recursivelyVisitUsers(GlobalValue &GV, - SmallPtrSetImpl &FuncsToAlwaysInline); public: static char ID; AMDGPUAlwaysInline(bool GlobalOpt = false) : ModulePass(ID), GlobalOpt(GlobalOpt) { } bool runOnModule(Module &M) override; - - void getAnalysisUsage(AnalysisUsage &AU) const override { - AU.setPreservesAll(); - } + StringRef getPassName() const override { return "AMDGPU Always Inline Pass"; } }; } // End anonymous namespace @@ -54,53 +46,15 @@ INITIALIZE_PASS(AMDGPUAlwaysInline, "amdgpu-always-inline", char AMDGPUAlwaysInline::ID = 0; -void AMDGPUAlwaysInline::recursivelyVisitUsers( - GlobalValue &GV, - SmallPtrSetImpl &FuncsToAlwaysInline) { - SmallVector Stack; - - SmallPtrSet Visited; - - for (User *U : GV.users()) - Stack.push_back(U); - - while (!Stack.empty()) { - User *U = Stack.pop_back_val(); - if (!Visited.insert(U).second) - continue; - - if (Instruction *I = dyn_cast(U)) { - Function *F = I->getParent()->getParent(); - if (!AMDGPU::isEntryFunctionCC(F->getCallingConv())) { - FuncsToAlwaysInline.insert(F); - Stack.push_back(F); - } - - // No need to look at further users, but we do need to inline any callers. - continue; - } - - for (User *UU : U->users()) - Stack.push_back(UU); - } -} - bool AMDGPUAlwaysInline::runOnModule(Module &M) { - AMDGPUAS AMDGPUAS = AMDGPU::getAMDGPUAS(M); - std::vector AliasesToRemove; - - SmallPtrSet FuncsToAlwaysInline; - SmallPtrSet FuncsToNoInline; + std::vector FuncsToClone; for (GlobalAlias &A : M.aliases()) { if (Function* F = dyn_cast(A.getAliasee())) { A.replaceAllUsesWith(F); AliasesToRemove.push_back(&A); } - - // FIXME: If the aliasee isn't a function, it's some kind of constant expr - // cast that won't be inlined through. } if (GlobalOpt) { @@ -109,51 +63,31 @@ bool AMDGPUAlwaysInline::runOnModule(Module &M) { } } - // Always force inlining of any function that uses an LDS global address. This - // is something of a workaround because we don't have a way of supporting LDS - // objects defined in functions. LDS is always allocated by a kernel, and it - // is difficult to manage LDS usage if a function may be used by multiple - // kernels. - // - // OpenCL doesn't allow declaring LDS in non-kernels, so in practice this - // should only appear when IPO passes manages to move LDs defined in a kernel - // into a single user function. - - for (GlobalVariable &GV : M.globals()) { - // TODO: Region address - unsigned AS = GV.getType()->getAddressSpace(); - if (AS != AMDGPUAS::LOCAL_ADDRESS && AS != AMDGPUAS.REGION_ADDRESS) - continue; - - recursivelyVisitUsers(GV, FuncsToAlwaysInline); - } + auto NewAttr = StressCalls ? Attribute::NoInline : Attribute::AlwaysInline; + auto IncompatAttr + = StressCalls ? Attribute::AlwaysInline : Attribute::NoInline; - if (!AMDGPUTargetMachine::EnableFunctionCalls || StressCalls) { - auto IncompatAttr - = StressCalls ? Attribute::AlwaysInline : Attribute::NoInline; - - for (Function &F : M) { - if (!F.isDeclaration() && !F.use_empty() && - !F.hasFnAttribute(IncompatAttr)) { - if (StressCalls) { - if (!FuncsToAlwaysInline.count(&F)) - FuncsToNoInline.insert(&F); - } else - FuncsToAlwaysInline.insert(&F); - } - } + for (Function &F : M) { + if (!F.hasLocalLinkage() && !F.isDeclaration() && !F.use_empty() && + !F.hasFnAttribute(IncompatAttr)) + FuncsToClone.push_back(&F); } - for (Function *F : FuncsToAlwaysInline) - F->addFnAttr(Attribute::AlwaysInline); - - for (Function *F : FuncsToNoInline) - F->addFnAttr(Attribute::NoInline); + for (Function *F : FuncsToClone) { + ValueToValueMapTy VMap; + Function *NewFunc = CloneFunction(F, VMap); + NewFunc->setLinkage(GlobalValue::InternalLinkage); + F->replaceAllUsesWith(NewFunc); + } - return !FuncsToAlwaysInline.empty() || !FuncsToNoInline.empty(); + for (Function &F : M) { + if (F.hasLocalLinkage() && !F.hasFnAttribute(IncompatAttr)) { + F.addFnAttr(NewAttr); + } + } + return false; } ModulePass *llvm::createAMDGPUAlwaysInlinePass(bool GlobalOpt) { return new AMDGPUAlwaysInline(GlobalOpt); } - diff --git a/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp b/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp index b406610..22ea160 100644 --- a/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp +++ b/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp @@ -117,12 +117,11 @@ static cl::opt LateCFGStructurize( cl::location(AMDGPUTargetMachine::EnableLateStructurizeCFG), cl::Hidden); -static cl::opt EnableAMDGPUFunctionCalls( +static cl::opt EnableAMDGPUFunctionCalls( "amdgpu-function-calls", + cl::Hidden, cl::desc("Enable AMDGPU function call support"), - cl::location(AMDGPUTargetMachine::EnableFunctionCalls), - cl::init(false), - cl::Hidden); + cl::init(false)); // Enable lib calls simplifications static cl::opt EnableLibCallSimplify( @@ -312,11 +311,10 @@ AMDGPUTargetMachine::AMDGPUTargetMachine(const Target &T, const Triple &TT, initAsmInfo(); } -bool AMDGPUTargetMachine::EnableLateStructurizeCFG = false; -bool AMDGPUTargetMachine::EnableFunctionCalls = false; - AMDGPUTargetMachine::~AMDGPUTargetMachine() = default; +bool AMDGPUTargetMachine::EnableLateStructurizeCFG = false; + StringRef AMDGPUTargetMachine::getGPUName(const Function &F) const { Attribute GPUAttr = F.getFnAttribute("target-cpu"); return GPUAttr.hasAttribute(Attribute::None) ? diff --git a/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.h b/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.h index 50b219d..4dcb1af 100644 --- a/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.h +++ b/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.h @@ -41,7 +41,6 @@ protected: public: static bool EnableLateStructurizeCFG; - static bool EnableFunctionCalls; AMDGPUTargetMachine(const Target &T, const Triple &TT, StringRef CPU, StringRef FS, TargetOptions Options, diff --git a/llvm/test/CodeGen/AMDGPU/early-inline.ll b/llvm/test/CodeGen/AMDGPU/early-inline.ll index eb53304..a4f970e 100644 --- a/llvm/test/CodeGen/AMDGPU/early-inline.ll +++ b/llvm/test/CodeGen/AMDGPU/early-inline.ll @@ -16,18 +16,10 @@ entry: ; CHECK: mul i32 ; CHECK-NOT: call i32 +; CHECK: define i32 @c_alias define amdgpu_kernel void @caller(i32 %x) { entry: %res = call i32 @callee(i32 %x) store volatile i32 %res, i32 addrspace(1)* undef ret void } - -; CHECK-LABEL: @alias_caller( -; CHECK-NOT: call -define amdgpu_kernel void @alias_caller(i32 %x) { -entry: - %res = call i32 @c_alias(i32 %x) - store volatile i32 %res, i32 addrspace(1)* undef - ret void -} diff --git a/llvm/test/CodeGen/AMDGPU/force-alwaysinline-lds-global-address.ll b/llvm/test/CodeGen/AMDGPU/force-alwaysinline-lds-global-address.ll deleted file mode 100644 index 63ee9e8..0000000 --- a/llvm/test/CodeGen/AMDGPU/force-alwaysinline-lds-global-address.ll +++ /dev/null @@ -1,77 +0,0 @@ -; RUN: opt -S -mtriple=amdgcn-amd-amdhsa -amdgpu-function-calls -amdgpu-always-inline %s | FileCheck -check-prefixes=CALLS-ENABLED,ALL %s -; RUN: opt -S -mtriple=amdgcn-amd-amdhsa -amdgpu-function-calls -amdgpu-stress-function-calls -amdgpu-always-inline %s | FileCheck -check-prefixes=STRESS-CALLS,ALL %s - -target datalayout = "e-p:32:32-p1:64:64-p2:64:64-p3:32:32-p4:64:64-p5:32:32-p24:64:64-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-A5" - -@lds0 = addrspace(3) global i32 undef, align 4 -@lds1 = addrspace(3) global [512 x i32] undef, align 4 -@nested.lds.address = addrspace(1) global i32 addrspace(3)* @lds0, align 4 -@gds0 = addrspace(2) global i32 undef, align 4 - -@alias.lds0 = alias i32, i32 addrspace(3)* @lds0 -@lds.cycle = addrspace(3) global i32 ptrtoint (i32 addrspace(3)* @lds.cycle to i32), align 4 - - -; ALL-LABEL: define i32 @load_lds_simple() #0 { -define i32 @load_lds_simple() { - %load = load i32, i32 addrspace(3)* @lds0, align 4 - ret i32 %load -} - -; ALL-LABEL: define i32 @load_gds_simple() #0 { -define i32 @load_gds_simple() { - %load = load i32, i32 addrspace(2)* @gds0, align 4 - ret i32 %load -} - -; ALL-LABEL: define i32 @load_lds_const_gep() #0 { -define i32 @load_lds_const_gep() { - %load = load i32, i32 addrspace(3)* getelementptr inbounds ([512 x i32], [512 x i32] addrspace(3)* @lds1, i64 0, i64 4), align 4 - ret i32 %load -} - -; ALL-LABEL: define i32 @load_lds_var_gep(i32 %idx) #0 { -define i32 @load_lds_var_gep(i32 %idx) { - %gep = getelementptr inbounds [512 x i32], [512 x i32] addrspace(3)* @lds1, i32 0, i32 %idx - %load = load i32, i32 addrspace(3)* %gep, align 4 - ret i32 %load -} - -; ALL-LABEL: define i32 addrspace(3)* @load_nested_address(i32 %idx) #0 { -define i32 addrspace(3)* @load_nested_address(i32 %idx) { - %load = load i32 addrspace(3)*, i32 addrspace(3)* addrspace(1)* @nested.lds.address, align 4 - ret i32 addrspace(3)* %load -} - -; ALL-LABEL: define i32 @load_lds_alias() #0 { -define i32 @load_lds_alias() { - %load = load i32, i32 addrspace(3)* @alias.lds0, align 4 - ret i32 %load -} - -; ALL-LABEL: define i32 @load_lds_cycle() #0 { -define i32 @load_lds_cycle() { - %load = load i32, i32 addrspace(3)* @lds.cycle, align 4 - ret i32 %load -} - -; ALL-LABEL: define i1 @icmp_lds_address() #0 { -define i1 @icmp_lds_address() { - ret i1 icmp eq (i32 addrspace(3)* @lds0, i32 addrspace(3)* null) -} - -; ALL-LABEL: define i32 @transitive_call() #0 { -define i32 @transitive_call() { - %call = call i32 @load_lds_simple() - ret i32 %call -} - -; ALL-LABEL: define i32 @recursive_call_lds(i32 %arg0) #0 { -define i32 @recursive_call_lds(i32 %arg0) { - %load = load i32, i32 addrspace(3)* @lds0, align 4 - %add = add i32 %arg0, %load - %call = call i32 @recursive_call_lds(i32 %add) - ret i32 %call -} - -; ALL: attributes #0 = { alwaysinline } diff --git a/llvm/test/CodeGen/AMDGPU/stress-calls.ll b/llvm/test/CodeGen/AMDGPU/stress-calls.ll index 8498076b..480d40d 100644 --- a/llvm/test/CodeGen/AMDGPU/stress-calls.ll +++ b/llvm/test/CodeGen/AMDGPU/stress-calls.ll @@ -1,4 +1,4 @@ -; RUN: opt -S -mtriple=amdgcn-amd-amdhsa -amdgpu-stress-function-calls -amdgpu-always-inline %s | FileCheck %s +; RUN: opt -S -amdgpu-stress-function-calls -amdgpu-always-inline %s | FileCheck %s ; CHECK: define internal fastcc i32 @alwaysinline_func(i32 %a) #0 { define internal fastcc i32 @alwaysinline_func(i32 %a) alwaysinline { -- 2.7.4