Skip to content

[LTO] enable ObjCARCContractPass only on optimized build #101114

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Aug 9, 2024
Merged

[LTO] enable ObjCARCContractPass only on optimized build #101114

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ebbba2b
Reapply "Run ObjCContractPass in Default Codegen Pipeline (#92331)"
DataCorrupted Jul 29, 2024
a1604de
[LTO] enable `ObjCARCContractPass` only on optimized build
DataCorrupted Jul 30, 2024
4fc7867
Revert unnecessary format changes in ObjCARCAnalysisUtils.h
DataCorrupted Jul 30, 2024
a45b3b9
Add `ModuleHasARC` check back to skip the pass on non objc modules.
DataCorrupted Aug 5, 2024
34e6238
Indicate that DT is preserved in ObjCARCContractPass
DataCorrupted Aug 6, 2024
c9c15bb
preserving AA analysis as well
DataCorrupted Aug 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions clang/lib/CodeGen/BackendUtil.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -588,12 +588,6 @@ bool EmitAssemblyHelper::AddEmitPasses(legacy::PassManager &CodeGenPasses,
// this also adds codegenerator level optimization passes.
CodeGenFileType CGFT = getCodeGenFileType(Action);

// Add ObjC ARC final-cleanup optimizations. This is done as part of the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cachemeifyoucan As far as I can tell, it's already the case that for non-LTO builds, this pass is only added for optimization level > 0. Is that ok to do for all builds?

I checked and directly providing clang with IR that contains llvm.objc.clang.arc.use and compiling it with -O0 fails instruction selection as a result of not running this pass, so I'm assuming these intrinsics are only introduced at -O1 or higher.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ahatanak probably know better. I think when the arc intrinsics are present, the pass is mandatory but I am not sure if the intrinsics are only produced for -O1 and higher.

Copy link
Member Author

@DataCorrupted DataCorrupted Jul 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't run this optimization on O0 has been there since LLVM 3.1 (1170b08). The committer @sunfishcode probably did it for a reason

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The intrinsic calls are needed to prevent ARC optimizer from moving releases to an earlier point that can cause objects to be deallocated prematurely. clang doesn't emit the intrinsic calls if OptimizationLevel is zero.

// "codegen" passes so that it isn't run multiple times when there is
// inlining happening.
if (CodeGenOpts.OptimizationLevel > 0)
CodeGenPasses.add(createObjCARCContractPass());

if (TM->addPassesToEmitFile(CodeGenPasses, OS, DwoOS, CGFT,
/*DisableVerify=*/!CodeGenOpts.VerifyModule)) {
Diags.Report(diag::err_fe_unable_to_interface_with_target);
Expand Down
19 changes: 19 additions & 0 deletions clang/test/CodeGen/thinlto-distributed-objc-contract-pass.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
; RUN: opt -thinlto-bc -o %t.o %s

; RUN: llvm-lto2 run -thinlto-distributed-indexes %t.o \
; RUN: -o %t2.index \
; RUN: -r=%t.o,_use_arc,px

; RUN: %clang_cc1 -O2 -triple x86_64-apple-darwin \
; RUN: -emit-obj -fthinlto-index=%t.o.thinlto.bc \
; RUN: -o %t.native.o -x ir %t.o

target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-apple-darwin"

define void @use_arc(ptr %a, ptr %b) {
call void (...) @llvm.objc.clang.arc.use(ptr %a, ptr %b) nounwind
ret void
}

declare void @llvm.objc.clang.arc.use(...) nounwind
3 changes: 0 additions & 3 deletions lld/MachO/LTO.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,6 @@ static lto::Config createConfig() {
c.CPU = getCPUStr();
c.MAttrs = getMAttrs();
c.DiagHandler = diagnosticHandler;
c.PreCodeGenPassesHook = [](legacy::PassManager &pm) {
pm.add(createObjCARCContractPass());
};

c.AlwaysEmitRegularLTOObj = !config->ltoObjPath.empty();

Expand Down
40 changes: 20 additions & 20 deletions llvm/include/llvm/Analysis/ObjCARCAnalysisUtils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,26 +41,26 @@ extern bool EnableARCOpts;
/// Test if the given module looks interesting to run ARC optimization
/// on.
inline bool ModuleHasARC(const Module &M) {
return
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we're not calling this anymore, I don't think we need this change.

M.getNamedValue("llvm.objc.retain") ||
M.getNamedValue("llvm.objc.release") ||
M.getNamedValue("llvm.objc.autorelease") ||
M.getNamedValue("llvm.objc.retainAutoreleasedReturnValue") ||
M.getNamedValue("llvm.objc.unsafeClaimAutoreleasedReturnValue") ||
M.getNamedValue("llvm.objc.retainBlock") ||
M.getNamedValue("llvm.objc.autoreleaseReturnValue") ||
M.getNamedValue("llvm.objc.autoreleasePoolPush") ||
M.getNamedValue("llvm.objc.loadWeakRetained") ||
M.getNamedValue("llvm.objc.loadWeak") ||
M.getNamedValue("llvm.objc.destroyWeak") ||
M.getNamedValue("llvm.objc.storeWeak") ||
M.getNamedValue("llvm.objc.initWeak") ||
M.getNamedValue("llvm.objc.moveWeak") ||
M.getNamedValue("llvm.objc.copyWeak") ||
M.getNamedValue("llvm.objc.retainedObject") ||
M.getNamedValue("llvm.objc.unretainedObject") ||
M.getNamedValue("llvm.objc.unretainedPointer") ||
M.getNamedValue("llvm.objc.clang.arc.use");
return M.getNamedValue("llvm.objc.retain") ||
M.getNamedValue("llvm.objc.release") ||
M.getNamedValue("llvm.objc.autorelease") ||
M.getNamedValue("llvm.objc.retainAutoreleasedReturnValue") ||
M.getNamedValue("llvm.objc.unsafeClaimAutoreleasedReturnValue") ||
M.getNamedValue("llvm.objc.retainBlock") ||
M.getNamedValue("llvm.objc.autoreleaseReturnValue") ||
M.getNamedValue("llvm.objc.autoreleasePoolPush") ||
M.getNamedValue("llvm.objc.loadWeakRetained") ||
M.getNamedValue("llvm.objc.loadWeak") ||
M.getNamedValue("llvm.objc.destroyWeak") ||
M.getNamedValue("llvm.objc.storeWeak") ||
M.getNamedValue("llvm.objc.initWeak") ||
M.getNamedValue("llvm.objc.moveWeak") ||
M.getNamedValue("llvm.objc.copyWeak") ||
M.getNamedValue("llvm.objc.retainedObject") ||
M.getNamedValue("llvm.objc.unretainedObject") ||
M.getNamedValue("llvm.objc.unretainedPointer") ||
M.getNamedValue("llvm.objc.clang.arc.noop.use") ||
M.getNamedValue("llvm.objc.clang.arc.use");
}

/// This is a wrapper around getUnderlyingObject which also knows how to
Expand Down
4 changes: 4 additions & 0 deletions llvm/lib/CodeGen/TargetPassConfig.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
#include "llvm/Support/WithColor.h"
#include "llvm/Target/CGPassBuilderOption.h"
#include "llvm/Target/TargetMachine.h"
#include "llvm/Transforms/ObjCARC.h"
#include "llvm/Transforms/Scalar.h"
#include "llvm/Transforms/Utils.h"
#include <cassert>
Expand Down Expand Up @@ -943,6 +944,9 @@ void TargetPassConfig::addCodeGenPrepare() {
void TargetPassConfig::addISelPrepare() {
addPreISel();

if (getOptLevel() != CodeGenOptLevel::None)
addPass(createObjCARCContractPass());

// Force codegen to run according to the callgraph.
if (requiresCodeGenSCCOrder())
addPass(new DummyCGSCCPass);
Expand Down
4 changes: 0 additions & 4 deletions llvm/lib/LTO/LTOCodeGenerator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,10 +136,6 @@ LTOCodeGenerator::LTOCodeGenerator(LLVMContext &Context)

Config.CodeModel = std::nullopt;
Config.StatsFile = LTOStatsFile;
Config.PreCodeGenPassesHook = [](legacy::PassManager &PM) {
PM.add(createObjCARCContractPass());
};

Config.RunCSIRInstr = LTORunCSIRInstr;
Config.CSIRProfile = LTOCSIRProfile;
}
Expand Down
4 changes: 0 additions & 4 deletions llvm/lib/LTO/ThinLTOCodeGenerator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,10 +335,6 @@ std::unique_ptr<MemoryBuffer> codegenModule(Module &TheModule,
raw_svector_ostream OS(OutputBuffer);
legacy::PassManager PM;

// If the bitcode files contain ARC code and were compiled with optimization,
// the ObjCARCContractPass must be run, so do it unconditionally here.
PM.add(createObjCARCContractPass());

// Setup the codegen now.
if (TM.addPassesToEmitFile(PM, OS, nullptr, CodeGenFileType::ObjectFile,
/* DisableVerify */ true))
Expand Down
14 changes: 14 additions & 0 deletions llvm/lib/Transforms/ObjCARC/ObjCARCContract.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include "ProvenanceAnalysis.h"
#include "llvm/ADT/Statistic.h"
#include "llvm/Analysis/AliasAnalysis.h"
#include "llvm/Analysis/BasicAliasAnalysis.h"
#include "llvm/Analysis/ObjCARCUtil.h"
#include "llvm/IR/Dominators.h"
#include "llvm/IR/EHPersonalities.h"
Expand Down Expand Up @@ -71,6 +72,9 @@ class ObjCARCContract {
ARCRuntimeEntryPoints EP;
BundledRetainClaimRVs *BundledInsts = nullptr;

/// A flag indicating whether this optimization pass should run.
bool Run;

/// The inline asm string to insert between calls and RetainRV calls to make
/// the optimization work on targets which need it.
const MDString *RVInstMarker;
Expand Down Expand Up @@ -527,6 +531,10 @@ bool ObjCARCContract::tryToPeepholeInstruction(
//===----------------------------------------------------------------------===//

bool ObjCARCContract::init(Module &M) {
Run = ModuleHasARC(M);
if (!Run)
return false;

EP.init(&M);

// Initialize RVInstMarker.
Expand All @@ -536,6 +544,9 @@ bool ObjCARCContract::init(Module &M) {
}

bool ObjCARCContract::run(Function &F, AAResults *A, DominatorTree *D) {
if (!Run)
return false;

if (!EnableARCOpts)
return false;

Expand Down Expand Up @@ -730,6 +741,9 @@ INITIALIZE_PASS_END(ObjCARCContractLegacyPass, "objc-arc-contract",
void ObjCARCContractLegacyPass::getAnalysisUsage(AnalysisUsage &AU) const {
AU.addRequired<AAResultsWrapperPass>();
AU.addRequired<DominatorTreeWrapperPass>();
AU.addPreserved<AAResultsWrapperPass>();
AU.addPreserved<BasicAAWrapperPass>();
AU.addPreserved<DominatorTreeWrapperPass>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also add AAResultsWrapper and BasicAAWrapperPass here? I think that should avoid some more pipeline diffs.

}

Pass *llvm::createObjCARCContractPass() {
Expand Down
5 changes: 4 additions & 1 deletion llvm/test/CodeGen/AArch64/O3-pipeline.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,11 +102,14 @@
; CHECK-NEXT: Dominator Tree Construction
; CHECK-NEXT: FunctionPass Manager
; CHECK-NEXT: Merge internal globals
; CHECK-NEXT: Dominator Tree Construction
; CHECK-NEXT: Basic Alias Analysis (stateless AA impl)
; CHECK-NEXT: Function Alias Analysis Results
; CHECK-NEXT: ObjC ARC contraction
; CHECK-NEXT: Prepare callbr
; 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
Expand Down
16 changes: 16 additions & 0 deletions llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,9 @@
; GCN-O1-NEXT: AMDGPU Rewrite Undef for PHI
; GCN-O1-NEXT: LCSSA Verifier
; GCN-O1-NEXT: Loop-Closed SSA Form Pass
; GCN-O1-NEXT: Basic Alias Analysis (stateless AA impl)
; GCN-O1-NEXT: Function Alias Analysis Results
; GCN-O1-NEXT: ObjC ARC contraction
; GCN-O1-NEXT: DummyCGSCCPass
; GCN-O1-NEXT: FunctionPass Manager
; GCN-O1-NEXT: Prepare callbr
Expand Down Expand Up @@ -571,6 +574,9 @@
; GCN-O1-OPTS-NEXT: AMDGPU Rewrite Undef for PHI
; GCN-O1-OPTS-NEXT: LCSSA Verifier
; GCN-O1-OPTS-NEXT: Loop-Closed SSA Form Pass
; GCN-O1-OPTS-NEXT: Basic Alias Analysis (stateless AA impl)
; GCN-O1-OPTS-NEXT: Function Alias Analysis Results
; GCN-O1-OPTS-NEXT: ObjC ARC contraction
; GCN-O1-OPTS-NEXT: DummyCGSCCPass
; GCN-O1-OPTS-NEXT: FunctionPass Manager
; GCN-O1-OPTS-NEXT: Prepare callbr
Expand Down Expand Up @@ -876,6 +882,11 @@
; GCN-O2-NEXT: LCSSA Verifier
; GCN-O2-NEXT: Loop-Closed SSA Form Pass
; GCN-O2-NEXT: Analysis if a function is memory bound
; 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
; GCN-O2-NEXT: ObjC ARC contraction
; GCN-O2-NEXT: DummyCGSCCPass
; GCN-O2-NEXT: FunctionPass Manager
; GCN-O2-NEXT: Prepare callbr
Expand Down Expand Up @@ -1194,6 +1205,11 @@
; GCN-O3-NEXT: LCSSA Verifier
; GCN-O3-NEXT: Loop-Closed SSA Form Pass
; GCN-O3-NEXT: Analysis if a function is memory bound
; 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
; GCN-O3-NEXT: ObjC ARC contraction
; GCN-O3-NEXT: DummyCGSCCPass
; GCN-O3-NEXT: FunctionPass Manager
; GCN-O3-NEXT: Prepare callbr
Expand Down
5 changes: 4 additions & 1 deletion llvm/test/CodeGen/ARM/O3-pipeline.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,14 @@
; CHECK-NEXT: Transform predicated vector loops to use MVE tail predication
; CHECK-NEXT: A No-Op Barrier Pass
; CHECK-NEXT: FunctionPass Manager
; CHECK-NEXT: Dominator Tree Construction
; CHECK-NEXT: Basic Alias Analysis (stateless AA impl)
; CHECK-NEXT: Function Alias Analysis Results
; CHECK-NEXT: ObjC ARC contraction
; CHECK-NEXT: Prepare callbr
; 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
Expand Down
3 changes: 3 additions & 0 deletions llvm/test/CodeGen/LoongArch/opt-pipeline.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@
; LAXX-NEXT: CodeGen Prepare
; LAXX-NEXT: Dominator Tree Construction
; LAXX-NEXT: Exception handling preparation
; LAXX-NEXT: Basic Alias Analysis (stateless AA impl)
; LAXX-NEXT: Function Alias Analysis Results
; LAXX-NEXT: ObjC ARC contraction
; LAXX-NEXT: Prepare callbr
; LAXX-NEXT: Safe Stack instrumentation pass
; LAXX-NEXT: Insert stack protectors
Expand Down
3 changes: 3 additions & 0 deletions llvm/test/CodeGen/M68k/pipeline.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@
; CHECK-NEXT: CodeGen Prepare
; CHECK-NEXT: Dominator Tree Construction
; CHECK-NEXT: Exception handling preparation
; CHECK-NEXT: Basic Alias Analysis (stateless AA impl)
; CHECK-NEXT: Function Alias Analysis Results
; CHECK-NEXT: ObjC ARC contraction
; CHECK-NEXT: Prepare callbr
; CHECK-NEXT: Safe Stack instrumentation pass
; CHECK-NEXT: Insert stack protectors
Expand Down
3 changes: 3 additions & 0 deletions llvm/test/CodeGen/PowerPC/O3-pipeline.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,9 @@
; CHECK-NEXT: Lazy Block Frequency Analysis
; CHECK-NEXT: Optimization Remark Emitter
; CHECK-NEXT: Hardware Loop Insertion
; CHECK-NEXT: Basic Alias Analysis (stateless AA impl)
; CHECK-NEXT: Function Alias Analysis Results
; CHECK-NEXT: ObjC ARC contraction
; CHECK-NEXT: Prepare callbr
; CHECK-NEXT: Safe Stack instrumentation pass
; CHECK-NEXT: Insert stack protectors
Expand Down
7 changes: 5 additions & 2 deletions llvm/test/CodeGen/RISCV/O3-pipeline.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,14 @@
; CHECK-NEXT: Exception handling preparation
; CHECK-NEXT: A No-Op Barrier Pass
; CHECK-NEXT: FunctionPass Manager
; CHECK-NEXT: Dominator Tree Construction
; CHECK-NEXT: Basic Alias Analysis (stateless AA impl)
; CHECK-NEXT: Function Alias Analysis Results
; CHECK-NEXT: ObjC ARC contraction
; CHECK-NEXT: Prepare callbr
; 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
Expand Down Expand Up @@ -193,7 +196,7 @@
; CHECK-NEXT: Machine Optimization Remark Emitter
; CHECK-NEXT: Stack Frame Layout Analysis
; CHECK-NEXT: RISC-V Zcmp move merging pass
; CHECK-NEXT: RISC-V Zcmp Push/Pop optimization pass
; CHECK-NEXT: RISC-V Zcmp Push/Pop optimization pass
; CHECK-NEXT: RISC-V Indirect Branch Tracking
; CHECK-NEXT: RISC-V pseudo instruction expansion pass
; CHECK-NEXT: RISC-V atomic pseudo instruction expansion pass
Expand Down
3 changes: 3 additions & 0 deletions llvm/test/CodeGen/X86/opt-pipeline.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@
; CHECK-NEXT: CodeGen Prepare
; CHECK-NEXT: Dominator Tree Construction
; CHECK-NEXT: Exception handling preparation
; CHECK-NEXT: Basic Alias Analysis (stateless AA impl)
; CHECK-NEXT: Function Alias Analysis Results
; CHECK-NEXT: ObjC ARC contraction
; CHECK-NEXT: Prepare callbr
; CHECK-NEXT: Safe Stack instrumentation pass
; CHECK-NEXT: Insert stack protectors
Expand Down
Loading