Skip to content

Commit b15106c

Browse files
goldsteinngoldsteinn
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.
1 parent 7567bda commit b15106c

File tree

4 files changed

+43
-24
lines changed

4 files changed

+43
-24
lines changed

‎llvm/lib/IR/Attributes.cpp‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "llvm/ADT/ArrayRef.h"
1919
#include "llvm/ADT/FoldingSet.h"
2020
#include "llvm/ADT/STLExtras.h"
21+
#include "llvm/ADT/SmallSet.h"
2122
#include "llvm/ADT/SmallVector.h"
2223
#include "llvm/ADT/StringExtras.h"
2324
#include "llvm/ADT/StringRef.h"
@@ -1800,14 +1801,20 @@ AttributeList::intersectWith(LLVMContext &C, AttributeList Other) const {
18001801
if (*this == Other)
18011802
return *this;
18021803

1803-
// At least for now, only intersect lists with same number of params.
1804-
if (getNumAttrSets() != Other.getNumAttrSets())
1805-
return std::nullopt;
1806-
18071804
SmallVector<std::pair<unsigned, AttributeSet>> IntersectedAttrs;
1808-
for (unsigned Idx : indexes()) {
1809-
auto IntersectedAS =
1810-
getAttributes(Idx).intersectWith(C, Other.getAttributes(Idx));
1805+
SmallSet<unsigned, 8> AllIndexes{};
1806+
AllIndexes.insert(indexes().begin(), indexes().end());
1807+
AllIndexes.insert(Other.indexes().begin(), Other.indexes().end());
1808+
1809+
for (unsigned Idx : AllIndexes) {
1810+
AttributeSet ThisSet = {};
1811+
AttributeSet OtherSet = {};
1812+
if (hasAttributesAtIndex(Idx))
1813+
ThisSet = getAttributes(Idx);
1814+
if (Other.hasAttributesAtIndex(Idx))
1815+
OtherSet = Other.getAttributes(Idx);
1816+
1817+
auto IntersectedAS = ThisSet.intersectWith(C, OtherSet);
18111818
// If any index fails to intersect, fail.
18121819
if (!IntersectedAS)
18131820
return std::nullopt;

‎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: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -610,11 +610,16 @@ TEST(Attributes, ListIntersect) {
610610

611611
AL0 = AL0.addParamAttribute(C, 1, Attribute::NoUndef);
612612
Res = AL0.intersectWith(C, AL1);
613-
ASSERT_FALSE(Res.has_value());
613+
ASSERT_TRUE(Res.has_value());
614+
ASSERT_TRUE(Res->hasRetAttr(Attribute::NoUndef));
615+
ASSERT_FALSE(Res->hasParamAttr(1, Attribute::NoUndef));
614616

615617
AL1 = AL1.addParamAttribute(C, 2, Attribute::NoUndef);
616618
Res = AL0.intersectWith(C, AL1);
617-
ASSERT_FALSE(Res.has_value());
619+
ASSERT_TRUE(Res.has_value());
620+
ASSERT_TRUE(Res->hasRetAttr(Attribute::NoUndef));
621+
ASSERT_FALSE(Res->hasParamAttr(1, Attribute::NoUndef));
622+
ASSERT_FALSE(Res->hasParamAttr(2, Attribute::NoUndef));
618623

619624
AL0 = AL0.addParamAttribute(C, 2, Attribute::NoUndef);
620625
AL1 = AL1.addParamAttribute(C, 1, Attribute::NoUndef);
@@ -692,7 +697,17 @@ TEST(Attributes, ListIntersect) {
692697

693698
AL1 = AL1.addParamAttribute(C, 3, Attribute::ReadNone);
694699
Res = AL0.intersectWith(C, AL1);
695-
ASSERT_FALSE(Res.has_value());
700+
ASSERT_TRUE(Res.has_value());
701+
ASSERT_TRUE(Res.has_value());
702+
ASSERT_TRUE(Res->hasFnAttr(Attribute::AlwaysInline));
703+
ASSERT_TRUE(Res->hasFnAttr(Attribute::ReadOnly));
704+
ASSERT_TRUE(Res->hasRetAttr(Attribute::NoUndef));
705+
ASSERT_FALSE(Res->hasRetAttr(Attribute::NonNull));
706+
ASSERT_TRUE(Res->hasParamAttr(1, Attribute::NoUndef));
707+
ASSERT_TRUE(Res->hasParamAttr(2, Attribute::NoUndef));
708+
ASSERT_FALSE(Res->hasParamAttr(2, Attribute::NonNull));
709+
ASSERT_FALSE(Res->hasParamAttr(2, Attribute::ReadNone));
710+
ASSERT_FALSE(Res->hasParamAttr(3, Attribute::ReadNone));
696711

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

0 commit comments

Comments
 (0)