Skip to content

Commit 93532d2

Browse files
goldsteinnSterling-Augustine
goldsteinn
authored and
Sterling-Augustine
committed
[SimplifyCFG][Attributes] Enabling sinking calls with differing number of attrsets
Prior impl would fail if the number of attribute sets on the two calls wasn't the same which is unnecessary as long as we aren't throwing away and must-preserve attrs. Closes llvm#110896
1 parent 04a73ab commit 93532d2

File tree

4 files changed

+64
-22
lines changed

4 files changed

+64
-22
lines changed

‎llvm/lib/IR/Attributes.cpp‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1800,12 +1800,10 @@ AttributeList::intersectWith(LLVMContext &C, AttributeList Other) const {
18001800
if (*this == Other)
18011801
return *this;
18021802

1803-
// At least for now, only intersect lists with same number of params.
1804-
if (getNumAttrSets() != Other.getNumAttrSets())
1805-
return std::nullopt;
1806-
18071803
SmallVector<std::pair<unsigned, AttributeSet>> IntersectedAttrs;
1808-
for (unsigned Idx : indexes()) {
1804+
auto IndexIt =
1805+
index_iterator(std::max(getNumAttrSets(), Other.getNumAttrSets()));
1806+
for (unsigned Idx : IndexIt) {
18091807
auto IntersectedAS =
18101808
getAttributes(Idx).intersectWith(C, Other.getAttributes(Idx));
18111809
// If any index fails to intersect, fail.

‎llvm/lib/IR/Instruction.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -875,7 +875,8 @@ bool Instruction::isIdenticalToWhenDefined(const Instruction *I,
875875

876876
// If both instructions have no operands, they are identical.
877877
if (getNumOperands() == 0 && I->getNumOperands() == 0)
878-
return this->hasSameSpecialState(I);
878+
return this->hasSameSpecialState(I, /*IgnoreAlignment=*/false,
879+
IntersectAttrs);
879880

880881
// We have two instructions of identical opcode and #operands. Check to see
881882
// if all operands are the same.

‎llvm/test/Transforms/SimplifyCFG/sink-cb-diff-attrs.ll‎

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,13 @@ declare void @side.effect()
99
define ptr @test_sink_no_args_oneside(i1 %c) {
1010
; CHECK-LABEL: define {{[^@]+}}@test_sink_no_args_oneside
1111
; CHECK-SAME: (i1 [[C:%.*]]) {
12-
; CHECK-NEXT: br i1 [[C]], label [[IF:%.*]], label [[ELSE:%.*]]
12+
; CHECK-NEXT: br i1 [[C]], label [[IF:%.*]], label [[END:%.*]]
1313
; CHECK: if:
1414
; CHECK-NEXT: call void @side.effect()
15-
; CHECK-NEXT: [[R:%.*]] = call ptr @foo0()
16-
; CHECK-NEXT: br label [[END:%.*]]
17-
; CHECK: else:
18-
; CHECK-NEXT: [[R2:%.*]] = call ptr @foo0() #[[ATTR0:[0-9]+]]
1915
; CHECK-NEXT: br label [[END]]
2016
; CHECK: end:
21-
; CHECK-NEXT: [[PR:%.*]] = phi ptr [ [[R]], [[IF]] ], [ [[R2]], [[ELSE]] ]
22-
; CHECK-NEXT: ret ptr [[PR]]
17+
; CHECK-NEXT: [[R2:%.*]] = call ptr @foo0()
18+
; CHECK-NEXT: ret ptr [[R2]]
2319
;
2420
br i1 %c, label %if, label %else
2521
if:
@@ -44,7 +40,7 @@ define ptr @test_sink_no_args_oneside_fail(i1 %c) {
4440
; CHECK-NEXT: [[R:%.*]] = call ptr @foo0()
4541
; CHECK-NEXT: br label [[END:%.*]]
4642
; CHECK: else:
47-
; CHECK-NEXT: [[R2:%.*]] = call ptr @foo0() #[[ATTR1:[0-9]+]]
43+
; CHECK-NEXT: [[R2:%.*]] = call ptr @foo0() #[[ATTR0:[0-9]+]]
4844
; CHECK-NEXT: br label [[END]]
4945
; CHECK: end:
5046
; CHECK-NEXT: [[PR:%.*]] = phi ptr [ [[R]], [[IF]] ], [ [[R2]], [[ELSE]] ]
@@ -72,7 +68,7 @@ define ptr @test_sink_int_attrs(i1 %c, ptr %p, ptr %p2, i64 %x) {
7268
; CHECK-NEXT: call void @side.effect()
7369
; CHECK-NEXT: br label [[END]]
7470
; CHECK: end:
75-
; CHECK-NEXT: [[R2:%.*]] = call ptr @foo2(ptr align 32 dereferenceable_or_null(100) [[P]], ptr align 32 dereferenceable(50) [[P2]], i64 range(i64 10, 100000) [[X]]) #[[ATTR2:[0-9]+]]
71+
; CHECK-NEXT: [[R2:%.*]] = call ptr @foo2(ptr align 32 dereferenceable_or_null(100) [[P]], ptr align 32 dereferenceable(50) [[P2]], i64 range(i64 10, 100000) [[X]]) #[[ATTR1:[0-9]+]]
7672
; CHECK-NEXT: ret ptr [[R2]]
7773
;
7874
br i1 %c, label %if, label %else
@@ -97,7 +93,7 @@ define ptr @test_sink_int_attrs2(i1 %c, ptr %p, i64 %x) {
9793
; CHECK-NEXT: call void @side.effect()
9894
; CHECK-NEXT: br label [[END]]
9995
; CHECK: end:
100-
; CHECK-NEXT: [[R2:%.*]] = call ptr @foo(ptr dereferenceable(50) [[P]], i64 range(i64 10, 1000) [[X]]) #[[ATTR0]]
96+
; CHECK-NEXT: [[R2:%.*]] = call ptr @foo(ptr dereferenceable(50) [[P]], i64 range(i64 10, 1000) [[X]]) #[[ATTR2:[0-9]+]]
10197
; CHECK-NEXT: ret ptr [[R2]]
10298
;
10399
br i1 %c, label %if, label %else
@@ -277,9 +273,9 @@ end:
277273
}
278274

279275
;.
280-
; CHECK: attributes #[[ATTR0]] = { memory(read) }
281-
; CHECK: attributes #[[ATTR1]] = { alwaysinline memory(read) }
282-
; CHECK: attributes #[[ATTR2]] = { memory(readwrite) }
276+
; CHECK: attributes #[[ATTR0]] = { alwaysinline memory(read) }
277+
; CHECK: attributes #[[ATTR1]] = { memory(readwrite) }
278+
; CHECK: attributes #[[ATTR2]] = { memory(read) }
283279
; CHECK: attributes #[[ATTR3]] = { mustprogress nocallback nofree willreturn }
284280
; CHECK: attributes #[[ATTR4]] = { alwaysinline nosync willreturn }
285281
; CHECK: attributes #[[ATTR5]] = { alwaysinline cold nocallback nofree nosync willreturn }

‎llvm/unittests/IR/AttributesTest.cpp‎

Lines changed: 50 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -596,6 +596,38 @@ TEST(Attributes, SetIntersectByValAlign) {
596596
}
597597
}
598598

599+
TEST(Attributes, ListIntersectDifferingMustPreserve) {
600+
LLVMContext C;
601+
std::optional<AttributeList> Res;
602+
{
603+
AttributeList AL0;
604+
AttributeList AL1;
605+
AL1 = AL1.addFnAttribute(C, Attribute::ReadOnly);
606+
AL0 = AL0.addParamAttribute(C, 0, Attribute::SExt);
607+
Res = AL0.intersectWith(C, AL1);
608+
ASSERT_FALSE(Res.has_value());
609+
Res = AL1.intersectWith(C, AL0);
610+
ASSERT_FALSE(Res.has_value());
611+
}
612+
{
613+
AttributeList AL0;
614+
AttributeList AL1;
615+
AL1 = AL1.addFnAttribute(C, Attribute::AlwaysInline);
616+
AL0 = AL0.addParamAttribute(C, 0, Attribute::ReadOnly);
617+
Res = AL0.intersectWith(C, AL1);
618+
ASSERT_FALSE(Res.has_value());
619+
Res = AL1.intersectWith(C, AL0);
620+
ASSERT_FALSE(Res.has_value());
621+
622+
AL0 = AL0.addFnAttribute(C, Attribute::AlwaysInline);
623+
AL1 = AL1.addParamAttribute(C, 1, Attribute::SExt);
624+
Res = AL0.intersectWith(C, AL1);
625+
ASSERT_FALSE(Res.has_value());
626+
Res = AL1.intersectWith(C, AL0);
627+
ASSERT_FALSE(Res.has_value());
628+
}
629+
}
630+
599631
TEST(Attributes, ListIntersect) {
600632
LLVMContext C;
601633
AttributeList AL0;
@@ -610,11 +642,16 @@ TEST(Attributes, ListIntersect) {
610642

611643
AL0 = AL0.addParamAttribute(C, 1, Attribute::NoUndef);
612644
Res = AL0.intersectWith(C, AL1);
613-
ASSERT_FALSE(Res.has_value());
645+
ASSERT_TRUE(Res.has_value());
646+
ASSERT_TRUE(Res->hasRetAttr(Attribute::NoUndef));
647+
ASSERT_FALSE(Res->hasParamAttr(1, Attribute::NoUndef));
614648

615649
AL1 = AL1.addParamAttribute(C, 2, Attribute::NoUndef);
616650
Res = AL0.intersectWith(C, AL1);
617-
ASSERT_FALSE(Res.has_value());
651+
ASSERT_TRUE(Res.has_value());
652+
ASSERT_TRUE(Res->hasRetAttr(Attribute::NoUndef));
653+
ASSERT_FALSE(Res->hasParamAttr(1, Attribute::NoUndef));
654+
ASSERT_FALSE(Res->hasParamAttr(2, Attribute::NoUndef));
618655

619656
AL0 = AL0.addParamAttribute(C, 2, Attribute::NoUndef);
620657
AL1 = AL1.addParamAttribute(C, 1, Attribute::NoUndef);
@@ -692,7 +729,17 @@ TEST(Attributes, ListIntersect) {
692729

693730
AL1 = AL1.addParamAttribute(C, 3, Attribute::ReadNone);
694731
Res = AL0.intersectWith(C, AL1);
695-
ASSERT_FALSE(Res.has_value());
732+
ASSERT_TRUE(Res.has_value());
733+
ASSERT_TRUE(Res.has_value());
734+
ASSERT_TRUE(Res->hasFnAttr(Attribute::AlwaysInline));
735+
ASSERT_TRUE(Res->hasFnAttr(Attribute::ReadOnly));
736+
ASSERT_TRUE(Res->hasRetAttr(Attribute::NoUndef));
737+
ASSERT_FALSE(Res->hasRetAttr(Attribute::NonNull));
738+
ASSERT_TRUE(Res->hasParamAttr(1, Attribute::NoUndef));
739+
ASSERT_TRUE(Res->hasParamAttr(2, Attribute::NoUndef));
740+
ASSERT_FALSE(Res->hasParamAttr(2, Attribute::NonNull));
741+
ASSERT_FALSE(Res->hasParamAttr(2, Attribute::ReadNone));
742+
ASSERT_FALSE(Res->hasParamAttr(3, Attribute::ReadNone));
696743

697744
AL0 = AL0.addParamAttribute(C, 3, Attribute::ReadNone);
698745
Res = AL0.intersectWith(C, AL1);

0 commit comments

Comments
 (0)