llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-codegen Author: AZero13 (AZero13) <details> <summary>Changes</summary> As per the new guidelines, I finished this with the intention of being admitted into the 21.x release. It was finished for a few weeks before it was merged, and I would like to backport this. I apolgoize if this is getting too close to the merge window, but this is just in time, no? --- Patch is 32.47 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/150771.diff 11 Files Affected: - (modified) clang/lib/CodeGen/BackendUtil.cpp (-6) - (modified) llvm/include/llvm/Transforms/ObjCARC.h (-4) - (modified) llvm/lib/Passes/PassRegistry.def (-1) - (modified) llvm/lib/Transforms/ObjCARC/ARCRuntimeEntryPoints.h (+16) - (modified) llvm/lib/Transforms/ObjCARC/CMakeLists.txt (-1) - (removed) llvm/lib/Transforms/ObjCARC/ObjCARCAPElim.cpp (-156) - (modified) llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp (+187-5) - (modified) llvm/test/Transforms/ObjCARC/apelim.ll (+1-1) - (modified) llvm/test/Transforms/ObjCARC/comdat-ipo.ll (+1-1) - (added) llvm/test/Transforms/ObjCARC/test_autorelease_pool.ll (+319) - (modified) llvm/utils/gn/secondary/llvm/lib/Transforms/ObjCARC/BUILD.gn (-1) ``````````diff diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp index 2f6d4c414e737..17d83ff5dbebe 100644 --- a/clang/lib/CodeGen/BackendUtil.cpp +++ b/clang/lib/CodeGen/BackendUtil.cpp @@ -1027,12 +1027,6 @@ void EmitAssemblyHelper::RunOptimizationPipeline( MPM.addPass( createModuleToFunctionPassAdaptor(ObjCARCExpandPass())); }); - PB.registerPipelineEarlySimplificationEPCallback( - [](ModulePassManager &MPM, OptimizationLevel Level, - ThinOrFullLTOPhase) { - if (Level != OptimizationLevel::O0) - MPM.addPass(ObjCARCAPElimPass()); - }); PB.registerScalarOptimizerLateEPCallback( [](FunctionPassManager &FPM, OptimizationLevel Level) { if (Level != OptimizationLevel::O0) diff --git a/llvm/include/llvm/Transforms/ObjCARC.h b/llvm/include/llvm/Transforms/ObjCARC.h index c927513469a35..c4b4c4f0b09c6 100644 --- a/llvm/include/llvm/Transforms/ObjCARC.h +++ b/llvm/include/llvm/Transforms/ObjCARC.h @@ -35,10 +35,6 @@ struct ObjCARCContractPass : public PassInfoMixin<ObjCARCContractPass> { LLVM_ABI PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM); }; -struct ObjCARCAPElimPass : public PassInfoMixin<ObjCARCAPElimPass> { - LLVM_ABI PreservedAnalyses run(Module &M, ModuleAnalysisManager &AM); -}; - struct ObjCARCExpandPass : public PassInfoMixin<ObjCARCExpandPass> { LLVM_ABI PreservedAnalyses run(Function &M, FunctionAnalysisManager &AM); }; diff --git a/llvm/lib/Passes/PassRegistry.def b/llvm/lib/Passes/PassRegistry.def index 9a943155aa19f..8d86e2b996449 100644 --- a/llvm/lib/Passes/PassRegistry.def +++ b/llvm/lib/Passes/PassRegistry.def @@ -119,7 +119,6 @@ MODULE_PASS("module-inline", ModuleInlinerPass()) MODULE_PASS("name-anon-globals", NameAnonGlobalPass()) MODULE_PASS("no-op-module", NoOpModulePass()) MODULE_PASS("nsan", NumericalStabilitySanitizerPass()) -MODULE_PASS("objc-arc-apelim", ObjCARCAPElimPass()) MODULE_PASS("openmp-opt", OpenMPOptPass()) MODULE_PASS("openmp-opt-postlink", OpenMPOptPass(ThinOrFullLTOPhase::FullLTOPostLink)) diff --git a/llvm/lib/Transforms/ObjCARC/ARCRuntimeEntryPoints.h b/llvm/lib/Transforms/ObjCARC/ARCRuntimeEntryPoints.h index 3fa844eda21cf..6135c7b938a3e 100644 --- a/llvm/lib/Transforms/ObjCARC/ARCRuntimeEntryPoints.h +++ b/llvm/lib/Transforms/ObjCARC/ARCRuntimeEntryPoints.h @@ -46,6 +46,8 @@ enum class ARCRuntimeEntryPointKind { UnsafeClaimRV, RetainAutorelease, RetainAutoreleaseRV, + AutoreleasePoolPush, + AutoreleasePoolPop, }; /// Declarations for ObjC runtime functions and constants. These are initialized @@ -67,6 +69,8 @@ class ARCRuntimeEntryPoints { UnsafeClaimRV = nullptr; RetainAutorelease = nullptr; RetainAutoreleaseRV = nullptr; + AutoreleasePoolPush = nullptr; + AutoreleasePoolPop = nullptr; } Function *get(ARCRuntimeEntryPointKind kind) { @@ -101,6 +105,12 @@ class ARCRuntimeEntryPoints { case ARCRuntimeEntryPointKind::RetainAutoreleaseRV: return getIntrinsicEntryPoint(RetainAutoreleaseRV, Intrinsic::objc_retainAutoreleaseReturnValue); + case ARCRuntimeEntryPointKind::AutoreleasePoolPush: + return getIntrinsicEntryPoint(AutoreleasePoolPush, + Intrinsic::objc_autoreleasePoolPush); + case ARCRuntimeEntryPointKind::AutoreleasePoolPop: + return getIntrinsicEntryPoint(AutoreleasePoolPop, + Intrinsic::objc_autoreleasePoolPop); } llvm_unreachable("Switch should be a covered switch."); @@ -143,6 +153,12 @@ class ARCRuntimeEntryPoints { /// Declaration for objc_retainAutoreleaseReturnValue(). Function *RetainAutoreleaseRV = nullptr; + /// Declaration for objc_autoreleasePoolPush(). + Function *AutoreleasePoolPush = nullptr; + + /// Declaration for objc_autoreleasePoolPop(). + Function *AutoreleasePoolPop = nullptr; + Function *getIntrinsicEntryPoint(Function *&Decl, Intrinsic::ID IntID) { if (Decl) return Decl; diff --git a/llvm/lib/Transforms/ObjCARC/CMakeLists.txt b/llvm/lib/Transforms/ObjCARC/CMakeLists.txt index 80867dbc270d7..4274667a2c2b7 100644 --- a/llvm/lib/Transforms/ObjCARC/CMakeLists.txt +++ b/llvm/lib/Transforms/ObjCARC/CMakeLists.txt @@ -2,7 +2,6 @@ add_llvm_component_library(LLVMObjCARCOpts ObjCARC.cpp ObjCARCOpts.cpp ObjCARCExpand.cpp - ObjCARCAPElim.cpp ObjCARCContract.cpp DependencyAnalysis.cpp ProvenanceAnalysis.cpp diff --git a/llvm/lib/Transforms/ObjCARC/ObjCARCAPElim.cpp b/llvm/lib/Transforms/ObjCARC/ObjCARCAPElim.cpp deleted file mode 100644 index dceb2ebb1863e..0000000000000 --- a/llvm/lib/Transforms/ObjCARC/ObjCARCAPElim.cpp +++ /dev/null @@ -1,156 +0,0 @@ -//===- ObjCARCAPElim.cpp - ObjC ARC Optimization --------------------------===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// -/// \file -/// -/// This file defines ObjC ARC optimizations. ARC stands for Automatic -/// Reference Counting and is a system for managing reference counts for objects -/// in Objective C. -/// -/// This specific file implements optimizations which remove extraneous -/// autorelease pools. -/// -/// WARNING: This file knows about certain library functions. It recognizes them -/// by name, and hardwires knowledge of their semantics. -/// -/// WARNING: This file knows about how certain Objective-C library functions are -/// used. Naive LLVM IR transformations which would otherwise be -/// behavior-preserving may break these assumptions. -/// -//===----------------------------------------------------------------------===// - -#include "llvm/ADT/STLExtras.h" -#include "llvm/Analysis/ObjCARCAnalysisUtils.h" -#include "llvm/Analysis/ObjCARCInstKind.h" -#include "llvm/IR/Constants.h" -#include "llvm/IR/InstrTypes.h" -#include "llvm/IR/PassManager.h" -#include "llvm/Support/Debug.h" -#include "llvm/Support/raw_ostream.h" -#include "llvm/Transforms/ObjCARC.h" - -using namespace llvm; -using namespace llvm::objcarc; - -#define DEBUG_TYPE "objc-arc-ap-elim" - -namespace { - -/// Interprocedurally determine if calls made by the given call site can -/// possibly produce autoreleases. -bool MayAutorelease(const CallBase &CB, unsigned Depth = 0) { - if (const Function *Callee = CB.getCalledFunction()) { - if (!Callee->hasExactDefinition()) - return true; - for (const BasicBlock &BB : *Callee) { - for (const Instruction &I : BB) - if (const CallBase *JCB = dyn_cast<CallBase>(&I)) - // This recursion depth limit is arbitrary. It's just great - // enough to cover known interesting testcases. - if (Depth < 3 && !JCB->onlyReadsMemory() && - MayAutorelease(*JCB, Depth + 1)) - return true; - } - return false; - } - - return true; -} - -bool OptimizeBB(BasicBlock *BB) { - bool Changed = false; - - Instruction *Push = nullptr; - for (Instruction &Inst : llvm::make_early_inc_range(*BB)) { - switch (GetBasicARCInstKind(&Inst)) { - case ARCInstKind::AutoreleasepoolPush: - Push = &Inst; - break; - case ARCInstKind::AutoreleasepoolPop: - // If this pop matches a push and nothing in between can autorelease, - // zap the pair. - if (Push && cast<CallInst>(&Inst)->getArgOperand(0) == Push) { - Changed = true; - LLVM_DEBUG(dbgs() << "ObjCARCAPElim::OptimizeBB: Zapping push pop " - "autorelease pair:\n" - " Pop: " - << Inst << "\n" - << " Push: " << *Push - << "\n"); - Inst.eraseFromParent(); - Push->eraseFromParent(); - } - Push = nullptr; - break; - case ARCInstKind::CallOrUser: - if (MayAutorelease(cast<CallBase>(Inst))) - Push = nullptr; - break; - default: - break; - } - } - - return Changed; -} - -bool runImpl(Module &M) { - if (!EnableARCOpts) - return false; - - // If nothing in the Module uses ARC, don't do anything. - if (!ModuleHasARC(M)) - return false; - // Find the llvm.global_ctors variable, as the first step in - // identifying the global constructors. In theory, unnecessary autorelease - // pools could occur anywhere, but in practice it's pretty rare. Global - // ctors are a place where autorelease pools get inserted automatically, - // so it's pretty common for them to be unnecessary, and it's pretty - // profitable to eliminate them. - GlobalVariable *GV = M.getGlobalVariable("llvm.global_ctors"); - if (!GV) - return false; - - assert(GV->hasDefinitiveInitializer() && - "llvm.global_ctors is uncooperative!"); - - bool Changed = false; - - // Dig the constructor functions out of GV's initializer. - ConstantArray *Init = cast<ConstantArray>(GV->getInitializer()); - for (User::op_iterator OI = Init->op_begin(), OE = Init->op_end(); - OI != OE; ++OI) { - Value *Op = *OI; - // llvm.global_ctors is an array of three-field structs where the second - // members are constructor functions. - Function *F = dyn_cast<Function>(cast<ConstantStruct>(Op)->getOperand(1)); - // If the user used a constructor function with the wrong signature and - // it got bitcasted or whatever, look the other way. - if (!F) - continue; - // Only look at function definitions. - if (F->isDeclaration()) - continue; - // Only look at functions with one basic block. - if (std::next(F->begin()) != F->end()) - continue; - // Ok, a single-block constructor function definition. Try to optimize it. - Changed |= OptimizeBB(&F->front()); - } - - return Changed; -} - -} // namespace - -PreservedAnalyses ObjCARCAPElimPass::run(Module &M, ModuleAnalysisManager &AM) { - if (!runImpl(M)) - return PreservedAnalyses::all(); - PreservedAnalyses PA; - PA.preserveSet<CFGAnalyses>(); - return PA; -} diff --git a/llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp b/llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp index 5eb3f51d38945..66a2c7632aadc 100644 --- a/llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp +++ b/llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp @@ -39,6 +39,7 @@ #include "llvm/Analysis/ObjCARCAnalysisUtils.h" #include "llvm/Analysis/ObjCARCInstKind.h" #include "llvm/Analysis/ObjCARCUtil.h" +#include "llvm/Analysis/OptimizationRemarkEmitter.h" #include "llvm/IR/BasicBlock.h" #include "llvm/IR/CFG.h" #include "llvm/IR/Constant.h" @@ -132,11 +133,8 @@ static const Value *FindSingleUseIdentifiedObject(const Value *Arg) { // // The second retain and autorelease can be deleted. -// TODO: It should be possible to delete -// objc_autoreleasePoolPush and objc_autoreleasePoolPop -// pairs if nothing is actually autoreleased between them. Also, autorelease -// calls followed by objc_autoreleasePoolPop calls (perhaps in ObjC++ code -// after inlining) can be turned into plain release calls. +// TODO: Autorelease calls followed by objc_autoreleasePoolPop calls (perhaps in +// ObjC++ code after inlining) can be turned into plain release calls. // TODO: Critical-edge splitting. If the optimial insertion point is // a critical edge, the current algorithm has to fail, because it doesn't @@ -566,6 +564,8 @@ class ObjCARCOpt { void OptimizeReturns(Function &F); + void OptimizeAutoreleasePools(Function &F); + template <typename PredicateT> static void cloneOpBundlesIf(CallBase *CI, SmallVectorImpl<OperandBundleDef> &OpBundles, @@ -2473,6 +2473,11 @@ bool ObjCARCOpt::run(Function &F, AAResults &AA) { (1 << unsigned(ARCInstKind::AutoreleaseRV)))) OptimizeReturns(F); + // Optimizations for autorelease pools. + if (UsedInThisFunction & ((1 << unsigned(ARCInstKind::AutoreleasepoolPush)) | + (1 << unsigned(ARCInstKind::AutoreleasepoolPop)))) + OptimizeAutoreleasePools(F); + // Gather statistics after optimization. #ifndef NDEBUG if (AreStatisticsEnabled()) { @@ -2485,6 +2490,183 @@ bool ObjCARCOpt::run(Function &F, AAResults &AA) { return Changed; } +/// Interprocedurally determine if calls made by the given call site can +/// possibly produce autoreleases. +bool MayAutorelease(const CallBase &CB, unsigned Depth = 0) { + if (CB.onlyReadsMemory()) + return false; + + // This recursion depth limit is arbitrary. It's just great + // enough to cover known interesting testcases. + if (Depth > 5) + return true; + + if (const Function *Callee = CB.getCalledFunction()) { + if (!Callee->hasExactDefinition()) + return true; + for (const BasicBlock &BB : *Callee) { + for (const Instruction &I : BB) { + // TODO: Ignore all instructions between autorelease pools + ARCInstKind InstKind = GetBasicARCInstKind(&I); + switch (InstKind) { + case ARCInstKind::Autorelease: + case ARCInstKind::AutoreleaseRV: + case ARCInstKind::FusedRetainAutorelease: + case ARCInstKind::FusedRetainAutoreleaseRV: + case ARCInstKind::LoadWeak: + // These may produce autoreleases + return true; + + case ARCInstKind::Retain: + case ARCInstKind::RetainRV: + case ARCInstKind::UnsafeClaimRV: + case ARCInstKind::RetainBlock: + case ARCInstKind::Release: + case ARCInstKind::NoopCast: + case ARCInstKind::LoadWeakRetained: + case ARCInstKind::StoreWeak: + case ARCInstKind::InitWeak: + case ARCInstKind::MoveWeak: + case ARCInstKind::CopyWeak: + case ARCInstKind::DestroyWeak: + case ARCInstKind::StoreStrong: + case ARCInstKind::AutoreleasepoolPush: + case ARCInstKind::AutoreleasepoolPop: + // These ObjC runtime functions don't produce autoreleases + break; + + case ARCInstKind::CallOrUser: + case ARCInstKind::Call: + // For non-ObjC function calls, recursively analyze + if (MayAutorelease(cast<CallBase>(I), Depth + 1)) + return true; + break; + + case ARCInstKind::IntrinsicUser: + case ARCInstKind::User: + case ARCInstKind::None: + // These are not relevant for autorelease analysis + break; + } + } + } + return false; + } + + return true; +} + +/// Optimize autorelease pools by eliminating empty push/pop pairs. +void ObjCARCOpt::OptimizeAutoreleasePools(Function &F) { + LLVM_DEBUG(dbgs() << "\n== ObjCARCOpt::OptimizeAutoreleasePools ==\n"); + + OptimizationRemarkEmitter ORE(&F); + + // Process each basic block independently. + // TODO: Can we optimize inter-block autorelease pool pairs? + // This would involve tracking autorelease pool state across blocks. + for (BasicBlock &BB : F) { + // Use a stack to track nested autorelease pools + SmallVector<std::pair<CallInst *, bool>, 4> + PoolStack; // {push_inst, has_autorelease_in_scope} + + for (Instruction &Inst : llvm::make_early_inc_range(BB)) { + ARCInstKind Class = GetBasicARCInstKind(&Inst); + + switch (Class) { + case ARCInstKind::AutoreleasepoolPush: { + // Start tracking a new autorelease pool scope + auto *Push = cast<CallInst>(&Inst); + PoolStack.push_back( + {Push, false}); // {push_inst, has_autorelease_in_scope} + LLVM_DEBUG(dbgs() << "Found autorelease pool push: " << *Push << "\n"); + break; + } + + case ARCInstKind::AutoreleasepoolPop: { + auto *Pop = cast<CallInst>(&Inst); + + if (PoolStack.empty()) + break; + + auto &TopPool = PoolStack.back(); + CallInst *PendingPush = TopPool.first; + bool HasAutoreleaseInScope = TopPool.second; + + // Pop the stack - remove this pool scope + PoolStack.pop_back(); + + // Bail if this pop doesn't match the pending push + if (Pop->getArgOperand(0)->stripPointerCasts() != PendingPush) + break; + + // Bail if there were autoreleases in this scope + if (HasAutoreleaseInScope) + break; + + // Optimize: eliminate this empty autorelease pool pair + ORE.emit([&]() { + return OptimizationRemark(DEBUG_TYPE, "AutoreleasePoolElimination", + PendingPush) + << "eliminated empty autorelease pool pair"; + }); + + // Replace all uses of push with poison before deletion + PendingPush->replaceAllUsesWith( + PoisonValue::get(PendingPush->getType())); + + Pop->eraseFromParent(); + PendingPush->eraseFromParent(); + + Changed = true; + ++NumNoops; + break; + } + case ARCInstKind::CallOrUser: + case ARCInstKind::Call: + if (!MayAutorelease(cast<CallBase>(Inst))) + break; + LLVM_FALLTHROUGH; + case ARCInstKind::Autorelease: + case ARCInstKind::AutoreleaseRV: + case ARCInstKind::FusedRetainAutorelease: + case ARCInstKind::FusedRetainAutoreleaseRV: + case ARCInstKind::LoadWeak: { + // Track that we have autorelease calls in the current pool scope + if (!PoolStack.empty()) { + PoolStack.back().second = true; // Set has_autorelease_in_scope = true + LLVM_DEBUG( + dbgs() + << "Found autorelease or potential autorelease in pool scope: " + << Inst << "\n"); + } + break; + } + + // Enumerate all remaining ARCInstKind cases explicitly + case ARCInstKind::Retain: + case ARCInstKind::RetainRV: + case ARCInstKind::UnsafeClaimRV: + case ARCInstKind::RetainBlock: + case ARCInstKind::Release: + case ARCInstKind::NoopCast: + case ARCInstKind::LoadWeakRetained: + case ARCInstKind::StoreWeak: + case ARCInstKind::InitWeak: + case ARCInstKind::MoveWeak: + case ARCInstKind::CopyWeak: + case ARCInstKind::DestroyWeak: + case ARCInstKind::StoreStrong: + case ARCInstKind::IntrinsicUser: + case ARCInstKind::User: + case ARCInstKind::None: + // These instruction kinds don't affect autorelease pool optimization + break; + } + } + } +} + /// @} /// diff --git a/llvm/test/Transforms/ObjCARC/apelim.ll b/llvm/test/Transforms/ObjCARC/apelim.ll index 2ac5d15d0df85..01179f3dec048 100644 --- a/llvm/test/Transforms/ObjCARC/apelim.ll +++ b/llvm/test/Transforms/ObjCARC/apelim.ll @@ -1,4 +1,4 @@ -; RUN: opt -S -passes=objc-arc-apelim < %s | FileCheck %s +; RUN: opt -S -passes=objc-arc < %s | FileCheck %s ; rdar://10227311 @llvm.global_ctors = appending global [2 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 65535, ptr @_GLOBAL__I_x, ptr null }, { i32, ptr, ptr } { i32 65535, ptr @_GLOBAL__I_y, ptr null }] diff --git a/llvm/test/Transforms/ObjCARC/comdat-ipo.ll b/llvm/test/Transforms/ObjCARC/comdat-ipo.ll index 3f91d3bea9f14..d43804c20d936 100644 --- a/llvm/test/Transforms/ObjCARC/comdat-ipo.ll +++ b/llvm/test/Transforms/ObjCARC/comdat-ipo.ll @@ -1,4 +1,4 @@ -; RUN: opt -S -passes=objc-arc-apelim < %s | FileCheck %s +; RUN: opt -S -passes=objc-arc < %s | FileCheck %s ; See PR26774 diff --git a/llvm/test/Transforms/ObjCARC/test_autorelease_pool.ll b/llvm/test/Transforms/ObjCARC/test_autorelease_pool.ll new file mode 100644 index 0000000000000..896717f92146f --- /dev/null +++ b/llvm/test/Transforms/ObjCARC/test_autorelease_pool.ll @@ -0,0 +1,319 @@ +; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5 +; Test for autorelease pool optimizations +; RUN: opt -passes=objc-arc < %s -S | FileCheck %s + +declare ptr @llvm.objc.autoreleasePoolPush() +declare void @llvm.objc.autoreleasePoolPop(ptr) +declare ptr @llvm.objc.autorelease(ptr) +declare ptr @llvm.objc.retain(ptr) +declare ptr @create_object() +declare void @use_object(ptr) +declare ptr @object_with_thing() +declare void @opaque_callee() + +; Empty autorelease pool should be eliminated +define void @test_empty_pool() { +; CHECK-LABEL: define void @test_empty_pool() { +; C... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/150771 _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
