[llvm] [IR][FunctionAttrs] Clarify memory effects of atomics (PR #193768)
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Thu Apr 23 07:56:43 PDT 2026
https://github.com/nikic created https://github.com/llvm/llvm-project/pull/193768
FunctionAttrs was treating atomic instructions, including with ordering strong than monotonic, as only reading/writing their operand.
I don't think doing this is correct, because we model the ordering constraints of synchronizing atomics via reading/writing "all" memory. So e.g. if you have a function with a release store on an argument, marking it as argmem-only is wrong, because that would permit reordering accesses to other locations around it. (What this PR is doing is not *sufficient* to model this correctly due to the fence-like effects on not-yet-escaped memory, but it brings us closer to correctness.)
I initially tried to implement mayReadFromMemory() and mayWriteToMemory() on top of getMemoryEffects(), but this caused significant compile-time regressions, so I've kept the logic duplicated.
(I'm not touching Attributor, which needs some major rework to properly infer memory effects.)
>From e4b442aad9a1400ea74665967eee9a7b75cd8317 Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Thu, 23 Apr 2026 14:49:52 +0200
Subject: [PATCH] [IR][FunctionAttrs] Clarify memory effects of atomics
FunctionAttrs was treating atomic instructions, including with
ordering strong than monotonic, as only reading/writing their
operand.
I don't think doing this is correct, because we model the ordering
constraints of synchronizing atomics via reading/writing "all"
memory. So e.g. if you have a function with a release store on
an argument, marking it as argmem-only is wrong, because that
would permit reordering accesses to other locations around it.
(What this PR is doing is not *sufficient* to model this correctly
due to the fence-like effects on not-yet-escaped memory, but it
brings us closer to correctness.)
I initially tried to implement mayReadFromMemory() and
mayWriteToMemory() on top of getMemoryEffects(), but this caused
significant compile-time regressions, so I've kept the logic
duplicated.
(I'm not touching Attributor, which needs some major rework to
properly deduce memory effects.)
---
llvm/include/llvm/IR/Instruction.h | 5 +++
llvm/lib/IR/Instruction.cpp | 45 +++++++++++++++++++
llvm/lib/Transforms/IPO/FunctionAttrs.cpp | 40 ++++++++---------
llvm/test/Transforms/FunctionAttrs/atomic.ll | 8 ++--
.../Transforms/FunctionAttrs/nocapture.ll | 6 +--
.../Transforms/FunctionAttrs/writeonly.ll | 2 +-
6 files changed, 76 insertions(+), 30 deletions(-)
diff --git a/llvm/include/llvm/IR/Instruction.h b/llvm/include/llvm/IR/Instruction.h
index 0b57ad4d0a379..4d7d1abb941f3 100644
--- a/llvm/include/llvm/IR/Instruction.h
+++ b/llvm/include/llvm/IR/Instruction.h
@@ -24,6 +24,7 @@
#include "llvm/IR/Value.h"
#include "llvm/Support/AtomicOrdering.h"
#include "llvm/Support/Compiler.h"
+#include "llvm/Support/ModRef.h"
#include <cstdint>
#include <utility>
@@ -836,6 +837,10 @@ class Instruction : public User,
return Opcode == Xor;
}
+ /// Return memory effects of the instruction. argmem here refers to the
+ /// operands of the instruction.
+ LLVM_ABI MemoryEffects getMemoryEffects() const LLVM_READONLY;
+
/// Return true if this instruction may modify memory.
LLVM_ABI bool mayWriteToMemory() const LLVM_READONLY;
diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp
index 1932ff72b0a35..c847892189206 100644
--- a/llvm/lib/IR/Instruction.cpp
+++ b/llvm/lib/IR/Instruction.cpp
@@ -1052,6 +1052,51 @@ bool Instruction::isUsedOutsideOfBlock(const BasicBlock *BB) const {
return false;
}
+MemoryEffects Instruction::getMemoryEffects() const {
+ switch (getOpcode()) {
+ default:
+ return MemoryEffects::none();
+ case Instruction::VAArg:
+ return MemoryEffects::argMemOnly();
+ case Instruction::CatchPad:
+ case Instruction::CatchRet:
+ case Instruction::Fence:
+ case Instruction::AtomicCmpXchg:
+ case Instruction::AtomicRMW:
+ return MemoryEffects::unknown();
+ case Instruction::Call:
+ case Instruction::Invoke:
+ case Instruction::CallBr:
+ return cast<CallBase>(this)->getMemoryEffects();
+ case Instruction::Load: {
+ auto *LI = cast<LoadInst>(this);
+ if (isStrongerThanMonotonic(LI->getOrdering()))
+ return MemoryEffects::unknown();
+
+ MemoryEffects ME = MemoryEffects::argMemOnly(
+ LI->isUnordered() ? ModRefInfo::Ref : ModRefInfo::ModRef);
+ if (LI->isVolatile())
+ ME |= MemoryEffects::inaccessibleMemOnly();
+ return ME;
+ }
+ case Instruction::Store: {
+ auto *SI = cast<StoreInst>(this);
+ if (isStrongerThanMonotonic(SI->getOrdering()))
+ return MemoryEffects::unknown();
+
+ MemoryEffects ME = MemoryEffects::argMemOnly(
+ SI->isUnordered() ? ModRefInfo::Mod : ModRefInfo::ModRef);
+ if (SI->isVolatile())
+ ME |= MemoryEffects::inaccessibleMemOnly();
+ return ME;
+ }
+ }
+}
+
+// This is duplicating the logic from getMemoryEffects() for performance
+// reasons. Computing the full MemoryEffects just to perform a Mod/Ref check
+// is expensive.
+
bool Instruction::mayReadFromMemory() const {
switch (getOpcode()) {
default: return false;
diff --git a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp b/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
index 49e40db5f40b0..b4091a6cb2985 100644
--- a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
@@ -186,6 +186,18 @@ checkFunctionMemoryAccess(Function &F, bool ThisBody, AAResults &AAR,
// Additional locations accessed if the SCC accesses argmem.
MemoryEffects RecursiveArgME = MemoryEffects::none();
+ auto AddNonArgMemoryEffects = [&ME](MemoryEffects InstME) {
+ // Merge instruction memory effects, including inaccessible and errno
+ // memory, but excluding argument memory, which is handled separately.
+ ME |= InstME.getWithoutLoc(IRMemLocation::ArgMem);
+
+ // If the instruction accesses captured memory (currently part of "other")
+ // and an argument is captured (currently not tracked), then it may also
+ // access argument memory.
+ ModRefInfo OtherMR = InstME.getModRef(IRMemLocation::Other);
+ ME |= MemoryEffects::argMemOnly(OtherMR);
+ };
+
// Inalloca and preallocated arguments are always clobbered by the call.
if (F.getAttributes().hasAttrSomewhere(Attribute::InAlloca) ||
F.getAttributes().hasAttrSomewhere(Attribute::Preallocated))
@@ -222,16 +234,7 @@ checkFunctionMemoryAccess(Function &F, bool ThisBody, AAResults &AAR,
if (isa<PseudoProbeInst>(I))
continue;
- // Merge callee's memory effects into caller's ones, including
- // inaccessible and errno memory, but excluding argument memory, which is
- // handled separately.
- ME |= CallME.getWithoutLoc(IRMemLocation::ArgMem);
-
- // If the call accesses captured memory (currently part of "other") and
- // an argument is captured (currently not tracked), then it may also
- // access argument memory.
- ModRefInfo OtherMR = CallME.getModRef(IRMemLocation::Other);
- ME |= MemoryEffects::argMemOnly(OtherMR);
+ AddNonArgMemoryEffects(CallME);
// Check whether all pointer arguments point to local memory, and
// ignore calls that only access local memory.
@@ -241,27 +244,20 @@ checkFunctionMemoryAccess(Function &F, bool ThisBody, AAResults &AAR,
continue;
}
- ModRefInfo MR = ModRefInfo::NoModRef;
- if (I.mayWriteToMemory())
- MR |= ModRefInfo::Mod;
- if (I.mayReadFromMemory())
- MR |= ModRefInfo::Ref;
- if (MR == ModRefInfo::NoModRef)
+ MemoryEffects InstME = I.getMemoryEffects();
+ if (InstME.doesNotAccessMemory())
continue;
std::optional<MemoryLocation> Loc = MemoryLocation::getOrNone(&I);
if (!Loc) {
// If no location is known, conservatively assume anything can be
// accessed.
- ME |= MemoryEffects(MR);
+ ME |= MemoryEffects(InstME.getModRef());
continue;
}
- // Volatile operations may access inaccessible memory.
- if (I.isVolatile())
- ME |= MemoryEffects::inaccessibleMemOnly(MR);
-
- addLocAccess(ME, *Loc, MR, AAR);
+ AddNonArgMemoryEffects(InstME);
+ addLocAccess(ME, *Loc, InstME.getModRef(IRMemLocation::ArgMem), AAR);
}
return {OrigME & ME, RecursiveArgME};
diff --git a/llvm/test/Transforms/FunctionAttrs/atomic.ll b/llvm/test/Transforms/FunctionAttrs/atomic.ll
index 8635f2bbdc498..140542e986bbf 100644
--- a/llvm/test/Transforms/FunctionAttrs/atomic.ll
+++ b/llvm/test/Transforms/FunctionAttrs/atomic.ll
@@ -1,10 +1,10 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-attributes
; RUN: opt -passes=function-attrs -S < %s | FileCheck %s
-; Atomic load/store to local doesn't affect whether a function is
-; readnone/readonly.
+; Even though the load/store is on alloca, we can't mark the function as
+; readnone due to the synchronization effect.
define i32 @test1(i32 %x) uwtable ssp {
-; CHECK: Function Attrs: mustprogress nofree norecurse nosync nounwind ssp willreturn memory(none) uwtable
+; CHECK: Function Attrs: mustprogress nofree norecurse nounwind ssp willreturn uwtable
; CHECK-LABEL: @test1(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[X_ADDR:%.*]] = alloca i32, align 4
@@ -21,7 +21,7 @@ entry:
; A function with an Acquire load is not readonly.
define i32 @test2(ptr %x) uwtable ssp {
-; CHECK: Function Attrs: mustprogress nofree norecurse nounwind ssp willreturn memory(argmem: readwrite) uwtable
+; CHECK: Function Attrs: mustprogress nofree norecurse nounwind ssp willreturn uwtable
; CHECK-LABEL: @test2(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[R:%.*]] = load atomic i32, ptr [[X:%.*]] seq_cst, align 4
diff --git a/llvm/test/Transforms/FunctionAttrs/nocapture.ll b/llvm/test/Transforms/FunctionAttrs/nocapture.ll
index b5577ad505ca6..13bdb7eccc0eb 100644
--- a/llvm/test/Transforms/FunctionAttrs/nocapture.ll
+++ b/llvm/test/Transforms/FunctionAttrs/nocapture.ll
@@ -646,7 +646,7 @@ define void @test6_2(ptr %x6_2, ptr %y6_2, ptr %z6_2) {
}
define void @test_cmpxchg(ptr %p) {
-; FNATTRS: Function Attrs: mustprogress nofree norecurse nounwind willreturn memory(argmem: readwrite)
+; FNATTRS: Function Attrs: mustprogress nofree norecurse nounwind willreturn
; FNATTRS-LABEL: define void @test_cmpxchg
; FNATTRS-SAME: (ptr captures(none) [[P:%.*]]) #[[ATTR13:[0-9]+]] {
; FNATTRS-NEXT: [[TMP1:%.*]] = cmpxchg ptr [[P]], i32 0, i32 1 acquire monotonic, align 4
@@ -663,7 +663,7 @@ define void @test_cmpxchg(ptr %p) {
}
define void @test_cmpxchg_ptr(ptr %p, ptr %q) {
-; FNATTRS: Function Attrs: mustprogress nofree norecurse nounwind willreturn memory(argmem: readwrite)
+; FNATTRS: Function Attrs: mustprogress nofree norecurse nounwind willreturn
; FNATTRS-LABEL: define void @test_cmpxchg_ptr
; FNATTRS-SAME: (ptr captures(none) [[P:%.*]], ptr [[Q:%.*]]) #[[ATTR13]] {
; FNATTRS-NEXT: [[TMP1:%.*]] = cmpxchg ptr [[P]], ptr null, ptr [[Q]] acquire monotonic, align 8
@@ -680,7 +680,7 @@ define void @test_cmpxchg_ptr(ptr %p, ptr %q) {
}
define void @test_atomicrmw(ptr %p) {
-; FNATTRS: Function Attrs: mustprogress nofree norecurse nounwind willreturn memory(argmem: readwrite)
+; FNATTRS: Function Attrs: mustprogress nofree norecurse nounwind willreturn
; FNATTRS-LABEL: define void @test_atomicrmw
; FNATTRS-SAME: (ptr captures(none) [[P:%.*]]) #[[ATTR13]] {
; FNATTRS-NEXT: [[TMP1:%.*]] = atomicrmw add ptr [[P]], i32 1 seq_cst, align 4
diff --git a/llvm/test/Transforms/FunctionAttrs/writeonly.ll b/llvm/test/Transforms/FunctionAttrs/writeonly.ll
index 27fd7fca28601..d70a717e9b3e8 100644
--- a/llvm/test/Transforms/FunctionAttrs/writeonly.ll
+++ b/llvm/test/Transforms/FunctionAttrs/writeonly.ll
@@ -162,7 +162,7 @@ define void @test_volatile(ptr %p) {
}
define void @test_atomicrmw(ptr %p) {
-; FNATTRS: Function Attrs: mustprogress nofree norecurse nounwind willreturn memory(argmem: readwrite)
+; FNATTRS: Function Attrs: mustprogress nofree norecurse nounwind willreturn
; FNATTRS-LABEL: define {{[^@]+}}@test_atomicrmw
; FNATTRS-SAME: (ptr captures(none) [[P:%.*]]) #[[ATTR7:[0-9]+]] {
; FNATTRS-NEXT: [[TMP1:%.*]] = atomicrmw add ptr [[P]], i8 0 seq_cst, align 1
More information about the llvm-commits
mailing list