[SimplifyCFG][Attributes] Preserve some non-equal attrs when intersecting #111014
+102
−9
Conversation
This file contains 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
…ting If for example we are intersecting `readonly` with `readnone`, we don't need to drop both, we can preserve `readonly`.
|
@llvm/pr-subscribers-llvm-ir @llvm/pr-subscribers-llvm-transforms Author: None (goldsteinn) Changes
Full diff: https://github.com/llvm/llvm-project/pull/111014.diff 3 Files Affected:
diff --git a/llvm/lib/IR/Attributes.cpp b/llvm/lib/IR/Attributes.cpp
index f2ba61ae51039e..aa5fdb48b74643 100644
--- a/llvm/lib/IR/Attributes.cpp
+++ b/llvm/lib/IR/Attributes.cpp
@@ -973,20 +973,28 @@ AttributeSet::intersectWith(LLVMContext &C, AttributeSet Other) const {
// Loop through all attributes in both this and Other in sorted order. If
// the attribute is only present in one of the sets, it will be set in
// Attr0. If it is present in both sets both Attr0 and Attr1 will be set.
+ // If the attr is only present in one of the sets, WithoutCur will point to
+ // the set that doesn't contain Attr0.
Attribute Attr0, Attr1;
- if (ItBegin1 == ItEnd1)
+ const AttributeSet *WithoutCur = nullptr;
+ if (ItBegin1 == ItEnd1) {
+ WithoutCur = &Other;
Attr0 = *ItBegin0++;
- else if (ItBegin0 == ItEnd0)
+ } else if (ItBegin0 == ItEnd0) {
+ WithoutCur = this;
Attr0 = *ItBegin1++;
- else {
+ } else {
int Cmp = ItBegin0->cmpKind(*ItBegin1);
if (Cmp == 0) {
Attr0 = *ItBegin0++;
Attr1 = *ItBegin1++;
- } else if (Cmp < 0)
+ } else if (Cmp < 0) {
+ WithoutCur = &Other;
Attr0 = *ItBegin0++;
- else
+ } else {
+ WithoutCur = this;
Attr0 = *ItBegin1++;
+ }
}
assert(Attr0.isValid() && "Iteration should always yield a valid attr");
@@ -1011,14 +1019,51 @@ AttributeSet::intersectWith(LLVMContext &C, AttributeSet Other) const {
// If we don't have both attributes, then fail if the attribute is
// must-preserve or drop it otherwise.
if (!Attr1.isValid()) {
+ assert(WithoutCur != nullptr &&
+ "Iteration with only one valid attr didn't set WithoutCur");
if (Attribute::intersectMustPreserve(Kind))
return std::nullopt;
+
+ // We can preserve some attributes without it being present in both this
+ // and Other. For example the intersection between ReadOnly and ReadNone
+ // is ReadOnly.
+ // NB: MemoryEffects don't need this extra logic. The `&` operator already
+ // does this type of intersection and missing memory effects implies
+ // unknown which we can't union.
+ switch (Kind) {
+ // ReadOnly + ReadNone -> ReadOnly
+ case Attribute::ReadOnly:
+ if (WithoutCur->hasAttribute(Attribute::ReadNone))
+ Intersected.addAttribute(Attr0);
+ break;
+ // ReadNone + ReadOnly -> ReadNone
+ case Attribute::ReadNone:
+ if (WithoutCur->hasAttribute(Attribute::ReadOnly))
+ Intersected.addAttribute(Attribute::ReadOnly);
+ break;
+ // Dereferenceable + DereferenceableOrNull -> DereferenceableOrNull
+ case Attribute::DereferenceableOrNull:
+ case Attribute::Dereferenceable: {
+ Attribute::AttrKind OtherKind = Kind == Attribute::Dereferenceable
+ ? Attribute::DereferenceableOrNull
+ : Attribute::Dereferenceable;
+ Attr1 = WithoutCur->getAttribute(OtherKind);
+ if (Attr1.isValid()) {
+ uint64_t NewVal =
+ std::min(Attr0.getValueAsInt(), Attr1.getValueAsInt());
+ Intersected.addRawIntAttr(Attribute::DereferenceableOrNull, NewVal);
+ }
+ } break;
+ default:
+ break;
+ }
continue;
}
// We have both attributes so apply the intersection rule.
assert(Attr1.hasKindAsEnum() && Kind == Attr1.getKindAsEnum() &&
"Iterator picked up two different attributes in the same iteration");
+ assert(WithoutCur == nullptr && "Iteration two valid attrs set WithoutCur");
// Attribute we can intersect with "and"
if (Attribute::intersectWithAnd(Kind)) {
diff --git a/llvm/test/Transforms/SimplifyCFG/hoist-cb-diff-attrs.ll b/llvm/test/Transforms/SimplifyCFG/hoist-cb-diff-attrs.ll
index ee6c0cfd43b253..079a86cf321ea0 100644
--- a/llvm/test/Transforms/SimplifyCFG/hoist-cb-diff-attrs.ll
+++ b/llvm/test/Transforms/SimplifyCFG/hoist-cb-diff-attrs.ll
@@ -30,7 +30,7 @@ else:
define ptr @test_hoist_int_attrs2(i1 %c, ptr %p, i64 %x) {
; CHECK-LABEL: define {{[^@]+}}@test_hoist_int_attrs2
; CHECK-SAME: (i1 [[C:%.*]], ptr [[P:%.*]], i64 [[X:%.*]]) {
-; CHECK-NEXT: [[R:%.*]] = call ptr @foo(ptr dereferenceable(50) [[P]], i64 range(i64 10, 1000) [[X]]) #[[ATTR1:[0-9]+]]
+; CHECK-NEXT: [[R:%.*]] = call ptr @foo(ptr dereferenceable(50) dereferenceable_or_null(50) [[P]], i64 range(i64 10, 1000) [[X]]) #[[ATTR1:[0-9]+]]
; CHECK-NEXT: br i1 [[C]], label [[COMMON_RET:%.*]], label [[ELSE:%.*]]
; CHECK: common.ret:
; CHECK-NEXT: ret ptr [[R]]
@@ -49,10 +49,33 @@ else:
ret ptr %r2
}
+define ptr @test_hoist_int_attrs3(i1 %c, ptr %p, ptr %p2, i64 %x) {
+; CHECK-LABEL: define {{[^@]+}}@test_hoist_int_attrs3
+; CHECK-SAME: (i1 [[C:%.*]], ptr [[P:%.*]], ptr [[P2:%.*]], i64 [[X:%.*]]) {
+; CHECK-NEXT: [[R:%.*]] = call ptr @foo2(ptr align 32 dereferenceable_or_null(100) [[P]], ptr align 32 dereferenceable_or_null(100) [[P2]], i64 range(i64 10, 100000) [[X]]) #[[ATTR0]]
+; CHECK-NEXT: br i1 [[C]], label [[COMMON_RET:%.*]], label [[ELSE:%.*]]
+; CHECK: common.ret:
+; CHECK-NEXT: ret ptr [[R]]
+; CHECK: else:
+; CHECK-NEXT: call void @side.effect()
+; CHECK-NEXT: br label [[COMMON_RET]]
+;
+ br i1 %c, label %if, label %else
+if:
+ %r = call ptr @foo2(ptr align 64 dereferenceable(100) %p, ptr dereferenceable(500) align 64 %p2, i64 range(i64 10, 1000) %x) memory(read)
+ ret ptr %r
+
+else:
+ %r2 = call ptr @foo2(ptr align 32 dereferenceable_or_null(200) %p, ptr dereferenceable_or_null(100) align 32 %p2, i64 range(i64 10000, 100000) %x) memory(write)
+ call void @side.effect()
+ ret ptr %r2
+}
+
+
define ptr @test_hoist_bool_attrs2(i1 %c, ptr %p, i64 %x) {
; CHECK-LABEL: define {{[^@]+}}@test_hoist_bool_attrs2
; CHECK-SAME: (i1 [[C:%.*]], ptr [[P:%.*]], i64 [[X:%.*]]) {
-; CHECK-NEXT: [[R:%.*]] = call noundef ptr @foo(ptr nonnull [[P]], i64 noundef [[X]]) #[[ATTR2:[0-9]+]]
+; CHECK-NEXT: [[R:%.*]] = call noundef ptr @foo(ptr nonnull readonly [[P]], i64 noundef [[X]]) #[[ATTR2:[0-9]+]]
; CHECK-NEXT: br i1 [[C]], label [[COMMON_RET:%.*]], label [[ELSE:%.*]]
; CHECK: common.ret:
; CHECK-NEXT: ret ptr [[R]]
diff --git a/llvm/test/Transforms/SimplifyCFG/sink-cb-diff-attrs.ll b/llvm/test/Transforms/SimplifyCFG/sink-cb-diff-attrs.ll
index fe9d87080dd111..bb9a43aa48b02f 100644
--- a/llvm/test/Transforms/SimplifyCFG/sink-cb-diff-attrs.ll
+++ b/llvm/test/Transforms/SimplifyCFG/sink-cb-diff-attrs.ll
@@ -93,7 +93,7 @@ define ptr @test_sink_int_attrs2(i1 %c, ptr %p, i64 %x) {
; CHECK-NEXT: call void @side.effect()
; CHECK-NEXT: br label [[END]]
; CHECK: end:
-; CHECK-NEXT: [[R2:%.*]] = call ptr @foo(ptr dereferenceable(50) [[P]], i64 range(i64 10, 1000) [[X]]) #[[ATTR2:[0-9]+]]
+; CHECK-NEXT: [[R2:%.*]] = call ptr @foo(ptr dereferenceable(50) dereferenceable_or_null(50) [[P]], i64 range(i64 10, 1000) [[X]]) #[[ATTR2:[0-9]+]]
; CHECK-NEXT: ret ptr [[R2]]
;
br i1 %c, label %if, label %else
@@ -110,6 +110,31 @@ end:
ret ptr %pr
}
+define ptr @test_sink_int_attrs3(i1 %c, ptr %p, i64 %x) {
+; CHECK-LABEL: define {{[^@]+}}@test_sink_int_attrs3
+; CHECK-SAME: (i1 [[C:%.*]], ptr [[P:%.*]], i64 [[X:%.*]]) {
+; CHECK-NEXT: br i1 [[C]], label [[IF:%.*]], label [[END:%.*]]
+; CHECK: if:
+; CHECK-NEXT: call void @side.effect()
+; CHECK-NEXT: br label [[END]]
+; CHECK: end:
+; CHECK-NEXT: [[R2:%.*]] = call ptr @foo(ptr dereferenceable_or_null(50) [[P]], i64 range(i64 10, 1000) [[X]]) #[[ATTR2]]
+; CHECK-NEXT: ret ptr [[R2]]
+;
+ br i1 %c, label %if, label %else
+if:
+ call void @side.effect()
+ %r = call ptr @foo(ptr dereferenceable_or_null(50) %p, i64 range(i64 10, 1000) %x) memory(read)
+ br label %end
+
+else:
+ %r2 = call ptr @foo(ptr dereferenceable(100) align 32 %p, i64 range(i64 11, 100) %x) memory(none)
+ br label %end
+end:
+ %pr = phi ptr [ %r, %if], [%r2, %else]
+ ret ptr %pr
+}
+
define ptr @test_sink_bool_attrs2(i1 %c, ptr %p, i64 %x) {
; CHECK-LABEL: define {{[^@]+}}@test_sink_bool_attrs2
; CHECK-SAME: (i1 [[C:%.*]], ptr [[P:%.*]], i64 [[X:%.*]]) {
@@ -118,7 +143,7 @@ define ptr @test_sink_bool_attrs2(i1 %c, ptr %p, i64 %x) {
; CHECK-NEXT: call void @side.effect()
; CHECK-NEXT: br label [[END]]
; CHECK: end:
-; CHECK-NEXT: [[R2:%.*]] = call noundef ptr @foo(ptr nonnull [[P]], i64 noundef [[X]]) #[[ATTR3:[0-9]+]]
+; CHECK-NEXT: [[R2:%.*]] = call noundef ptr @foo(ptr nonnull readonly [[P]], i64 noundef [[X]]) #[[ATTR3:[0-9]+]]
; CHECK-NEXT: ret ptr [[R2]]
;
br i1 %c, label %if, label %else
|
goldsteinn
changed the title
|
(Sorry realized I missed some of the deref + deref_or_null cases before). |
|
NB This appears to currently be a no-op, although once we integrate this into more places it might be nice to have :) |
nikic
requested changes
Oct 3, 2024
Yeah that would help elsewhere. |
MemoryEffects is the combination of Location + ModRef. For arguments, specifying the location is always "argmem", so we only need to specify the ModRef information, which is encoded by readonly/writeonly. So I don't think using MemoryEffects on arguments would make sense. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
If for example we are intersecting
readonlywithreadnone, wedon't need to drop both, we can preserve
readonly.