Skip to content

[licm] clone MD_prof when hoisting conditional branch #152232

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 1 commit into from
Aug 13, 2025
+93 −3
Merged

[licm] clone MD_prof when hoisting conditional branch #152232

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 9 additions & 0 deletions llvm/lib/IR/Instruction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,18 @@
#include "llvm/IR/Operator.h"
#include "llvm/IR/ProfDataUtils.h"
#include "llvm/IR/Type.h"
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/Compiler.h"
using namespace llvm;

// FIXME: Flag used for an ablation performance test, Issue #147390. Placing it
// here because referencing IR should be feasible from anywhere. Will be
// removed after the ablation test.
cl::opt<bool> ProfcheckDisableMetadataFixes(
Copy link
Member Author

@mtrofin mtrofin Aug 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

moved it to IR because... well, see the stacked PR (still draft because I am thinking where to actually put the test)

Copy link
Member Author

Choose a reason for hiding this comment

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

This comment doesn't make sense anymore, but I'd leave it in IR to avoid later churn.

"profcheck-disable-metadata-fixes", cl::Hidden, cl::init(false),
cl::desc(
"Disable metadata propagation fixes discovered through Issue #147390"));

InsertPosition::InsertPosition(Instruction *InsertBefore)
: InsertAt(InsertBefore ? InsertBefore->getIterator()
: InstListType::iterator()) {}
Expand Down
17 changes: 14 additions & 3 deletions llvm/lib/Transforms/Scalar/LICM.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,8 @@ cl::opt<unsigned> llvm::SetLicmMssaNoAccForPromotionCap(
"number of accesses allowed to be present in a loop in order to "
"enable memory promotion."));

extern cl::opt<bool> ProfcheckDisableMetadataFixes;

