[llvm] [X86][AvoidStoreForwardingBlocks] Skip volatile/atomic accesses. (PR #199698)
Justin Lebar via llvm-commits
llvm-commits at lists.llvm.org
Tue May 26 20:07:36 PDT 2026
https://github.com/jlebar updated https://github.com/llvm/llvm-project/pull/199698
>From d2136cf65459c9ca2a69672a6cb60f820316042f Mon Sep 17 00:00:00 2001
From: Justin Lebar <justin.lebar at gmail.com>
Date: Sun, 24 May 2026 21:50:41 -0700
Subject: [PATCH 1/2] [X86][AvoidStoreForwardingBlocks] Skip volatile/atomic
accesses.
The pass splits an XMM/YMM load+store pair into smaller copies when a
preceding narrower store would block store-to-load forwarding into the
load, but it didn't check the MachineMemOperand's isVolatile/isAtomic bits.
This bug was found by a large run of Opus 4.7 looking for bugs in LLVM.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply at anthropic.com>
---
.../X86/X86AvoidStoreForwardingBlocks.cpp | 8 +-
llvm/test/CodeGen/X86/avoid-sfb.ll | 171 ++++++++++++++++++
2 files changed, 178 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/Target/X86/X86AvoidStoreForwardingBlocks.cpp b/llvm/lib/Target/X86/X86AvoidStoreForwardingBlocks.cpp
index f450b04d25892..044a951bc7c3e 100644
--- a/llvm/lib/Target/X86/X86AvoidStoreForwardingBlocks.cpp
+++ b/llvm/lib/Target/X86/X86AvoidStoreForwardingBlocks.cpp
@@ -551,7 +551,13 @@ void X86AvoidSFBImpl::findPotentiallylBlockedCopies(MachineFunction &MF) {
isRelevantAddressingMode(&MI) &&
isRelevantAddressingMode(&StoreMI) &&
MI.hasOneMemOperand() && StoreMI.hasOneMemOperand()) {
- if (!alias(**MI.memoperands_begin(), **StoreMI.memoperands_begin()))
+ // Don't split volatile or atomic accesses.
+ const MachineMemOperand *LMMO = *MI.memoperands_begin();
+ const MachineMemOperand *SMMO = *StoreMI.memoperands_begin();
+ if (LMMO->isVolatile() || LMMO->isAtomic() || SMMO->isVolatile() ||
+ SMMO->isAtomic())
+ continue;
+ if (!alias(*LMMO, *SMMO))
BlockedLoadsStoresPairs.push_back(std::make_pair(&MI, &StoreMI));
}
}
diff --git a/llvm/test/CodeGen/X86/avoid-sfb.ll b/llvm/test/CodeGen/X86/avoid-sfb.ll
index 22b4fddf88e45..6b064ac9bf5b8 100644
--- a/llvm/test/CodeGen/X86/avoid-sfb.ll
+++ b/llvm/test/CodeGen/X86/avoid-sfb.ll
@@ -1096,6 +1096,177 @@ entry:
ret void
}
+; Volatile accesses are observable and must not be split into smaller copies.
+
+define void @test_volatile_sfb(ptr nocapture noalias %s1, ptr nocapture noalias %s2, i32 %x) {
+; SSE-LABEL: test_volatile_sfb:
+; SSE: # %bb.0: # %entry
+; SSE-NEXT: movl %edx, 4(%rdi)
+; SSE-NEXT: movaps (%rdi), %xmm0
+; SSE-NEXT: movaps %xmm0, (%rsi)
+; SSE-NEXT: retq
+;
+; AVX-LABEL: test_volatile_sfb:
+; AVX: # %bb.0: # %entry
+; AVX-NEXT: movl %edx, 4(%rdi)
+; AVX-NEXT: vmovaps (%rdi), %xmm0
+; AVX-NEXT: vmovaps %xmm0, (%rsi)
+; AVX-NEXT: retq
+entry:
+ %b = getelementptr inbounds %struct.S, ptr %s1, i64 0, i32 1
+ store i32 %x, ptr %b, align 4
+ %v = load volatile <2 x i64>, ptr %s1, align 16
+ store volatile <2 x i64> %v, ptr %s2, align 16
+ ret void
+}
+
+define void @test_volatile_load_only(ptr nocapture noalias %s1, ptr nocapture noalias %s2, i32 %x) {
+; SSE-LABEL: test_volatile_load_only:
+; SSE: # %bb.0: # %entry
+; SSE-NEXT: movl %edx, 4(%rdi)
+; SSE-NEXT: movaps (%rdi), %xmm0
+; SSE-NEXT: movups %xmm0, (%rsi)
+; SSE-NEXT: retq
+;
+; AVX-LABEL: test_volatile_load_only:
+; AVX: # %bb.0: # %entry
+; AVX-NEXT: movl %edx, 4(%rdi)
+; AVX-NEXT: vmovaps (%rdi), %xmm0
+; AVX-NEXT: vmovups %xmm0, (%rsi)
+; AVX-NEXT: retq
+entry:
+ %b = getelementptr inbounds %struct.S, ptr %s1, i64 0, i32 1
+ store i32 %x, ptr %b, align 4
+ %v = load volatile <2 x i64>, ptr %s1, align 16
+ store <2 x i64> %v, ptr %s2, align 1
+ ret void
+}
+
+define void @test_volatile_store_only(ptr nocapture noalias %s1, ptr nocapture noalias %s2, i32 %x) {
+; SSE-LABEL: test_volatile_store_only:
+; SSE: # %bb.0: # %entry
+; SSE-NEXT: movl %edx, 4(%rdi)
+; SSE-NEXT: movups (%rdi), %xmm0
+; SSE-NEXT: movaps %xmm0, (%rsi)
+; SSE-NEXT: retq
+;
+; AVX-LABEL: test_volatile_store_only:
+; AVX: # %bb.0: # %entry
+; AVX-NEXT: movl %edx, 4(%rdi)
+; AVX-NEXT: vmovups (%rdi), %xmm0
+; AVX-NEXT: vmovaps %xmm0, (%rsi)
+; AVX-NEXT: retq
+entry:
+ %b = getelementptr inbounds %struct.S, ptr %s1, i64 0, i32 1
+ store i32 %x, ptr %b, align 4
+ %v = load <2 x i64>, ptr %s1, align 1
+ store volatile <2 x i64> %v, ptr %s2, align 16
+ ret void
+}
+
+; Atomic accesses must preserve their hardware atomicity and must not be split
+; into smaller copies. On AVX targets, aligned 128-bit atomic load/store is
+; lowered to a single [V]MOVAPS/[V]MOVDQA; the SFB pass must leave that
+; instruction alone. (On baseline x86-64 without cmpxchg16b, atomic i128
+; accesses are lowered to libcalls and never reach this pass, so the SSE check
+; lines here just document that baseline lowering.)
+
+define void @test_atomic_sfb(ptr nocapture noalias %s1, ptr nocapture noalias %s2, i32 %x) {
+; SSE-LABEL: test_atomic_sfb:
+; SSE: # %bb.0: # %entry
+; SSE-NEXT: pushq %rbx
+; SSE-NEXT: .cfi_def_cfa_offset 16
+; SSE-NEXT: .cfi_offset %rbx, -16
+; SSE-NEXT: movq %rsi, %rbx
+; SSE-NEXT: movl %edx, 4(%rdi)
+; SSE-NEXT: xorl %esi, %esi
+; SSE-NEXT: callq __atomic_load_16 at PLT
+; SSE-NEXT: movq %rbx, %rdi
+; SSE-NEXT: movq %rax, %rsi
+; SSE-NEXT: xorl %ecx, %ecx
+; SSE-NEXT: callq __atomic_store_16 at PLT
+; SSE-NEXT: popq %rbx
+; SSE-NEXT: .cfi_def_cfa_offset 8
+; SSE-NEXT: retq
+;
+; AVX-LABEL: test_atomic_sfb:
+; AVX: # %bb.0: # %entry
+; AVX-NEXT: movl %edx, 4(%rdi)
+; AVX-NEXT: vmovaps (%rdi), %xmm0
+; AVX-NEXT: vmovaps %xmm0, (%rsi)
+; AVX-NEXT: retq
+entry:
+ %b = getelementptr inbounds %struct.S, ptr %s1, i64 0, i32 1
+ store i32 %x, ptr %b, align 4
+ %v = load atomic i128, ptr %s1 unordered, align 16
+ store atomic i128 %v, ptr %s2 unordered, align 16
+ ret void
+}
+
+define void @test_atomic_load_only(ptr nocapture noalias %s1, ptr nocapture noalias %s2, i32 %x) {
+; SSE-LABEL: test_atomic_load_only:
+; SSE: # %bb.0: # %entry
+; SSE-NEXT: pushq %rbx
+; SSE-NEXT: .cfi_def_cfa_offset 16
+; SSE-NEXT: .cfi_offset %rbx, -16
+; SSE-NEXT: movq %rsi, %rbx
+; SSE-NEXT: movl %edx, 4(%rdi)
+; SSE-NEXT: xorl %esi, %esi
+; SSE-NEXT: callq __atomic_load_16 at PLT
+; SSE-NEXT: movq %rdx, %xmm0
+; SSE-NEXT: movq %rax, %xmm1
+; SSE-NEXT: punpcklqdq {{.*#+}} xmm1 = xmm1[0],xmm0[0]
+; SSE-NEXT: movdqu %xmm1, (%rbx)
+; SSE-NEXT: popq %rbx
+; SSE-NEXT: .cfi_def_cfa_offset 8
+; SSE-NEXT: retq
+;
+; AVX-LABEL: test_atomic_load_only:
+; AVX: # %bb.0: # %entry
+; AVX-NEXT: movl %edx, 4(%rdi)
+; AVX-NEXT: vmovaps (%rdi), %xmm0
+; AVX-NEXT: vmovups %xmm0, (%rsi)
+; AVX-NEXT: retq
+entry:
+ %b = getelementptr inbounds %struct.S, ptr %s1, i64 0, i32 1
+ store i32 %x, ptr %b, align 4
+ %v = load atomic i128, ptr %s1 unordered, align 16
+ %vv = bitcast i128 %v to <2 x i64>
+ store <2 x i64> %vv, ptr %s2, align 1
+ ret void
+}
+
+define void @test_atomic_store_only(ptr nocapture noalias %s1, ptr nocapture noalias %s2, i32 %x) {
+; SSE-LABEL: test_atomic_store_only:
+; SSE: # %bb.0: # %entry
+; SSE-NEXT: pushq %rax
+; SSE-NEXT: .cfi_def_cfa_offset 16
+; SSE-NEXT: movq %rsi, %rax
+; SSE-NEXT: movl %edx, 4(%rdi)
+; SSE-NEXT: movq 8(%rdi), %rdx
+; SSE-NEXT: movq (%rdi), %rsi
+; SSE-NEXT: movq %rax, %rdi
+; SSE-NEXT: xorl %ecx, %ecx
+; SSE-NEXT: callq __atomic_store_16 at PLT
+; SSE-NEXT: popq %rax
+; SSE-NEXT: .cfi_def_cfa_offset 8
+; SSE-NEXT: retq
+;
+; AVX-LABEL: test_atomic_store_only:
+; AVX: # %bb.0: # %entry
+; AVX-NEXT: movl %edx, 4(%rdi)
+; AVX-NEXT: vmovups (%rdi), %xmm0
+; AVX-NEXT: vmovaps %xmm0, (%rsi)
+; AVX-NEXT: retq
+entry:
+ %b = getelementptr inbounds %struct.S, ptr %s1, i64 0, i32 1
+ store i32 %x, ptr %b, align 4
+ %v = load <2 x i64>, ptr %s1, align 1
+ %vv = bitcast <2 x i64> %v to i128
+ store atomic i128 %vv, ptr %s2 unordered, align 16
+ ret void
+}
+
; Function Attrs: argmemonly nounwind
declare void @llvm.memcpy.p0.p0.i64(ptr nocapture writeonly, ptr nocapture readonly, i64, i32, i1) #1
>From 424c2179fa16bcc78ebb896fd74c589d89f3d3ec Mon Sep 17 00:00:00 2001
From: Justin Lebar <justin.lebar at gmail.com>
Date: Tue, 26 May 2026 20:07:22 -0700
Subject: [PATCH 2/2] review comments
---
llvm/test/CodeGen/X86/avoid-sfb.ll | 20 ++++++--------------
1 file changed, 6 insertions(+), 14 deletions(-)
diff --git a/llvm/test/CodeGen/X86/avoid-sfb.ll b/llvm/test/CodeGen/X86/avoid-sfb.ll
index 6b064ac9bf5b8..a50d9c9287eca 100644
--- a/llvm/test/CodeGen/X86/avoid-sfb.ll
+++ b/llvm/test/CodeGen/X86/avoid-sfb.ll
@@ -1098,7 +1098,7 @@ entry:
; Volatile accesses are observable and must not be split into smaller copies.
-define void @test_volatile_sfb(ptr nocapture noalias %s1, ptr nocapture noalias %s2, i32 %x) {
+define void @test_volatile_sfb(ptr noalias %s1, ptr noalias %s2, i32 %x) {
; SSE-LABEL: test_volatile_sfb:
; SSE: # %bb.0: # %entry
; SSE-NEXT: movl %edx, 4(%rdi)
@@ -1120,7 +1120,7 @@ entry:
ret void
}
-define void @test_volatile_load_only(ptr nocapture noalias %s1, ptr nocapture noalias %s2, i32 %x) {
+define void @test_volatile_load_only(ptr noalias %s1, ptr noalias %s2, i32 %x) {
; SSE-LABEL: test_volatile_load_only:
; SSE: # %bb.0: # %entry
; SSE-NEXT: movl %edx, 4(%rdi)
@@ -1142,7 +1142,7 @@ entry:
ret void
}
-define void @test_volatile_store_only(ptr nocapture noalias %s1, ptr nocapture noalias %s2, i32 %x) {
+define void @test_volatile_store_only(ptr noalias %s1, ptr noalias %s2, i32 %x) {
; SSE-LABEL: test_volatile_store_only:
; SSE: # %bb.0: # %entry
; SSE-NEXT: movl %edx, 4(%rdi)
@@ -1171,12 +1171,10 @@ entry:
; accesses are lowered to libcalls and never reach this pass, so the SSE check
; lines here just document that baseline lowering.)
-define void @test_atomic_sfb(ptr nocapture noalias %s1, ptr nocapture noalias %s2, i32 %x) {
+define void @test_atomic_sfb(ptr noalias %s1, ptr noalias %s2, i32 %x) nounwind {
; SSE-LABEL: test_atomic_sfb:
; SSE: # %bb.0: # %entry
; SSE-NEXT: pushq %rbx
-; SSE-NEXT: .cfi_def_cfa_offset 16
-; SSE-NEXT: .cfi_offset %rbx, -16
; SSE-NEXT: movq %rsi, %rbx
; SSE-NEXT: movl %edx, 4(%rdi)
; SSE-NEXT: xorl %esi, %esi
@@ -1186,7 +1184,6 @@ define void @test_atomic_sfb(ptr nocapture noalias %s1, ptr nocapture noalias %s
; SSE-NEXT: xorl %ecx, %ecx
; SSE-NEXT: callq __atomic_store_16 at PLT
; SSE-NEXT: popq %rbx
-; SSE-NEXT: .cfi_def_cfa_offset 8
; SSE-NEXT: retq
;
; AVX-LABEL: test_atomic_sfb:
@@ -1203,12 +1200,10 @@ entry:
ret void
}
-define void @test_atomic_load_only(ptr nocapture noalias %s1, ptr nocapture noalias %s2, i32 %x) {
+define void @test_atomic_load_only(ptr noalias %s1, ptr noalias %s2, i32 %x) nounwind {
; SSE-LABEL: test_atomic_load_only:
; SSE: # %bb.0: # %entry
; SSE-NEXT: pushq %rbx
-; SSE-NEXT: .cfi_def_cfa_offset 16
-; SSE-NEXT: .cfi_offset %rbx, -16
; SSE-NEXT: movq %rsi, %rbx
; SSE-NEXT: movl %edx, 4(%rdi)
; SSE-NEXT: xorl %esi, %esi
@@ -1218,7 +1213,6 @@ define void @test_atomic_load_only(ptr nocapture noalias %s1, ptr nocapture noal
; SSE-NEXT: punpcklqdq {{.*#+}} xmm1 = xmm1[0],xmm0[0]
; SSE-NEXT: movdqu %xmm1, (%rbx)
; SSE-NEXT: popq %rbx
-; SSE-NEXT: .cfi_def_cfa_offset 8
; SSE-NEXT: retq
;
; AVX-LABEL: test_atomic_load_only:
@@ -1236,11 +1230,10 @@ entry:
ret void
}
-define void @test_atomic_store_only(ptr nocapture noalias %s1, ptr nocapture noalias %s2, i32 %x) {
+define void @test_atomic_store_only(ptr noalias %s1, ptr noalias %s2, i32 %x) nounwind {
; SSE-LABEL: test_atomic_store_only:
; SSE: # %bb.0: # %entry
; SSE-NEXT: pushq %rax
-; SSE-NEXT: .cfi_def_cfa_offset 16
; SSE-NEXT: movq %rsi, %rax
; SSE-NEXT: movl %edx, 4(%rdi)
; SSE-NEXT: movq 8(%rdi), %rdx
@@ -1249,7 +1242,6 @@ define void @test_atomic_store_only(ptr nocapture noalias %s1, ptr nocapture noa
; SSE-NEXT: xorl %ecx, %ecx
; SSE-NEXT: callq __atomic_store_16 at PLT
; SSE-NEXT: popq %rax
-; SSE-NEXT: .cfi_def_cfa_offset 8
; SSE-NEXT: retq
;
; AVX-LABEL: test_atomic_store_only:
More information about the llvm-commits
mailing list