Skip to content

[profcheck] Add unknown branch weights to expanded cmpxchg loop. #165841

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
Nov 5, 2025
Merged

[profcheck] Add unknown branch weights to expanded cmpxchg loop. #165841

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
8 changes: 7 additions & 1 deletion llvm/lib/CodeGen/AtomicExpandPass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
#include "llvm/IR/MDBuilder.h"
#include "llvm/IR/MemoryModelRelaxationAnnotations.h"
#include "llvm/IR/Module.h"
#include "llvm/IR/ProfDataUtils.h"
#include "llvm/IR/Type.h"
#include "llvm/IR/User.h"
#include "llvm/IR/Value.h"
Expand Down Expand Up @@ -1680,7 +1681,12 @@ Value *AtomicExpandImpl::insertRMWCmpXchgLoop(

Loaded->addIncoming(NewLoaded, LoopBB);

Builder.CreateCondBr(Success, ExitBB, LoopBB);
Instruction *CondBr = Builder.CreateCondBr(Success, ExitBB, LoopBB);

// Atomic RMW expands to a cmpxchg loop, Since precise branch weights
// cannot be easily determined here, we mark the branch as "unknown" (50/50)
// to prevent misleading optimizations.
setExplicitlyUnknownBranchWeightsIfProfiled(*CondBr, *F, DEBUG_TYPE);
Copy link
Member

@tgymnich tgymnich Nov 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
setExplicitlyUnknownBranchWeightsIfProfiled(*CondBr, *F, DEBUG_TYPE);
setExplicitlyUnknownBranchWeightsIfProfiled(*CondBr, DEBUG_TYPE, *F);

The argument order seems wrong. (was changed 2h ago #166032)

Copy link
Member

@tgymnich tgymnich Nov 5, 2025

Choose a reason for hiding this comment

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

#166601

Copy link
Contributor Author

@jinhuang1102 jinhuang1102 Nov 5, 2025

Choose a reason for hiding this comment

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

Thank you Tim!


Builder.SetInsertPoint(ExitBB, ExitBB->begin());
return NewLoaded;
Expand Down
14 changes: 11 additions & 3 deletions llvm/test/Transforms/AtomicExpand/AArch64/atomicrmw-fp.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-globals
; RUN: opt -S -mtriple=aarch64-linux-gnu -passes=atomic-expand %s | FileCheck %s

define float @test_atomicrmw_fadd_f32(ptr %ptr, float %value) {
define float @test_atomicrmw_fadd_f32(ptr %ptr, float %value) !prof !0 {
; CHECK-LABEL: @test_atomicrmw_fadd_f32(
; CHECK-NEXT: [[TMP1:%.*]] = load float, ptr [[PTR:%.*]], align 4
; CHECK-NEXT: br label [[ATOMICRMW_START:%.*]]
Expand All @@ -14,7 +14,7 @@ define float @test_atomicrmw_fadd_f32(ptr %ptr, float %value) {
; CHECK-NEXT: [[SUCCESS:%.*]] = extractvalue { i32, i1 } [[TMP4]], 1
; CHECK-NEXT: [[NEWLOADED:%.*]] = extractvalue { i32, i1 } [[TMP4]], 0
; CHECK-NEXT: [[TMP5]] = bitcast i32 [[NEWLOADED]] to float
; CHECK-NEXT: br i1 [[SUCCESS]], label [[ATOMICRMW_END:%.*]], label [[ATOMICRMW_START]]
; CHECK-NEXT: br i1 [[SUCCESS]], label [[ATOMICRMW_END:%.*]], label [[ATOMICRMW_START]], !prof [[PROF1:![0-9]+]]
; CHECK: atomicrmw.end:
; CHECK-NEXT: ret float [[TMP5]]
;
Expand Down Expand Up @@ -336,3 +336,11 @@ define <2 x half> @atomicrmw_fminimum_2_x_half(ptr %ptr, <2 x half> %val) {
%res = atomicrmw fminimum ptr %ptr, <2 x half> %val seq_cst
ret <2 x half> %res
}

!0 = !{!"function_entry_count", i64 1000}
;.
; CHECK: attributes #[[ATTR0:[0-9]+]] = { nocallback nocreateundeforpoison nofree nosync nounwind speculatable willreturn memory(none) }
Copy link
Contributor Author

@jinhuang1102 jinhuang1102 Nov 5, 2025

Choose a reason for hiding this comment

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

Manually add nocreateundeforpoison to pass the upstream test checks.

Copy link
Contributor

@boomanaiden154 boomanaiden154 Nov 5, 2025

Choose a reason for hiding this comment

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

I'm guessing something else was changed to infer the new attribute and your local opt binary was too old.

;.
; CHECK: [[META0:![0-9]+]] = !{!"function_entry_count", i64 1000}
; CHECK: [[PROF1]] = !{!"unknown", !"atomic-expand"}
;.