Skip to content

Commit 374cbfd

Browse files
authored
[licm] clone MD_prof when hoisting conditional branch (#152232)
The profiling - related metadata information for the hoisted conditional branch should be copied from the original branch, not from the current terminator of the block it's hoisted to. The patch adds a way to disable the fix just so we can do an ablation test, after which the flag will be removed. The same flag will be reused for other similar fixes. (This was identified through `profcheck` (see Issue #147390), and this PR addresses most of the test failures (when running under profcheck) under `Transforms/LICM`.)
1 parent e860896 commit 374cbfd

File tree

3 files changed

+93
-3
lines changed

3 files changed

+93
-3
lines changed

llvm/lib/IR/Instruction.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,18 @@
2626
#include "llvm/IR/Operator.h"
2727
#include "llvm/IR/ProfDataUtils.h"
2828
#include "llvm/IR/Type.h"
29+
#include "llvm/Support/CommandLine.h"
2930
#include "llvm/Support/Compiler.h"
3031
using namespace llvm;
3132

33+
// FIXME: Flag used for an ablation performance test, Issue #147390. Placing it
34+
// here because referencing IR should be feasible from anywhere. Will be
35+
// removed after the ablation test.
36+
cl::opt<bool> ProfcheckDisableMetadataFixes(
37+
"profcheck-disable-metadata-fixes", cl::Hidden, cl::init(false),
38+
cl::desc(
39+
"Disable metadata propagation fixes discovered through Issue #147390"));
40+
3241
InsertPosition::InsertPosition(Instruction *InsertBefore)
3342
: InsertAt(InsertBefore ? InsertBefore->getIterator()
3443
: InstListType::iterator()) {}

llvm/lib/Transforms/Scalar/LICM.cpp

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,8 @@ cl::opt<unsigned> llvm::SetLicmMssaNoAccForPromotionCap(
169169
"number of accesses allowed to be present in a loop in order to "
170170
"enable memory promotion."));
171171

