[FunctionAttrs][Attributor] Update nofree inference#196266
Merged
Merged
Conversation
llvmorg-github-actions
Bot
added
llvm:transforms
clang:openmp
|
@llvm/pr-subscribers-llvm-transforms Author: Nikita Popov (nikic) ChangesThis updates nofree inference after the semantics changes in #195658. FunctionAttrs currently only supports function-level nofree inference, so we just need to check for potentially synchronizing instructions. Attributor also supports argument nofree, in which case we make additional changes:
Patch is 194.41 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/196266.diff 31 Files Affected:
diff --git a/clang/test/CodeGen/sanitize-metadata-nosanitize.c b/clang/test/CodeGen/sanitize-metadata-nosanitize.c
index 01183709476f9..019853f51e6dd 100644
--- a/clang/test/CodeGen/sanitize-metadata-nosanitize.c
+++ b/clang/test/CodeGen/sanitize-metadata-nosanitize.c
@@ -21,7 +21,7 @@ __attribute__((noinline, not_tail_called)) void escape(const volatile void *p) {
sink = p;
}
-// CHECK: Function Attrs: mustprogress nofree norecurse nounwind willreturn memory(write, argmem: readwrite, inaccessiblemem: none, target_mem: none)
+// CHECK: Function Attrs: mustprogress norecurse nounwind willreturn memory(write, argmem: readwrite, inaccessiblemem: none, target_mem: none)
// CHECK-LABEL: define dso_local i32 @normal_function(
// CHECK-SAME: ptr noundef [[X:%.*]], ptr noundef readonly captures(none) [[Y:%.*]]) local_unnamed_addr #[[ATTR1:[0-9]+]] !pcsections [[META7:![0-9]+]] {
// CHECK-NEXT: [[ENTRY:.*:]]
@@ -38,7 +38,7 @@ int normal_function(int *x, int *y) {
return *y;
}
-// CHECK: Function Attrs: disable_sanitizer_instrumentation mustprogress nofree norecurse nounwind willreturn memory(write, argmem: readwrite, inaccessiblemem: none, target_mem: none)
+// CHECK: Function Attrs: disable_sanitizer_instrumentation mustprogress norecurse nounwind willreturn memory(write, argmem: readwrite, inaccessiblemem: none, target_mem: none)
// CHECK-LABEL: define dso_local i32 @test_disable_sanitize_instrumentation(
// CHECK-SAME: ptr noundef [[X:%.*]], ptr noundef readonly captures(none) [[Y:%.*]]) local_unnamed_addr #[[ATTR2:[0-9]+]] {
// CHECK-NEXT: [[ENTRY:.*:]]
@@ -55,7 +55,7 @@ __attribute__((disable_sanitizer_instrumentation)) int test_disable_sanitize_ins
return *y;
}
-// CHECK: Function Attrs: mustprogress nofree norecurse nounwind willreturn memory(write, argmem: readwrite, inaccessiblemem: none, target_mem: none)
+// CHECK: Function Attrs: mustprogress norecurse nounwind willreturn memory(write, argmem: readwrite, inaccessiblemem: none, target_mem: none)
// CHECK-LABEL: define dso_local i32 @test_no_sanitize_thread(
// CHECK-SAME: ptr noundef [[X:%.*]], ptr noundef readonly captures(none) [[Y:%.*]]) local_unnamed_addr #[[ATTR3:[0-9]+]] !pcsections [[META13:![0-9]+]] {
// CHECK-NEXT: [[ENTRY:.*:]]
@@ -72,7 +72,7 @@ __attribute__((no_sanitize("thread"))) int test_no_sanitize_thread(int *x, int *
return *y;
}
-// CHECK: Function Attrs: mustprogress nofree norecurse nounwind willreturn memory(write, argmem: readwrite, inaccessiblemem: none, target_mem: none)
+// CHECK: Function Attrs: mustprogress norecurse nounwind willreturn memory(write, argmem: readwrite, inaccessiblemem: none, target_mem: none)
// CHECK-LABEL: define dso_local i32 @test_no_sanitize_all(
// CHECK-SAME: ptr noundef [[X:%.*]], ptr noundef readonly captures(none) [[Y:%.*]]) local_unnamed_addr #[[ATTR3]] !pcsections [[META13]] {
// CHECK-NEXT: [[ENTRY:.*:]]
@@ -90,9 +90,9 @@ __attribute__((no_sanitize("all"))) int test_no_sanitize_all(int *x, int *y) {
}
//.
// CHECK: attributes #[[ATTR0]] = { mustprogress nofree noinline norecurse nosync nounwind willreturn memory(write, argmem: none, inaccessiblemem: none, target_mem: none) "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
-// CHECK: attributes #[[ATTR1]] = { mustprogress nofree norecurse nounwind willreturn memory(write, argmem: readwrite, inaccessiblemem: none, target_mem: none) "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
-// CHECK: attributes #[[ATTR2]] = { disable_sanitizer_instrumentation mustprogress nofree norecurse nounwind willreturn memory(write, argmem: readwrite, inaccessiblemem: none, target_mem: none) "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
-// CHECK: attributes #[[ATTR3]] = { mustprogress nofree norecurse nounwind willreturn memory(write, argmem: readwrite, inaccessiblemem: none, target_mem: none) "min-legal-vector-width"="0" "no-trapping-math"="true" "no_sanitize_thread" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
+// CHECK: attributes #[[ATTR1]] = { mustprogress norecurse nounwind willreturn memory(write, argmem: readwrite, inaccessiblemem: none, target_mem: none) "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
+// CHECK: attributes #[[ATTR2]] = { disable_sanitizer_instrumentation mustprogress norecurse nounwind willreturn memory(write, argmem: readwrite, inaccessiblemem: none, target_mem: none) "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
+// CHECK: attributes #[[ATTR3]] = { mustprogress norecurse nounwind willreturn memory(write, argmem: readwrite, inaccessiblemem: none, target_mem: none) "min-legal-vector-width"="0" "no-trapping-math"="true" "no_sanitize_thread" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
// CHECK: attributes #[[ATTR4:[0-9]+]] = { nounwind "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
//.
// CHECK: [[META0:![0-9]+]] = !{!"{{.*}}clang version {{.*}}"}
diff --git a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
index 7c50b9faf3c80..3f33a26751726 100644
--- a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
+++ b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
@@ -2281,6 +2281,15 @@ struct AANoFreeImpl : public AANoFree {
/// See AbstractAttribute::updateImpl(...).
ChangeStatus updateImpl(Attributor &A) override {
+ // Make sure that synchronization cannot establish happens-before with a
+ // free on another thread.
+ const IRPosition &IRP = getIRPosition();
+ bool IsKnown;
+ if (!AA::hasAssumedIRAttr<Attribute::NoSync>(
+ A, this, IRPosition::function_scope(IRP), DepClassTy::REQUIRED,
+ IsKnown))
+ return indicatePessimisticFixpoint();
+
auto CheckForNoFree = [&](Instruction &I) {
bool IsKnown;
return AA::hasAssumedIRAttr<Attribute::NoFree>(
@@ -2292,6 +2301,7 @@ struct AANoFreeImpl : public AANoFree {
if (!A.checkForAllCallLikeInstructions(CheckForNoFree, *this,
UsedAssumedInformation))
return indicatePessimisticFixpoint();
+
return ChangeStatus::UNCHANGED;
}
@@ -2346,27 +2356,41 @@ struct AANoFreeFloating : AANoFreeImpl {
return true;
unsigned ArgNo = CB->getArgOperandNo(&U);
+ // Even if the argument is nofree, we still need to check for nocapture,
+ // as the call may capture the argument without freeing it, and the
+ // captured argument is freed later.
bool IsKnown;
- return AA::hasAssumedIRAttr<Attribute::NoFree>(
- A, this, IRPosition::callsite_argument(*CB, ArgNo),
- DepClassTy::REQUIRED, IsKnown);
+ if (!AA::hasAssumedIRAttr<Attribute::NoFree>(
+ A, this, IRPosition::callsite_argument(*CB, ArgNo),
+ DepClassTy::REQUIRED, IsKnown))
+ return false;
+
+ const AANoCapture *NoCaptureAA = nullptr;
+ if (!AA::hasAssumedIRAttr<Attribute::Captures>(
+ A, this, IRPosition::callsite_argument(*CB, ArgNo),
+ DepClassTy::REQUIRED, IsKnown, false, &NoCaptureAA)) {
+ if (NoCaptureAA && NoCaptureAA->isAssumedNoCaptureMaybeReturned()) {
+ Follow = true;
+ return true;
+ }
+ return false;
+ }
+
+ return true;
}
- if (isa<GetElementPtrInst>(UserI) || isa<PHINode>(UserI) ||
- isa<SelectInst>(UserI)) {
+ UseCaptureInfo CI = DetermineUseCaptureKind(U, /*Base=*/nullptr);
+ if (!capturesAnyProvenance(CI))
+ return true;
+ if (capturesAnyProvenance(CI.ResultCC)) {
Follow = true;
return true;
}
- if (isa<LoadInst>(UserI))
- return true;
-
- if (isa<StoreInst>(UserI))
- return U.getOperandNo() == StoreInst::getPointerOperandIndex();
if (isa<ReturnInst>(UserI) && getIRPosition().isArgumentPosition())
return true;
- // Unknown user.
+ // Capturing user.
return false;
};
if (!A.checkForAllUses(Pred, *this, AssociatedValue))
diff --git a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp b/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
index c6113fbb96b3f..06b1be0ec902d 100644
--- a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
@@ -1919,8 +1919,11 @@ static bool InstrBreaksNonThrowing(Instruction &I, const SCCNodeSet &SCCNodes) {
/// Helper for NoFree inference predicate InstrBreaksAttribute.
static bool InstrBreaksNoFree(Instruction &I, const SCCNodeSet &SCCNodes) {
CallBase *CB = dyn_cast<CallBase>(&I);
- if (!CB)
- return false;
+ if (!CB) {
+ // Synchronization may establish happens-before with a free on another
+ // thread.
+ return I.maySynchronize();
+ }
if (CB->hasFnAttr(Attribute::NoFree))
return false;
diff --git a/llvm/test/Transforms/Attributor/ArgumentPromotion/X86/thiscall.ll b/llvm/test/Transforms/Attributor/ArgumentPromotion/X86/thiscall.ll
index 4795b424b0d2d..139f42c6d36e1 100644
--- a/llvm/test/Transforms/Attributor/ArgumentPromotion/X86/thiscall.ll
+++ b/llvm/test/Transforms/Attributor/ArgumentPromotion/X86/thiscall.ll
@@ -59,8 +59,8 @@ declare ptr @llvm.stacksave()
declare void @llvm.stackrestore(ptr)
;.
; TUNIT: attributes #[[ATTR0:[0-9]+]] = { nocallback nofree nosync nounwind willreturn }
-; TUNIT: attributes #[[ATTR1]] = { nofree willreturn }
+; TUNIT: attributes #[[ATTR1]] = { willreturn }
;.
; CGSCC: attributes #[[ATTR0:[0-9]+]] = { nocallback nofree nosync nounwind willreturn }
-; CGSCC: attributes #[[ATTR1]] = { nofree willreturn }
+; CGSCC: attributes #[[ATTR1]] = { willreturn }
;.
diff --git a/llvm/test/Transforms/Attributor/align-atomic.ll b/llvm/test/Transforms/Attributor/align-atomic.ll
index 0b363741cc168..3c3db2ce27ca4 100644
--- a/llvm/test/Transforms/Attributor/align-atomic.ll
+++ b/llvm/test/Transforms/Attributor/align-atomic.ll
@@ -26,7 +26,7 @@ define i32 @atomicrmw_add_propagate(ptr align 8 %ptr, i32 %val) {
; Should increase alignment to 8, not 16.
define ptr @atomicrmw_non_ptr_op_no_propagate(ptr %ptr, ptr align 16 %val) {
; CHECK-LABEL: define ptr @atomicrmw_non_ptr_op_no_propagate(
-; CHECK-SAME: ptr nofree noundef nonnull align 2 captures(none) dereferenceable(8) [[PTR:%.*]], ptr nofree align 16 [[VAL:%.*]]) #[[ATTR0]] {
+; CHECK-SAME: ptr nofree noundef nonnull align 2 captures(none) dereferenceable(8) [[PTR:%.*]], ptr align 16 [[VAL:%.*]]) #[[ATTR0]] {
; CHECK-NEXT: [[RESULT:%.*]] = atomicrmw xchg ptr [[PTR]], ptr [[VAL]] seq_cst, align 2
; CHECK-NEXT: ret ptr [[RESULT]]
;
@@ -49,7 +49,7 @@ define i32 @cmpxchg_propagate(ptr align 8 %ptr, i32 %cmp, i32 %val) {
; Should not increase alignment
define ptr @cmpxchg_no_propagate(ptr %ptr, ptr align 16 %cmp, ptr align 32 %val) {
; CHECK-LABEL: define ptr @cmpxchg_no_propagate(
-; CHECK-SAME: ptr nofree noundef nonnull align 2 captures(none) dereferenceable(8) [[PTR:%.*]], ptr nofree align 16 [[CMP:%.*]], ptr nofree align 32 [[VAL:%.*]]) #[[ATTR0]] {
+; CHECK-SAME: ptr nofree noundef nonnull align 2 captures(none) dereferenceable(8) [[PTR:%.*]], ptr align 16 [[CMP:%.*]], ptr align 32 [[VAL:%.*]]) #[[ATTR0]] {
; CHECK-NEXT: [[PAIR:%.*]] = cmpxchg ptr [[PTR]], ptr [[CMP]], ptr [[VAL]] seq_cst monotonic, align 2
; CHECK-NEXT: [[RESULT:%.*]] = extractvalue { ptr, i1 } [[PAIR]], 0
; CHECK-NEXT: ret ptr [[RESULT]]
diff --git a/llvm/test/Transforms/Attributor/align.ll b/llvm/test/Transforms/Attributor/align.ll
index 586aecb7cae1f..904233408c144 100644
--- a/llvm/test/Transforms/Attributor/align.ll
+++ b/llvm/test/Transforms/Attributor/align.ll
@@ -1115,7 +1115,7 @@ entry:
}
define i64 @infer_align_atomicrmw(ptr align 4 %p) {
-; TUNIT: Function Attrs: mustprogress nofree norecurse nounwind willreturn memory(argmem: readwrite)
+; TUNIT: Function Attrs: mustprogress norecurse nounwind willreturn memory(argmem: readwrite)
; TUNIT-LABEL: define {{[^@]+}}@infer_align_atomicrmw
; TUNIT-SAME: (ptr nofree align 16 captures(none) [[P:%.*]]) #[[ATTR12:[0-9]+]] {
; TUNIT-NEXT: [[ARRAYIDX0:%.*]] = getelementptr i64, ptr [[P]], i64 1
@@ -1123,7 +1123,7 @@ define i64 @infer_align_atomicrmw(ptr align 4 %p) {
; TUNIT-NEXT: [[RET:%.*]] = atomicrmw add ptr [[ARRAYIDX1]], i64 4 seq_cst, align 16
; TUNIT-NEXT: ret i64 [[RET]]
;
-; CGSCC: Function Attrs: mustprogress nofree norecurse nounwind willreturn memory(argmem: readwrite)
+; CGSCC: Function Attrs: mustprogress norecurse nounwind willreturn memory(argmem: readwrite)
; CGSCC-LABEL: define {{[^@]+}}@infer_align_atomicrmw
; CGSCC-SAME: (ptr nofree align 16 captures(none) [[P:%.*]]) #[[ATTR14:[0-9]+]] {
; CGSCC-NEXT: [[ARRAYIDX0:%.*]] = getelementptr i64, ptr [[P]], i64 1
@@ -1138,17 +1138,17 @@ define i64 @infer_align_atomicrmw(ptr align 4 %p) {
}
define ptr @infer_align_atomicrmw_ptr(ptr align 4 %p, ptr %val) {
-; TUNIT: Function Attrs: mustprogress nofree norecurse nounwind willreturn memory(argmem: readwrite)
+; TUNIT: Function Attrs: mustprogress norecurse nounwind willreturn memory(argmem: readwrite)
; TUNIT-LABEL: define {{[^@]+}}@infer_align_atomicrmw_ptr
-; TUNIT-SAME: (ptr nofree align 16 captures(none) [[P:%.*]], ptr nofree [[VAL:%.*]]) #[[ATTR12]] {
+; TUNIT-SAME: (ptr nofree align 16 captures(none) [[P:%.*]], ptr [[VAL:%.*]]) #[[ATTR12]] {
; TUNIT-NEXT: [[ARRAYIDX0:%.*]] = getelementptr i64, ptr [[P]], i64 1
; TUNIT-NEXT: [[ARRAYIDX1:%.*]] = getelementptr i64, ptr [[ARRAYIDX0]], i64 3
; TUNIT-NEXT: [[RET:%.*]] = atomicrmw xchg ptr [[ARRAYIDX1]], ptr [[VAL]] seq_cst, align 16
; TUNIT-NEXT: ret ptr [[RET]]
;
-; CGSCC: Function Attrs: mustprogress nofree norecurse nounwind willreturn memory(argmem: readwrite)
+; CGSCC: Function Attrs: mustprogress norecurse nounwind willreturn memory(argmem: readwrite)
; CGSCC-LABEL: define {{[^@]+}}@infer_align_atomicrmw_ptr
-; CGSCC-SAME: (ptr nofree align 16 captures(none) [[P:%.*]], ptr nofree [[VAL:%.*]]) #[[ATTR14]] {
+; CGSCC-SAME: (ptr nofree align 16 captures(none) [[P:%.*]], ptr [[VAL:%.*]]) #[[ATTR14]] {
; CGSCC-NEXT: [[ARRAYIDX0:%.*]] = getelementptr i64, ptr [[P]], i64 1
; CGSCC-NEXT: [[ARRAYIDX1:%.*]] = getelementptr i64, ptr [[ARRAYIDX0]], i64 3
; CGSCC-NEXT: [[RET:%.*]] = atomicrmw xchg ptr [[ARRAYIDX1]], ptr [[VAL]] seq_cst, align 16
@@ -1161,7 +1161,7 @@ define ptr @infer_align_atomicrmw_ptr(ptr align 4 %p, ptr %val) {
}
define i64 @infer_align_cmpxchg(ptr align 4 %p) {
-; TUNIT: Function Attrs: mustprogress nofree norecurse nounwind willreturn memory(argmem: readwrite)
+; TUNIT: Function Attrs: mustprogress norecurse nounwind willreturn memory(argmem: readwrite)
; TUNIT-LABEL: define {{[^@]+}}@infer_align_cmpxchg
; TUNIT-SAME: (ptr nofree align 16 captures(none) [[P:%.*]]) #[[ATTR12]] {
; TUNIT-NEXT: [[ARRAYIDX0:%.*]] = getelementptr i64, ptr [[P]], i64 1
@@ -1170,7 +1170,7 @@ define i64 @infer_align_cmpxchg(ptr align 4 %p) {
; TUNIT-NEXT: [[RET:%.*]] = extractvalue { i64, i1 } [[CMPX]], 0
; TUNIT-NEXT: ret i64 [[RET]]
;
-; CGSCC: Function Attrs: mustprogress nofree norecurse nounwind willreturn memory(argmem: readwrite)
+; CGSCC: Function Attrs: mustprogress norecurse nounwind willreturn memory(argmem: readwrite)
; CGSCC-LABEL: define {{[^@]+}}@infer_align_cmpxchg
; CGSCC-SAME: (ptr nofree align 16 captures(none) [[P:%.*]]) #[[ATTR14]] {
; CGSCC-NEXT: [[ARRAYIDX0:%.*]] = getelementptr i64, ptr [[P]], i64 1
@@ -1187,18 +1187,18 @@ define i64 @infer_align_cmpxchg(ptr align 4 %p) {
}
define ptr @infer_align_cmpxchg_ptr(ptr align 4 %p, ptr %cmp0, ptr %cmp1) {
-; TUNIT: Function Attrs: mustprogress nofree norecurse nounwind willreturn memory(argmem: readwrite)
+; TUNIT: Function Attrs: mustprogress norecurse nounwind willreturn memory(argmem: readwrite)
; TUNIT-LABEL: define {{[^@]+}}@infer_align_cmpxchg_ptr
-; TUNIT-SAME: (ptr nofree align 16 captures(none) [[P:%.*]], ptr nofree [[CMP0:%.*]], ptr nofree [[CMP1:%.*]]) #[[ATTR12]] {
+; TUNIT-SAME: (ptr nofree align 16 captures(none) [[P:%.*]], ptr [[CMP0:%.*]], ptr [[CMP1:%.*]]) #[[ATTR12]] {
; TUNIT-NEXT: [[ARRAYIDX0:%.*]] = getelementptr i64, ptr [[P]], i64 1
; TUNIT-NEXT: [[ARRAYIDX1:%.*]] = getelementptr i64, ptr [[ARRAYIDX0]], i64 3
; TUNIT-NEXT: [[CMPX:%.*]] = cmpxchg ptr [[ARRAYIDX1]], ptr [[CMP0]], ptr [[CMP1]] seq_cst seq_cst, align 16
; TUNIT-NEXT: [[RET:%.*]] = extractvalue { ptr, i1 } [[CMPX]], 0
; TUNIT-NEXT: ret ptr [[RET]]
;
-; CGSCC: Function Attrs: mustprogress nofree norecurse nounwind willreturn memory(argmem: readwrite)
+; CGSCC: Function Attrs: mustprogress norecurse nounwind willreturn memory(argmem: readwrite)
; CGSCC-LABEL: define {{[^@]+}}@infer_align_cmpxchg_ptr
-; CGSCC-SAME: (ptr nofree align 16 captures(none) [[P:%.*]], ptr nofree [[CMP0:%.*]], ptr nofree [[CMP1:%.*]]) #[[ATTR14]] {
+; CGSCC-SAME: (ptr nofree align 16 captures(none) [[P:%.*]], ptr [[CMP0:%.*]], ptr [[CMP1:%.*]]) #[[ATTR14]] {
; CGSCC-NEXT: [[ARRAYIDX0:%.*]] = getelementptr i64, ptr [[P]], i64 1
; CGSCC-NEXT: [[ARRAYIDX1:%.*]] = getelementptr i64, ptr [[ARRAYIDX0]], i64 3
; CGSCC-NEXT: [[CMPX:%.*]] = cmpxchg ptr [[ARRAYIDX1]], ptr [[CMP0]], ptr [[CMP1]] seq_cst seq_cst, align 16
@@ -1230,7 +1230,7 @@ attributes #2 = { null_pointer_is_valid }
; TUNIT: attributes #[[ATTR9]] = { mustprogress nofree norecurse nosync nounwind willreturn memory(write) }
; TUNIT: attributes #[[ATTR10]] = { mustprogress nofree norecurse nosync nounwind willreturn memory(none) }
; TUNIT: attributes #[[ATTR11]] = { mustprogress nofree norecurse nosync nounwind willreturn memory(read) }
-; TUNIT: attributes #[[ATTR12]] = { mustprogress nofree norecurse nounwind willreturn memory(argmem: readwrite) }
+; TUNIT: attributes #[[ATTR12]] = { mustprogress norecurse nounwind willreturn memory(argmem: readwrite) }
; TUNIT: attributes #[[ATTR13]] = { nofree nosync nounwind }
; TUNIT: attributes #[[ATTR14]] = { nofree nosync nounwind willreturn memory(read) }
; TUNIT: attributes #[[ATTR15]] = { nofree norecurse nosync nounwind willreturn }
@@ -1249,7 +1249,7 @@ attributes #2 = { null_pointer_is_valid }
; CGSCC: attributes #[[ATTR11]] = { mustprogress nofree norecurse nosync nounwind willreturn memory(none) }
; CGSCC: attributes #[[ATTR12]] = { mustprogress nofree nosync nounwind willreturn memory(read) }
; CGSCC: attributes #[[ATTR13]] = { mustprogress nofree nosync nounwind willreturn memory(none) }
-; CGSCC: attributes #[[ATTR14]] = { mustprogress nofree norecurse nounwind willreturn memory(argmem: readwrite) }
+; CGSCC: attributes #[[ATTR14]] = { mustprogress norecurse nounwind willreturn memory(argmem: readwrite) }
; CGSCC: attributes #[[ATTR15]] = { nofree nosync willreturn }
; CGSCC: attributes #[[ATTR16]] = { nofree nosync nounwind }
; CGSCC: attributes #[[ATTR17]] = { nofree willreturn memory(read) }
diff --git a/llvm/test/Transforms/Attributor/dereferenceable-1.ll b/llvm/test/Transforms/Attributor/dereferenceable-1.ll
index 5699284128388..33e621c953d13 100644
--- a/llvm/test/Transforms/Attributor/dereferenceable-1.ll
+++ b/llvm/test/Transforms/Attributor/dereferenceable-1.ll
@@ -820,7 +820,7 @@ f:
; TUNIT: attributes #[[ATTR6:[0-9]+]] = { nocallback nofree nosync nounwind willreturn memory(inaccessiblemem: write) }
; TUNIT: attributes #[[ATTR7]] = { nofree nosync nounwind willreturn memory(write) }
; TUNIT: attributes #[[ATTR8]] = { nofree nosync nounwind memory(write) }
-; TUNIT: attributes #[[ATTR9]] = { nofree willreturn memory(write) }
+; TUNIT: attributes #[[ATTR9]] = { willreturn memory(write) }
; TUNIT: attributes #[[ATTR10]] = { nounwind }
;.
; CGSCC: attributes #[[ATTR0]] = { mustprogress nofree norecurse nosync nounwind willreturn memory(none) }
@@ -832,7 +832,7 @@ f:
; CGSCC: attributes #[[ATTR6:[0-9]+]] = { nocallback nofree nosync nounwind willreturn memory(inaccessiblemem: write) }
; CGSCC: attributes #[[ATTR7]] = { nofree nounwind willreturn memory(write) }
; CGSCC: attributes #[[ATTR8]] = { nofree nosync ...
[truncated]
|
Contributor
Author
|
Ping |
Comment on lines
299
to
-300
| define double @test12(ptr nocapture readonly %a) { | ||
| ; CHECK: Function Attrs: nofree nounwind |
Contributor
There was a problem hiding this comment.
I'm not immediately clear why the argument-level nofree is retained here, though the function attribute is not, and how the function could cause a free through synchronization. Is this desirable? I also wonder if we should orthogonally teach BuildLibCalls to annotate cos, sin, et alia with nosync.
Contributor
There was a problem hiding this comment.
I just noticed a prior (broader) attempt to add nosync to lib-calls (https://reviews.llvm.org/D98605). At least for math functions, I think it should be fine to set noysnc (previous concerns were on FP exceptions and errno).
Contributor
Author
There was a problem hiding this comment.
I'm not immediately clear why the argument-level nofree is retained here,
This is because for argument to not be nofree, you'd need both synchronization in the function, and the argument to escape. The argument here doesn't escape, so the potential synchronization doesn't matter.
though the function attribute is not, and how the function could cause a free through synchronization.
This is due to imprecision in how I implemented this in Attributor. I just made nosync a precondition for nofree, but it would be more precise to do what I do in FunctionAttrs, and only require nofree for calls and nosync for everything else. Then we could infer nofree here because the call has nofree, but not nosync.
Is this desirable? I also wonder if we should orthogonally teach BuildLibCalls to annotate cos, sin, et alia with nosync.
Yes, we should do this.
I just noticed a prior (broader) attempt to add nosync to lib-calls (https://reviews.llvm.org/D98605). At least for math functions, I think it should be fine to set noysnc (previous concerns were on FP exceptions and errno).
Yes, it looks like this patch was a bit overly ambitious. We should start with just the math libcalls and the simple string libcalls.
Contributor
Author
There was a problem hiding this comment.
though the function attribute is not, and how the function could cause a free through synchronization.
This is due to imprecision in how I implemented this in Attributor. I just made nosync a precondition for nofree, but it would be more precise to do what I do in FunctionAttrs, and only require nofree for calls and nosync for everything else. Then we could infer nofree here because the call has nofree, but not nosync.
I've done this now, restoring the nofree in this case.
nikic
force-pushed
the
nofree-inference-update
branch
from
May 15, 2026 08:21
00996db to
7647b30
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
nikic
force-pushed
the
nofree-inference-update
branch
from
May 15, 2026 08:23
7647b30 to
34b5b72
Compare
🐧 Linux x64 Test Results
Failed Tests(click on a test name to see its output) lldb-apilldb-api.python_api/run_locker/TestRunLocker.pyIf these failures are unrelated to your changes (for example tests are broken or flaky at HEAD), please open an issue at https://github.com/llvm/llvm-project/issues and add the |
antoniofrighetto
approved these changes
May 18, 2026
| const AANoCapture *NoCaptureAA = nullptr; | ||
| if (!AA::hasAssumedIRAttr<Attribute::Captures>( | ||
| A, this, IRPosition::callsite_argument(*CB, ArgNo), | ||
| DepClassTy::REQUIRED, IsKnown, false, &NoCaptureAA)) { |
Contributor
There was a problem hiding this comment.
Nit:
Suggested change
| DepClassTy::REQUIRED, IsKnown, false, &NoCaptureAA)) { | |
| DepClassTy::REQUIRED, IsKnown, /*IgnoreSubsumingPositions=*/false, &NoCaptureAA)) { |
nikic
enabled auto-merge (squash)
May 28, 2026 06:59
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/129/builds/45062 Here is the relevant piece of the build log for the reference |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This updates nofree inference after the semantics changes in #195658.
FunctionAttrs currently only supports function-level nofree inference, so we just need to check for potentially synchronizing instructions.
Attributor also supports argument nofree, in which case we make additional changes: