[llvm] [MachineOutliner] Remove LOHs from outlined candidates (PR #143617)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 10 19:49:08 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-backend-aarch64
Author: Ellis Hoag (ellishg)
<details>
<summary>Changes</summary>
Remove Linker Optimization Hints (LOHs) from outlining candidates instead of simply preventing outlining if LOH labels are found in the candidate. This will improve the effectiveness of the machine outliner when LOHs are enabled (which is the default).
In https://discourse.llvm.org/t/loh-conflicting-with-machineoutliner/83279/1 it was observed that the machine outliner is much more effective when LOHs are disabled. Rather than completely disabling LOH, this PR aims to keep LOH in most places and removing them from outlined functions where it could be illegal. Note that we are conservatively removing all LOHs from outlined functions for simplicity, but I believe we could retain LOHs that are in the intersection of all candidates.
It should be ok to remove these LOHs since these blocks are being outlined anyway, which will harm performance much more than the gain from keeping the LOHs.
---
Full diff: https://github.com/llvm/llvm-project/pull/143617.diff
6 Files Affected:
- (modified) llvm/include/llvm/CodeGen/TargetInstrInfo.h (+6)
- (modified) llvm/lib/CodeGen/MachineOutliner.cpp (+18)
- (modified) llvm/lib/Target/AArch64/AArch64InstrInfo.cpp (+9-7)
- (modified) llvm/lib/Target/AArch64/AArch64InstrInfo.h (+1)
- (modified) llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.h (+13)
- (added) llvm/test/CodeGen/AArch64/machine-outliner-loh.ll (+63)
``````````diff
diff --git a/llvm/include/llvm/CodeGen/TargetInstrInfo.h b/llvm/include/llvm/CodeGen/TargetInstrInfo.h
index b5b83c7ff1164..3aec8791b7957 100644
--- a/llvm/include/llvm/CodeGen/TargetInstrInfo.h
+++ b/llvm/include/llvm/CodeGen/TargetInstrInfo.h
@@ -2184,6 +2184,12 @@ class LLVM_ABI TargetInstrInfo : public MCInstrInfo {
MachineBasicBlock::iterator &MIT,
unsigned Flags) const;
+ /// Remove all Linker Optimization Hints associated with instructions in
+ // \p MIs and \return the number of hints removed.
+ virtual size_t clearLOHs(const SmallPtrSetImpl<MachineInstr *> &MIs) const {
+ return 0;
+ }
+
/// Optional target hook that returns true if \p MBB is safe to outline from,
/// and returns any target-specific information in \p Flags.
virtual bool isMBBSafeToOutlineFrom(MachineBasicBlock &MBB,
diff --git a/llvm/lib/CodeGen/MachineOutliner.cpp b/llvm/lib/CodeGen/MachineOutliner.cpp
index e48612369a5db..02a08e4d50818 100644
--- a/llvm/lib/CodeGen/MachineOutliner.cpp
+++ b/llvm/lib/CodeGen/MachineOutliner.cpp
@@ -104,6 +104,7 @@ STATISTIC(StableHashAttempts,
"Count of hashing attempts made for outlined functions");
STATISTIC(StableHashDropped,
"Count of unsuccessful hashing attempts for outlined functions");
+STATISTIC(NumRemovedLOHs, "Total number of Linker Optimization Hints removed");
// Set to true if the user wants the outliner to run on linkonceodr linkage
// functions. This is false by default because the linker can dedupe linkonceodr
@@ -1075,6 +1076,23 @@ bool MachineOutliner::outline(
<< " B) > threshold (" << OutlinerBenefitThreshold
<< " B)\n");
+ // Remove all Linker Optimization Hints from the candidates since we did not
+ // check if the set of hints are the same for each of them.
+ // TODO: The intersection of the LOHs from all candidates should be legal in
+ // the outlined function.
+ SmallPtrSet<MachineInstr *, 2> MIs;
+ std::optional<size_t> MinRemovedLOHs;
+ for (Candidate &C : OF->Candidates) {
+ const TargetInstrInfo &TII = *C.getMF()->getSubtarget().getInstrInfo();
+ for (MachineInstr &MI : C)
+ MIs.insert(&MI);
+ size_t NumRemoved = TII.clearLOHs(MIs);
+ MIs.clear();
+ MinRemovedLOHs =
+ std::min(MinRemovedLOHs.value_or(NumRemoved), NumRemoved);
+ }
+ NumRemovedLOHs += MinRemovedLOHs.value_or(0);
+
// It's beneficial. Create the function and outline its sequence's
// occurrences.
OF->MF = createOutlinedFunction(M, *OF, Mapper, OutlinedFunctionNum);
diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
index 951cb93ea8f8c..ea04bd4cf2428 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
@@ -9641,14 +9641,20 @@ AArch64InstrInfo::getOutlinableRanges(MachineBasicBlock &MBB,
return Ranges;
}
+size_t
+AArch64InstrInfo::clearLOHs(const SmallPtrSetImpl<MachineInstr *> &MIs) const {
+ if (MIs.empty())
+ return 0;
+ auto *MI = *MIs.begin();
+ auto *FuncInfo = MI->getMF()->getInfo<AArch64FunctionInfo>();
+ return FuncInfo->clearLOHs(MIs);
+}
+
outliner::InstrType
AArch64InstrInfo::getOutliningTypeImpl(const MachineModuleInfo &MMI,
MachineBasicBlock::iterator &MIT,
unsigned Flags) const {
MachineInstr &MI = *MIT;
- MachineBasicBlock *MBB = MI.getParent();
- MachineFunction *MF = MBB->getParent();
- AArch64FunctionInfo *FuncInfo = MF->getInfo<AArch64FunctionInfo>();
// Don't outline anything used for return address signing. The outlined
// function will get signed later if needed
@@ -9676,10 +9682,6 @@ AArch64InstrInfo::getOutliningTypeImpl(const MachineModuleInfo &MMI,
return outliner::InstrType::Illegal;
}
- // Don't outline LOHs.
- if (FuncInfo->getLOHRelated().count(&MI))
- return outliner::InstrType::Illegal;
-
// We can only outline these if we will tail call the outlined function, or
// fix up the CFI offsets. Currently, CFI instructions are outlined only if
// in a tail call.
diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.h b/llvm/lib/Target/AArch64/AArch64InstrInfo.h
index 7c255da333e4b..45817e85e3826 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.h
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.h
@@ -493,6 +493,7 @@ class AArch64InstrInfo final : public AArch64GenInstrInfo {
outliner::InstrType getOutliningTypeImpl(const MachineModuleInfo &MMI,
MachineBasicBlock::iterator &MIT,
unsigned Flags) const override;
+ size_t clearLOHs(const SmallPtrSetImpl<MachineInstr *> &MIs) const override;
SmallVector<
std::pair<MachineBasicBlock::iterator, MachineBasicBlock::iterator>>
getOutlinableRanges(MachineBasicBlock &MBB, unsigned &Flags) const override;
diff --git a/llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.h b/llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.h
index 361d5ec3f2b22..0c0217cf4fa3f 100644
--- a/llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.h
+++ b/llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.h
@@ -504,6 +504,19 @@ class AArch64FunctionInfo final : public MachineFunctionInfo {
LOHRelated.insert_range(Args);
}
+ size_t clearLOHs(const SmallPtrSetImpl<MachineInstr *> &MIs) {
+ size_t InitialSize = LOHContainerSet.size();
+ erase_if(LOHContainerSet, [&](const auto &D) {
+ return any_of(D.getArgs(), [&](auto *Arg) { return MIs.contains(Arg); });
+ });
+ // In theory there could be an LOH with one label in MIs and another label
+ // outside MIs, however we don't know if the label outside MIs is used in
+ // any other LOHs, so we can't remove them from LOHRelated. In that case, we
+ // might produce a few extra labels, but it won't break anything.
+ LOHRelated.remove_if([&](auto *MI) { return MIs.contains(MI); });
+ return InitialSize - LOHContainerSet.size();
+ };
+
SmallVectorImpl<ForwardedRegister> &getForwardedMustTailRegParms() {
return ForwardedMustTailRegParms;
}
diff --git a/llvm/test/CodeGen/AArch64/machine-outliner-loh.ll b/llvm/test/CodeGen/AArch64/machine-outliner-loh.ll
new file mode 100644
index 0000000000000..66914d70c23f9
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/machine-outliner-loh.ll
@@ -0,0 +1,63 @@
+; RUN: llc -verify-machineinstrs -mtriple=aarch64-apple-darwin < %s | FileCheck %s --implicit-check-not=.loh --check-prefixes=CHECK,LOH
+; RUN: llc -verify-machineinstrs -mtriple=aarch64-apple-darwin -enable-machine-outliner < %s | FileCheck %s --implicit-check-not=.loh --check-prefixes=CHECK,OUTLINE
+
+ at A = global i32 0, align 4
+ at B = global i32 0, align 4
+
+declare void @foo();
+declare void @bar(ptr %a);
+declare void @goo(ptr %a);
+
+; CHECK-LABEL: _a0:
+define i32 @a0(i32 %a) {
+ %addr = getelementptr inbounds i32, ptr @A, i32 0
+ %res = load i32, ptr %addr, align 4
+ ; LOH: [[L0:Lloh.+]]:
+ ; LOH-NEXT: adrp x19, _A at PAGE
+ ; LOH-NEXT: [[L1:Lloh.+]]:
+ ; LOH-NEXT: add x19, x19, _A at PAGEOFF
+ call void @foo()
+ ; OUTLINE: bl _OUTLINED_FUNCTION_0
+ ; OUTLINE-NEXT: mov x0, x19
+ ; OUTLINE-NEXT: bl _bar
+ call void @bar(ptr %addr)
+ %addr2 = getelementptr inbounds i32, ptr @B, i32 4
+ store i32 0, ptr %addr2, align 4
+ ; CHECK: [[L2:Lloh.+]]:
+ ; CHECK-NEXT: adrp x8, _B at PAGE
+ ; CHECK-NEXT: [[L3:Lloh.+]]:
+ ; CHECK-NEXT: add x8, x8, _B at PAGEOFF
+ ; CHECK-NEXT: mov w0, w20
+ ; CHECK-NEXT: [[L4:Lloh.+]]:
+ ; CHECK-NEXT: str wzr, [x8, #16]
+ ret i32 %res
+ ; LOH-DAG: .loh AdrpAdd [[L0]], [[L1]]
+ ; CHECK-DAG: .loh AdrpAddStr [[L2]], [[L3]], [[L4]]
+ ; CHECK: .cfi_endproc
+}
+
+; CHECK-LABEL: _a1:
+define i32 @a1(i32 %a) {
+ %addr = getelementptr inbounds i32, ptr @A, i32 0
+ %res = load i32, ptr %addr, align 4
+ ; LOH: [[L5:Lloh.+]]:
+ ; LOH-NEXT: adrp x19, _A at PAGE
+ ; LOH-NEXT: [[L6:Lloh.+]]:
+ ; LOH-NEXT: add x19, x19, _A at PAGEOFF
+ call void @foo()
+ ; OUTLINE: bl _OUTLINED_FUNCTION_0
+ ; OUTLINE-NEXT: mov x0, x19
+ ; OUTLINE-NEXT: bl _goo
+ call void @goo(ptr %addr)
+ ret i32 %res
+ ; LOH: .loh AdrpAdd [[L5]], [[L6]]
+ ; CHECK: .cfi_endproc
+}
+
+; Note: it is not safe to add LOHs to this function as outlined functions do not
+; follow calling convention and thus x19 could be live across the call.
+; OUTLINE: _OUTLINED_FUNCTION_0:
+; OUTLINE: adrp x19, _A at PAGE
+; OUTLINE: add x19, x19, _A at PAGEOFF
+; OUTLINE: ldr w20, [x19]
+; OUTLINE: b _foo
``````````
</details>
https://github.com/llvm/llvm-project/pull/143617
More information about the llvm-commits
mailing list