172+
extern cl::opt<bool> ProfcheckDisableMetadataFixes;
173+
172174
static bool inSubLoop(BasicBlock *BB, Loop *CurLoop, LoopInfo *LI);
173175
static bool isNotUsedOrFoldableInLoop(const Instruction &I, const Loop *CurLoop,
174176
const LoopSafetyInfo *SafetyInfo,
@@ -857,9 +859,18 @@ class ControlFlowHoister {
857859
}
858860

859861
// Now finally clone BI.
860-
ReplaceInstWithInst(
861-
HoistTarget->getTerminator(),
862-
BranchInst::Create(HoistTrueDest, HoistFalseDest, BI->getCondition()));
862+
auto *NewBI =
863+
BranchInst::Create(HoistTrueDest, HoistFalseDest, BI->getCondition(),
864+
HoistTarget->getTerminator()->getIterator());
865+
HoistTarget->getTerminator()->eraseFromParent();
866+
// md_prof should also come from the original branch - since the
867+
// condition was hoisted, the branch probabilities shouldn't change.
868+
if (!ProfcheckDisableMetadataFixes)
869+
NewBI->copyMetadata(*BI, {LLVMContext::MD_prof});
870+
// FIXME: Issue #152767: debug info should also be the same as the
871+
// original branch, **if** the user explicitly indicated that.
872+
NewBI->setDebugLoc(HoistTarget->getTerminator()->getDebugLoc());
873+
863874
++NumClonedBranches;
864875

865876
assert(CurLoop->getLoopPreheader() &&
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-globals --version 2
2+
; Test that hoisting conditional branches copies the debug and profiling info
3+
; metadata from the branch being hoisted.
4+
; RUN: opt -S -passes=licm -licm-control-flow-hoisting=1 %s -o - | FileCheck %s
5+
6+
define void @triangle_phi(i32 %x, ptr %p) {
7+
; CHECK-LABEL: define void @triangle_phi
8+
; CHECK-SAME: (i32 [[X:%.*]], ptr [[P:%.*]]) {
9+
; CHECK-NEXT: entry:
10+
; CHECK-NEXT: [[CMP1:%.*]] = icmp sgt i32 [[X]], 0
11+
; CHECK-NEXT: br i1 [[CMP1]], label [[IF_LICM:%.*]], label [[THEN_LICM:%.*]], !prof [[PROF2:![0-9]+]]
12+
; CHECK: if.licm:
13+
; CHECK-NEXT: [[ADD:%.*]] = add i32 [[X]], 1
14+
; CHECK-NEXT: br label [[THEN_LICM]]
15+
; CHECK: then.licm:
16+
; CHECK-NEXT: [[PHI:%.*]] = phi i32 [ [[ADD]], [[IF_LICM]] ], [ [[X]], [[ENTRY:%.*]] ]
17+
; CHECK-NEXT: store i32 [[PHI]], ptr [[P]], align 4
18+
; CHECK-NEXT: [[CMP2:%.*]] = icmp ne i32 [[PHI]], 0
19+
; CHECK-NEXT: br label [[LOOP:%.*]]
20+
; CHECK: loop:
21+
; CHECK-NEXT: br i1 [[CMP1]], label [[IF:%.*]], label [[THEN:%.*]], !dbg [[DBG3:![0-9]+]], !prof [[PROF2]]
22+
; CHECK: if:
23+
; CHECK-NEXT: br label [[THEN]]
24+
; CHECK: then:
25+
; CHECK-NEXT: br i1 [[CMP2]], label [[LOOP]], label [[END:%.*]], !dbg [[DBG7:![0-9]+]], !prof [[PROF8:![0-9]+]]
26+
; CHECK: end:
27+
; CHECK-NEXT: ret void
28+
;
29+
entry:
30+
br label %loop, !dbg !5
31+
loop:
32+
%cmp1 = icmp sgt i32 %x, 0
33+
br i1 %cmp1, label %if, label %then, !dbg !6, !prof !8
34+
if:
35+
%add = add i32 %x, 1
36+
br label %then
37+
38+
then:
39+
%phi = phi i32 [ %add, %if ], [ %x, %loop ]
40+
store i32 %phi, ptr %p
41+
%cmp2 = icmp ne i32 %phi, 0
42+
br i1 %cmp2, label %loop, label %end, !dbg !7, !prof !9
43+
44+
end:
45+
ret void
46+
}
47+
48+
!llvm.module.flags = !{!2, !3}
49+
50+
!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1)
51+
!1 = !DIFile(filename: "t", directory: "/")
52+
!2 = !{i32 7, !"Dwarf Version", i32 5}
53+
!3 = !{i32 2, !"Debug Info Version", i32 3}
54+
!4 = distinct !DISubprogram(name: "triangle_phi", linkageName: "triangle_phi", scope: !1, file: !1, line: 1, unit: !0)
55+
!5 = !DILocation(line: 1, column: 22, scope: !4)
56+
!6 = !DILocation(line: 2, column: 22, scope: !4)
57+
!7 = !DILocation(line: 3, column: 22, scope: !4)
58+
!8 = !{!"branch_weights", i32 5, i32 7}
59+
!9 = !{!"branch_weights", i32 13, i32 11}
60+
;.
61+
; CHECK: [[META0:![0-9]+]] = !{i32 7, !"Dwarf Version", i32 5}
62+
; CHECK: [[META1:![0-9]+]] = !{i32 2, !"Debug Info Version", i32 3}
63+
; CHECK: [[PROF2]] = !{!"branch_weights", i32 5, i32 7}
64+
; CHECK: [[DBG3]] = !DILocation(line: 2, column: 22, scope: [[META4:![0-9]+]])
65+
; CHECK: [[META4]] = distinct !DISubprogram(name: "triangle_phi", linkageName: "triangle_phi", scope: [[META5:![0-9]+]], file: [[META5]], line: 1, spFlags: DISPFlagDefinition, unit: [[META6:![0-9]+]])
66+
; CHECK: [[META5]] = !DIFile(filename: "{{.*}}t", directory: {{.*}})
67+
; CHECK: [[META6]] = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: [[META5]], isOptimized: false, runtimeVersion: 0, emissionKind: NoDebug)
68+
; CHECK: [[DBG7]] = !DILocation(line: 3, column: 22, scope: [[META4]])
69+
; CHECK: [[PROF8]] = !{!"branch_weights", i32 13, i32 11}
70+
;.

0 commit comments

Comments
 (0)