Skip to content

[SPARC] Weaken emitted barriers for atomic ops #154950

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 11 commits into from
Oct 8, 2025
Merged

[SPARC] Weaken emitted barriers for atomic ops #154950

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
928bf41
[WIP][SPARC] Weaken emitted barriers for atomic ops
koachan Aug 17, 2025
b21acd7
Update tests
koachan Aug 21, 2025
d78147f
Don't emit acq barriers on V8
koachan Aug 22, 2025
cc5db00
Update opt tests
koachan Aug 22, 2025
5e80c1e
Use Pseudo to emit V8 full barrier
koachan Aug 27, 2025
894a6e0
Remove FIXME comment
koachan Aug 27, 2025
5a9fa67
Move V8BAR expansion to expandPostRAPseudo
koachan Aug 28, 2025
2c52cb7
Remove unneeded includes
koachan Sep 2, 2025
4af6985
Also handle CAS instructions
koachan Sep 26, 2025
01b1001
Remove unneeded includes & update tests
koachan Sep 27, 2025
eca8a14
Don't emit BUNDLEs when printing instruction bundles
koachan Oct 7, 2025
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/Target/Sparc/SparcAsmPrinter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,15 @@ void SparcAsmPrinter::lowerToMCInst(const MachineInstr *MI, MCInst &OutMI) {
void SparcAsmPrinter::emitInstruction(const MachineInstr *MI) {
Sparc_MC::verifyInstructionPredicates(MI->getOpcode(),
getSubtargetInfo().getFeatureBits());
if (MI->isBundle()) {
const MachineBasicBlock *MBB = MI->getParent();
MachineBasicBlock::const_instr_iterator I = ++MI->getIterator();
while (I != MBB->instr_end() && I->isInsideBundle()) {
emitInstruction(&*I);
++I;
}
return;
}

switch (MI->getOpcode()) {
default: break;
Expand Down
26 changes: 26 additions & 0 deletions llvm/lib/Target/Sparc/SparcISelLowering.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include "llvm/IR/DerivedTypes.h"
#include "llvm/IR/DiagnosticInfo.h"
#include "llvm/IR/Function.h"
#include "llvm/IR/IRBuilder.h"
#include "llvm/IR/Module.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/KnownBits.h"
Expand Down Expand Up @@ -3562,3 +3563,28 @@ void SparcTargetLowering::AdjustInstrPostInstrSelection(MachineInstr &MI,
if (!Node->hasAnyUseOfValue(0))
MI.getOperand(0).setReg(SP::G0);
}

Instruction *SparcTargetLowering::emitLeadingFence(IRBuilderBase &Builder,
Instruction *Inst,
AtomicOrdering Ord) const {
bool HasStoreSemantics =
isa<AtomicCmpXchgInst, AtomicRMWInst, StoreInst>(Inst);
if (HasStoreSemantics && isReleaseOrStronger(Ord))
return Builder.CreateFence(AtomicOrdering::Release);
return nullptr;
}

Instruction *SparcTargetLowering::emitTrailingFence(IRBuilderBase &Builder,
Instruction *Inst,
AtomicOrdering Ord) const {
// V8 loads already come with implicit acquire barrier so there's no need to
// emit it again.
bool HasLoadSemantics = isa<AtomicCmpXchgInst, AtomicRMWInst, LoadInst>(Inst);
if (Subtarget->isV9() && HasLoadSemantics && isAcquireOrStronger(Ord))
return Builder.CreateFence(AtomicOrdering::Acquire);

// SC plain stores would need a trailing full barrier.
if (isa<StoreInst>(Inst) && Ord == AtomicOrdering::SequentiallyConsistent)
return Builder.CreateFence(Ord);
return nullptr;
}
5 changes: 5 additions & 0 deletions llvm/lib/Target/Sparc/SparcISelLowering.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,11 @@ namespace llvm {
bool isFMAFasterThanFMulAndFAdd(const MachineFunction &MF,
EVT VT) const override;

Instruction *emitLeadingFence(IRBuilderBase &Builder, Instruction *Inst,
AtomicOrdering Ord) const override;
Instruction *emitTrailingFence(IRBuilderBase &Builder, Instruction *Inst,
AtomicOrdering Ord) const override;

bool shouldInsertFencesForAtomic(const Instruction *I) const override {
// FIXME: We insert fences for each atomics and generate
// sub-optimal code for PSO/TSO. (Approximately nobody uses any
Expand Down
16 changes: 16 additions & 0 deletions llvm/lib/Target/Sparc/SparcInstrInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,22 @@ bool SparcInstrInfo::expandPostRAPseudo(MachineInstr &MI) const {
.addImm(Offset);
return true;
}
case SP::V8BAR: {
assert(!Subtarget.isV9() &&
"V8BAR should not be emitted on V9 processors!");

// Emit stbar; ldstub [%sp-1], %g0
// The sequence acts as a full barrier on V8 systems.
MachineBasicBlock &MBB = *MI.getParent();
MachineInstr &InstSTBAR =
*BuildMI(MBB, MI, MI.getDebugLoc(), get(SP::STBAR));
MachineInstr &InstLDSTUB =
*BuildMI(MBB, MI, MI.getDebugLoc(), get(SP::LDSTUBri), SP::G0)
.addReg(SP::O6)
.addImm(-1);
MIBundleBuilder(MBB, InstSTBAR, InstLDSTUB);
Copy link
Contributor

@arsenm arsenm Sep 25, 2025

Choose a reason for hiding this comment

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

Suggested change
MIBundleBuilder(MBB, InstSTBAR, InstLDSTUB);
finalizeBundle(MBB, InstSTBAR, InstLDSTUB);

I'm not sure what MIBundleBuilder is for, but I think you need to use finalizeBundle to get this properly prefixed with a BUNDLE instruction

Copy link
Contributor Author

@koachan koachan Sep 26, 2025

Choose a reason for hiding this comment

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

to get this properly prefixed with a BUNDLE instruction

A question: the BUNDLE instruction isn't supposed to show up in the asm output right? Because after I changed it to follow your suggestion, the test fails like this:

atomics-ordering.ll:118:23: error: SPARC32-LEON4-NEXT: is not on the line after the previous match
; SPARC32-LEON4-NEXT: stbar
                      ^
<stdin>:42:2: note: 'next' match was here
 stbar
 ^
<stdin>:40:15: note: previous match ended here
 st %o1, [%o0]
              ^
<stdin>:41:1: note: non-matching line after previous match is here
 BUNDLE
^

Input file: <stdin>
Check file: /home/k/llvm-debug/src/llvm-project/llvm/test/CodeGen/SPARC/atomics-ordering.ll

-dump-input=help explains the following input dump.

Input was:
<<<<<<
          .
          .
          .
         37: store_sc: ! @store_sc 
         38: ! %bb.0: 
         39:  stbar 
         40:  st %o1, [%o0] 
         41:  BUNDLE 
         42:  stbar 
next:118      !~~~~  error: match on wrong line
         43:  ldstub [%sp+-1], %g0 
         44:  retl 
         45:  nop 
         46: .Lfunc_end3: 
         47:  .size store_sc, .Lfunc_end3-store_sc 
          .
          .
          .
>>>>>>

Copy link
Contributor

@arsenm arsenm Sep 26, 2025

Choose a reason for hiding this comment

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

The mc lowering should be skipping it if it's not really a bundle

Copy link
Contributor Author

@koachan koachan Sep 26, 2025

Choose a reason for hiding this comment

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

Hmm, I dunno what do you mean by MC lowering here, sorry
In my understanding after doing finalizeBundle the stbar; ldstub sequence will be prefixed by a BUNDLE instruction, but then a later pass ought to remove the BUNDLE prefix right?

Copy link
Contributor Author

@koachan koachan Oct 3, 2025

Choose a reason for hiding this comment

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

Ping @arsenm?
Still confused what should I do here for proper bundlemaking...

MBB.erase(MI);
}
}
return false;
}
29 changes: 25 additions & 4 deletions llvm/lib/Target/Sparc/SparcInstrInfo.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -561,6 +561,9 @@ class Pseudo<dag outs, dag ins, string asmstr, list<dag> pattern>
let isPseudo = 1;
}

// Full memory barrier for V8.
def V8BAR : Pseudo<(outs), (ins), "!V8BAR", []>, Requires<[HasNoV9]>;

// GETPCX for PIC
let Defs = [O7] in {
def GETPCX : Pseudo<(outs getPCX:$getpcseq), (ins), "$getpcseq", [] >;
Expand Down Expand Up @@ -1957,12 +1960,30 @@ def : Pat<(i32 (zextloadi1 ADDRri:$src)), (LDUBri ADDRri:$src)>;
def : Pat<(store (i32 0), ADDRrr:$dst), (STrr ADDRrr:$dst, (i32 G0))>;
def : Pat<(store (i32 0), ADDRri:$dst), (STri ADDRri:$dst, (i32 G0))>;

// store bar for all atomic_fence in V8.
let Predicates = [HasNoV9] in
def : Pat<(atomic_fence timm, timm), (STBAR)>;
// All load-type operations in V8 comes with implicit acquire semantics.
let Predicates = [HasNoV9] in {
// Acquire -> nop
def : Pat<(atomic_fence (i32 4), timm), (NOP)>;
// Release / AcqRel -> stbar
def : Pat<(atomic_fence (i32 5), timm), (STBAR)>;
// AcqRel and stronger -> stbar; ldstub [%sp-1], %g0
def : Pat<(atomic_fence timm, timm), (V8BAR)>;
}

let Predicates = [HasV9] in
// We have to handle both 32 and 64-bit cases.
let Predicates = [HasV9] in {
// Acquire -> membar #LoadLoad | #LoadStore
def : Pat<(atomic_fence (i32 4), timm), (MEMBARi 0x5)>;
def : Pat<(atomic_fence (i64 4), timm), (MEMBARi 0x5)>;
// Release -> membar #LoadStore | #StoreStore
def : Pat<(atomic_fence (i32 5), timm), (MEMBARi 0xc)>;
def : Pat<(atomic_fence (i64 5), timm), (MEMBARi 0xc)>;
// AcqRel -> membar #LoadLoad | #LoadStore | #StoreStore
def : Pat<(atomic_fence (i32 6), timm), (MEMBARi 0xd)>;
def : Pat<(atomic_fence (i64 6), timm), (MEMBARi 0xd)>;
// SeqCst -> membar #StoreLoad | #LoadLoad | #LoadStore | #StoreStore
def : Pat<(atomic_fence timm, timm), (MEMBARi 0xf)>;
}

// atomic_load addr -> load addr
def : Pat<(i32 (atomic_load_azext_8 ADDRrr:$src)), (LDUBrr ADDRrr:$src)>;
Expand Down
24 changes: 12 additions & 12 deletions llvm/test/CodeGen/SPARC/atomicrmw-uinc-udec-wrap.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ define i8 @atomicrmw_uinc_wrap_i8(ptr %ptr, i8 %val) {
; CHECK-LABEL: atomicrmw_uinc_wrap_i8:
; CHECK: .cfi_startproc
; CHECK-NEXT: ! %bb.0:
; CHECK-NEXT: membar #LoadLoad | #StoreLoad | #LoadStore | #StoreStore
; CHECK-NEXT: membar #LoadStore | #StoreStore
; CHECK-NEXT: and %o0, -4, %o2
; CHECK-NEXT: mov 3, %o3
; CHECK-NEXT: andn %o3, %o0, %o0
Expand Down Expand Up @@ -36,7 +36,7 @@ define i8 @atomicrmw_uinc_wrap_i8(ptr %ptr, i8 %val) {
; CHECK-NEXT: nop
; CHECK-NEXT: ! %bb.2: ! %atomicrmw.end
; CHECK-NEXT: srl %o4, %o0, %o0
; CHECK-NEXT: membar #LoadLoad | #StoreLoad | #LoadStore | #StoreStore
; CHECK-NEXT: membar #LoadLoad | #LoadStore
; CHECK-NEXT: retl
; CHECK-NEXT: nop
%result = atomicrmw uinc_wrap ptr %ptr, i8 %val seq_cst
Expand All @@ -47,7 +47,7 @@ define i16 @atomicrmw_uinc_wrap_i16(ptr %ptr, i16 %val) {
; CHECK-LABEL: atomicrmw_uinc_wrap_i16:
; CHECK: .cfi_startproc
; CHECK-NEXT: ! %bb.0:
; CHECK-NEXT: membar #LoadLoad | #StoreLoad | #LoadStore | #StoreStore
; CHECK-NEXT: membar #LoadStore | #StoreStore
; CHECK-NEXT: and %o0, -4, %o2
; CHECK-NEXT: and %o0, 3, %o0
; CHECK-NEXT: xor %o0, 2, %o0
Expand Down Expand Up @@ -79,7 +79,7 @@ define i16 @atomicrmw_uinc_wrap_i16(ptr %ptr, i16 %val) {
; CHECK-NEXT: nop
; CHECK-NEXT: ! %bb.2: ! %atomicrmw.end
; CHECK-NEXT: srl %o5, %o0, %o0
; CHECK-NEXT: membar #LoadLoad | #StoreLoad | #LoadStore | #StoreStore
; CHECK-NEXT: membar #LoadLoad | #LoadStore
; CHECK-NEXT: retl
; CHECK-NEXT: nop
%result = atomicrmw uinc_wrap ptr %ptr, i16 %val seq_cst
Expand All @@ -90,7 +90,7 @@ define i32 @atomicrmw_uinc_wrap_i32(ptr %ptr, i32 %val) {
; CHECK-LABEL: atomicrmw_uinc_wrap_i32:
; CHECK: .cfi_startproc
; CHECK-NEXT: ! %bb.0:
; CHECK-NEXT: membar #LoadLoad | #StoreLoad | #LoadStore | #StoreStore
; CHECK-NEXT: membar #LoadStore | #StoreStore
; CHECK-NEXT: ld [%o0], %o2
; CHECK-NEXT: .LBB2_1: ! %atomicrmw.start
; CHECK-NEXT: ! =>This Inner Loop Header: Depth=1
Expand All @@ -106,7 +106,7 @@ define i32 @atomicrmw_uinc_wrap_i32(ptr %ptr, i32 %val) {
; CHECK-NEXT: bne %icc, .LBB2_1
; CHECK-NEXT: nop
; CHECK-NEXT: ! %bb.2: ! %atomicrmw.end
; CHECK-NEXT: membar #LoadLoad | #StoreLoad | #LoadStore | #StoreStore
; CHECK-NEXT: membar #LoadLoad | #LoadStore
; CHECK-NEXT: retl
; CHECK-NEXT: mov %o2, %o0
%result = atomicrmw uinc_wrap ptr %ptr, i32 %val seq_cst
Expand Down Expand Up @@ -160,7 +160,7 @@ define i8 @atomicrmw_udec_wrap_i8(ptr %ptr, i8 %val) {
; CHECK-LABEL: atomicrmw_udec_wrap_i8:
; CHECK: .cfi_startproc
; CHECK-NEXT: ! %bb.0:
; CHECK-NEXT: membar #LoadLoad | #StoreLoad | #LoadStore | #StoreStore
; CHECK-NEXT: membar #LoadStore | #StoreStore
; CHECK-NEXT: and %o0, -4, %o2
; CHECK-NEXT: mov 3, %o3
; CHECK-NEXT: andn %o3, %o0, %o0
Expand Down Expand Up @@ -193,7 +193,7 @@ define i8 @atomicrmw_udec_wrap_i8(ptr %ptr, i8 %val) {
; CHECK-NEXT: nop
; CHECK-NEXT: ! %bb.2: ! %atomicrmw.end
; CHECK-NEXT: srl %o5, %o0, %o0
; CHECK-NEXT: membar #LoadLoad | #StoreLoad | #LoadStore | #StoreStore
; CHECK-NEXT: membar #LoadLoad | #LoadStore
; CHECK-NEXT: retl
; CHECK-NEXT: nop
%result = atomicrmw udec_wrap ptr %ptr, i8 %val seq_cst
Expand All @@ -204,7 +204,7 @@ define i16 @atomicrmw_udec_wrap_i16(ptr %ptr, i16 %val) {
; CHECK-LABEL: atomicrmw_udec_wrap_i16:
; CHECK: .cfi_startproc
; CHECK-NEXT: ! %bb.0:
; CHECK-NEXT: membar #LoadLoad | #StoreLoad | #LoadStore | #StoreStore
; CHECK-NEXT: membar #LoadStore | #StoreStore
; CHECK-NEXT: and %o0, -4, %o2
; CHECK-NEXT: and %o0, 3, %o0
; CHECK-NEXT: xor %o0, 2, %o0
Expand Down Expand Up @@ -238,7 +238,7 @@ define i16 @atomicrmw_udec_wrap_i16(ptr %ptr, i16 %val) {
; CHECK-NEXT: nop
; CHECK-NEXT: ! %bb.2: ! %atomicrmw.end
; CHECK-NEXT: srl %g2, %o0, %o0
; CHECK-NEXT: membar #LoadLoad | #StoreLoad | #LoadStore | #StoreStore
; CHECK-NEXT: membar #LoadLoad | #LoadStore
; CHECK-NEXT: retl
; CHECK-NEXT: nop
%result = atomicrmw udec_wrap ptr %ptr, i16 %val seq_cst
Expand All @@ -249,7 +249,7 @@ define i32 @atomicrmw_udec_wrap_i32(ptr %ptr, i32 %val) {
; CHECK-LABEL: atomicrmw_udec_wrap_i32:
; CHECK: .cfi_startproc
; CHECK-NEXT: ! %bb.0:
; CHECK-NEXT: membar #LoadLoad | #StoreLoad | #LoadStore | #StoreStore
; CHECK-NEXT: membar #LoadStore | #StoreStore
; CHECK-NEXT: ld [%o0], %o2
; CHECK-NEXT: .LBB6_1: ! %atomicrmw.start
; CHECK-NEXT: ! =>This Inner Loop Header: Depth=1
Expand All @@ -267,7 +267,7 @@ define i32 @atomicrmw_udec_wrap_i32(ptr %ptr, i32 %val) {
; CHECK-NEXT: bne %icc, .LBB6_1
; CHECK-NEXT: nop
; CHECK-NEXT: ! %bb.2: ! %atomicrmw.end
; CHECK-NEXT: membar #LoadLoad | #StoreLoad | #LoadStore | #StoreStore
; CHECK-NEXT: membar #LoadLoad | #LoadStore
; CHECK-NEXT: retl
; CHECK-NEXT: mov %o2, %o0
%result = atomicrmw udec_wrap ptr %ptr, i32 %val seq_cst
Expand Down
Loading