static bool inSubLoop(BasicBlock *BB, Loop *CurLoop, LoopInfo *LI);
static bool isNotUsedOrFoldableInLoop(const Instruction &I, const Loop *CurLoop,
const LoopSafetyInfo *SafetyInfo,
Expand Down Expand Up @@ -857,9 +859,18 @@ class ControlFlowHoister {
}

// Now finally clone BI.
ReplaceInstWithInst(
HoistTarget->getTerminator(),
BranchInst::Create(HoistTrueDest, HoistFalseDest, BI->getCondition()));
auto *NewBI =
BranchInst::Create(HoistTrueDest, HoistFalseDest, BI->getCondition(),
HoistTarget->getTerminator()->getIterator());
HoistTarget->getTerminator()->eraseFromParent();
// md_prof should also come from the original branch - since the
// condition was hoisted, the branch probabilities shouldn't change.
if (!ProfcheckDisableMetadataFixes)
Copy link
Contributor

Choose a reason for hiding this comment

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

Control flow hoisting in LICM is not enabled by default, so I don't think this option is useful.

Copy link
Member Author

Choose a reason for hiding this comment

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

For its temporary purpose, it's easier if I just control all these kinds of changes this way. I'll remove it after (basically I want to lump all fixes I can detect via profcheck, and it's OK if some aren't enabled anyway - just want to approximate the benefit)

NewBI->copyMetadata(*BI, {LLVMContext::MD_prof});
// FIXME: Issue #152767: debug info should also be the same as the
// original branch, **if** the user explicitly indicated that.
NewBI->setDebugLoc(HoistTarget->getTerminator()->getDebugLoc());

++NumClonedBranches;

assert(CurLoop->getLoopPreheader() &&
Expand Down
70 changes: 70 additions & 0 deletions llvm/test/Transforms/LICM/hoist-phi-metadata.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-globals --version 2
; Test that hoisting conditional branches copies the debug and profiling info
; metadata from the branch being hoisted.
; RUN: opt -S -passes=licm -licm-control-flow-hoisting=1 %s -o - | FileCheck %s

define void @triangle_phi(i32 %x, ptr %p) {
; CHECK-LABEL: define void @triangle_phi
; CHECK-SAME: (i32 [[X:%.*]], ptr [[P:%.*]]) {
; CHECK-NEXT: entry:
; CHECK-NEXT: [[CMP1:%.*]] = icmp sgt i32 [[X]], 0
; CHECK-NEXT: br i1 [[CMP1]], label [[IF_LICM:%.*]], label [[THEN_LICM:%.*]], !prof [[PROF2:![0-9]+]]
; CHECK: if.licm:
; CHECK-NEXT: [[ADD:%.*]] = add i32 [[X]], 1
; CHECK-NEXT: br label [[THEN_LICM]]
; CHECK: then.licm:
; CHECK-NEXT: [[PHI:%.*]] = phi i32 [ [[ADD]], [[IF_LICM]] ], [ [[X]], [[ENTRY:%.*]] ]
; CHECK-NEXT: store i32 [[PHI]], ptr [[P]], align 4
; CHECK-NEXT: [[CMP2:%.*]] = icmp ne i32 [[PHI]], 0
; CHECK-NEXT: br label [[LOOP:%.*]]
; CHECK: loop:
; CHECK-NEXT: br i1 [[CMP1]], label [[IF:%.*]], label [[THEN:%.*]], !dbg [[DBG3:![0-9]+]], !prof [[PROF2]]
; CHECK: if:
; CHECK-NEXT: br label [[THEN]]
; CHECK: then:
; CHECK-NEXT: br i1 [[CMP2]], label [[LOOP]], label [[END:%.*]], !dbg [[DBG7:![0-9]+]], !prof [[PROF8:![0-9]+]]
; CHECK: end:
; CHECK-NEXT: ret void
;
entry:
br label %loop, !dbg !5
loop:
%cmp1 = icmp sgt i32 %x, 0
br i1 %cmp1, label %if, label %then, !dbg !6, !prof !8
if:
%add = add i32 %x, 1
br label %then

then:
%phi = phi i32 [ %add, %if ], [ %x, %loop ]
store i32 %phi, ptr %p
%cmp2 = icmp ne i32 %phi, 0
br i1 %cmp2, label %loop, label %end, !dbg !7, !prof !9

end:
ret void
}

!llvm.module.flags = !{!2, !3}

!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1)
!1 = !DIFile(filename: "t", directory: "/")
!2 = !{i32 7, !"Dwarf Version", i32 5}
!3 = !{i32 2, !"Debug Info Version", i32 3}
!4 = distinct !DISubprogram(name: "triangle_phi", linkageName: "triangle_phi", scope: !1, file: !1, line: 1, unit: !0)
!5 = !DILocation(line: 1, column: 22, scope: !4)
!6 = !DILocation(line: 2, column: 22, scope: !4)
!7 = !DILocation(line: 3, column: 22, scope: !4)
!8 = !{!"branch_weights", i32 5, i32 7}
!9 = !{!"branch_weights", i32 13, i32 11}
;.
; CHECK: [[META0:![0-9]+]] = !{i32 7, !"Dwarf Version", i32 5}
; CHECK: [[META1:![0-9]+]] = !{i32 2, !"Debug Info Version", i32 3}
; CHECK: [[PROF2]] = !{!"branch_weights", i32 5, i32 7}
; CHECK: [[DBG3]] = !DILocation(line: 2, column: 22, scope: [[META4:![0-9]+]])
; CHECK: [[META4]] = distinct !DISubprogram(name: "triangle_phi", linkageName: "triangle_phi", scope: [[META5:![0-9]+]], file: [[META5]], line: 1, spFlags: DISPFlagDefinition, unit: [[META6:![0-9]+]])
; CHECK: [[META5]] = !DIFile(filename: "{{.*}}t", directory: {{.*}})
; CHECK: [[META6]] = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: [[META5]], isOptimized: false, runtimeVersion: 0, emissionKind: NoDebug)
; CHECK: [[DBG7]] = !DILocation(line: 3, column: 22, scope: [[META4]])
; CHECK: [[PROF8]] = !{!"branch_weights", i32 13, i32 11}
;.