Skip to content

[Metadata] Preserve MD_prof when merging instructions when one is missing. #132433

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
Apr 2, 2025
Merged

[Metadata] Preserve MD_prof when merging instructions when one is missing. #132433

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
19 changes: 13 additions & 6 deletions llvm/lib/Transforms/Utils/Local.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3355,9 +3355,10 @@ static void combineMetadata(Instruction *K, const Instruction *J,
case LLVMContext::MD_invariant_group:
// Preserve !invariant.group in K.
break;
// Keep empty cases for mmra, memprof, and callsite to prevent them from
// being removed as unknown metadata. The actual merging is handled
// Keep empty cases for prof, mmra, memprof, and callsite to prevent them
// from being removed as unknown metadata. The actual merging is handled
// separately below.
case LLVMContext::MD_prof:
case LLVMContext::MD_mmra:
case LLVMContext::MD_memprof:
case LLVMContext::MD_callsite:
Expand Down Expand Up @@ -3386,10 +3387,6 @@ static void combineMetadata(Instruction *K, const Instruction *J,
if (!AAOnly)
K->setMetadata(Kind, JMD);
break;
case LLVMContext::MD_prof:
if (!AAOnly && DoesKMove)
K->setMetadata(Kind, MDNode::getMergedProfMetadata(KMD, JMD, K, J));
break;
case LLVMContext::MD_noalias_addrspace:
if (DoesKMove)
K->setMetadata(Kind,
Expand Down Expand Up @@ -3436,6 +3433,16 @@ static void combineMetadata(Instruction *K, const Instruction *J,
K->setMetadata(LLVMContext::MD_callsite,
MDNode::getMergedCallsiteMetadata(KCallSite, JCallSite));
}

// Merge prof metadata.
// Handle separately to support cases where only one instruction has the
// metadata.
auto *JProf = J->getMetadata(LLVMContext::MD_prof);
auto *KProf = K->getMetadata(LLVMContext::MD_prof);
if (!AAOnly && (JProf || KProf)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The old handling was guarded on DoesKMove - what is the implication of removing that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing the condition was intentional. In the case that DoesKMove is true, the merging of md_prof can be skipped if J does not have prof metadata (a minor optimization). I felt that it was simpler to just perform the merge regardless. Let me know if you feel otherwise.

K->setMetadata(LLVMContext::MD_prof,
MDNode::getMergedProfMetadata(KProf, JProf, K, J));
}
}

void llvm::combineMetadataForCSE(Instruction *K, const Instruction *J,
Expand Down
62 changes: 62 additions & 0 deletions llvm/test/Transforms/SimplifyCFG/merge-direct-call-branch-weights-preserve-hoist.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-globals --version 2
; RUN: opt < %s -passes='simplifycfg<no-sink-common-insts;hoist-common-insts>' -simplifycfg-require-and-preserve-domtree=1 -S | FileCheck %s --check-prefix=HOIST

; Test case based on C++ code with manualy annotated !prof metadata.
; This is to test that when calls to 'func1' from 'if.then' block
; and 'if.else' block are hoisted, the branch_weights are merged and
; attached to merged call rather than dropped.
;
; int func1(int a, int b) ;
; int func2(int a, int b) ;

; int func(int a, int b, bool c) {
; int sum= 0;
; if(c) {
; sum += func1(a, b);
; } else {
; sum += func1(a, b);
; sum -= func2(a, b);
; }
; return sum;
; }
define i32 @_Z4funciib(i32 %a, i32 %b, i1 %c) {
; HOIST-LABEL: define i32 @_Z4funciib
; HOIST-SAME: (i32 [[A:%.*]], i32 [[B:%.*]], i1 [[C:%.*]]) {
; HOIST-NEXT: entry:
; HOIST-NEXT: [[CALL:%.*]] = tail call i32 @_Z5func1ii(i32 [[A]], i32 [[B]]), !prof [[PROF0:![0-9]+]]
; HOIST-NEXT: br i1 [[C]], label [[IF_END:%.*]], label [[IF_ELSE:%.*]]
; HOIST: if.else:
; HOIST-NEXT: [[CALL3:%.*]] = tail call i32 @_Z5func2ii(i32 [[A]], i32 [[B]])
; HOIST-NEXT: [[SUB:%.*]] = sub i32 [[CALL]], [[CALL3]]
; HOIST-NEXT: br label [[IF_END]]
; HOIST: if.end:
; HOIST-NEXT: [[SUM_0:%.*]] = phi i32 [ [[SUB]], [[IF_ELSE]] ], [ [[CALL]], [[ENTRY:%.*]] ]
; HOIST-NEXT: ret i32 [[SUM_0]]
;
entry:
br i1 %c, label %if.then, label %if.else

if.then: ; preds = %entry
%call = tail call i32 @_Z5func1ii(i32 %a, i32 %b)
br label %if.end

if.else: ; preds = %entry
%call1 = tail call i32 @_Z5func1ii(i32 %a, i32 %b), !prof !0
%call3 = tail call i32 @_Z5func2ii(i32 %a, i32 %b)
%sub = sub i32 %call1, %call3
br label %if.end

if.end: ; preds = %if.else, %if.then
%sum.0 = phi i32 [ %call, %if.then ], [ %sub, %if.else ]
ret i32 %sum.0
}

declare i32 @_Z5func1ii(i32, i32)

declare i32 @_Z5func2ii(i32, i32)

!0 = !{!"branch_weights", i32 10}

;.
; HOIST: [[PROF0]] = !{!"branch_weights", i32 10}
;.
63 changes: 63 additions & 0 deletions llvm/test/Transforms/SimplifyCFG/merge-direct-call-branch-weights-preserve-sink.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-globals --version 2
; RUN: opt < %s -passes='simplifycfg<sink-common-insts;no-hoist-common-insts>' -simplifycfg-require-and-preserve-domtree=1 -S | FileCheck %s --check-prefix=SINK


; Test case based on the following C++ code with manualy annotated !prof metadata.
; This is to test that when calls to 'func1' from 'if.then' and 'if.else' are
; sinked, the branch weights are merged and attached to sinked call.
;
; int func1(int a, int b) ;
; int func2(int a, int b) ;

; int func(int a, int b, bool c) {
; int sum = 0;
; if (c) {
; sum += func1(a,b);
; } else {
; b -= func2(a,b);
; sum += func1(a,b);
; }
; return sum;
; }

define i32 @_Z4funciib(i32 %a, i32 %b, i1 %c) {
; SINK-LABEL: define i32 @_Z4funciib
; SINK-SAME: (i32 [[A:%.*]], i32 [[B:%.*]], i1 [[C:%.*]]) {
; SINK-NEXT: entry:
; SINK-NEXT: br i1 [[C]], label [[IF_END:%.*]], label [[IF_ELSE:%.*]]
; SINK: if.else:
; SINK-NEXT: [[CALL1:%.*]] = tail call i32 @_Z5func2ii(i32 [[A]], i32 [[B]])
; SINK-NEXT: [[SUB:%.*]] = sub i32 [[B]], [[CALL1]]
; SINK-NEXT: br label [[IF_END]]
; SINK: if.end:
; SINK-NEXT: [[SUB_SINK:%.*]] = phi i32 [ [[SUB]], [[IF_ELSE]] ], [ [[B]], [[ENTRY:%.*]] ]
; SINK-NEXT: [[CALL2:%.*]] = tail call i32 @_Z5func1ii(i32 [[A]], i32 [[SUB_SINK]]), !prof [[PROF0:![0-9]+]]
; SINK-NEXT: ret i32 [[CALL2]]
;
entry:
br i1 %c, label %if.then, label %if.else

if.then: ; preds = %entry
%call = tail call i32 @_Z5func1ii(i32 %a, i32 %b), !prof !0
br label %if.end

if.else: ; preds = %entry
%call1 = tail call i32 @_Z5func2ii(i32 %a, i32 %b)
%sub = sub i32 %b, %call1
%call2 = tail call i32 @_Z5func1ii(i32 %a, i32 %sub)
br label %if.end

if.end: ; preds = %if.else, %if.then
%sum.0 = phi i32 [ %call, %if.then ], [ %call2, %if.else ]
ret i32 %sum.0
}

declare i32 @_Z5func1ii(i32, i32)

declare i32 @_Z5func2ii(i32, i32)

!0 = !{!"branch_weights", i32 10}

;.
; SINK: [[PROF0]] = !{!"branch_weights", i32 10}
;.
Loading