[llvm] [SimplifyCFG][Attributes] Enabling sinking calls with differing number of attrsets (PR #110896)

via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 2 10:16:09 PDT 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-transforms

Author: None (goldsteinn)

<details>
<summary>Changes</summary>

- **[SimplifyCFG] Add tests for sinking calls with differing number of attrs; NFC**
- **[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.

---
Full diff: https://github.com/llvm/llvm-project/pull/110896.diff


4 Files Affected:

- (modified) llvm/lib/IR/Attributes.cpp (+14-7) 
- (modified) llvm/lib/IR/Instruction.cpp (+2-1) 
- (modified) llvm/test/Transforms/SimplifyCFG/sink-cb-diff-attrs.ll (+75-19) 
- (modified) llvm/unittests/IR/AttributesTest.cpp (+18-3) 


``````````diff
diff --git a/llvm/lib/IR/Attributes.cpp b/llvm/lib/IR/Attributes.cpp
index c5874c58c0ecae..9f0f2d983dd74c 100644
--- a/llvm/lib/IR/Attributes.cpp
+++ b/llvm/lib/IR/Attributes.cpp
@@ -18,6 +18,7 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
@@ -1800,14 +1801,20 @@ AttributeList::intersectWith(LLVMContext &C, AttributeList Other) const {
   if (*this == Other)
     return *this;
 
-  // At least for now, only intersect lists with same number of params.
-  if (getNumAttrSets() != Other.getNumAttrSets())
-    return std::nullopt;
-
   SmallVector<std::pair<unsigned, AttributeSet>> IntersectedAttrs;
-  for (unsigned Idx : indexes()) {
-    auto IntersectedAS =
-        getAttributes(Idx).intersectWith(C, Other.getAttributes(Idx));
+  SmallSet<unsigned, 8> AllIndexes{};
+  AllIndexes.insert(indexes().begin(), indexes().end());
+  AllIndexes.insert(Other.indexes().begin(), Other.indexes().end());
+
+  for (unsigned Idx : AllIndexes) {
+    AttributeSet ThisSet = {};
+    AttributeSet OtherSet = {};
+    if (hasAttributesAtIndex(Idx))
+      ThisSet = getAttributes(Idx);
+    if (Other.hasAttributesAtIndex(Idx))
+      OtherSet = Other.getAttributes(Idx);
+
+    auto IntersectedAS = ThisSet.intersectWith(C, OtherSet);
     // If any index fails to intersect, fail.
     if (!IntersectedAS)
       return std::nullopt;
diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp
index b84e62d2e43b7e..eec751078698b2 100644
--- a/llvm/lib/IR/Instruction.cpp
+++ b/llvm/lib/IR/Instruction.cpp
@@ -875,7 +875,8 @@ bool Instruction::isIdenticalToWhenDefined(const Instruction *I,
 
   // If both instructions have no operands, they are identical.
   if (getNumOperands() == 0 && I->getNumOperands() == 0)
-    return this->hasSameSpecialState(I);
+    return this->hasSameSpecialState(I, /*IgnoreAlignment=*/false,
+                                     IntersectAttrs);
 
   // We have two instructions of identical opcode and #operands.  Check to see
   // if all operands are the same.
diff --git a/llvm/test/Transforms/SimplifyCFG/sink-cb-diff-attrs.ll b/llvm/test/Transforms/SimplifyCFG/sink-cb-diff-attrs.ll
index 1998055ca75f2e..fe9d87080dd111 100644
--- a/llvm/test/Transforms/SimplifyCFG/sink-cb-diff-attrs.ll
+++ b/llvm/test/Transforms/SimplifyCFG/sink-cb-diff-attrs.ll
@@ -1,10 +1,65 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature --check-attributes --check-globals
 ; RUN: opt < %s -passes='simplifycfg<sink-common-insts>' -simplifycfg-require-and-preserve-domtree=1 -S | FileCheck %s
 
+declare ptr @foo0()
 declare ptr @foo(ptr %p, i64 %x)
 declare ptr @foo2(ptr %p, ptr %p2, i64 %x)
 declare void @side.effect()
 
+define ptr @test_sink_no_args_oneside(i1 %c) {
+; CHECK-LABEL: define {{[^@]+}}@test_sink_no_args_oneside
+; CHECK-SAME: (i1 [[C:%.*]]) {
+; 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 @foo0()
+; CHECK-NEXT:    ret ptr [[R2]]
+;
+  br i1 %c, label %if, label %else
+if:
+  call void @side.effect()
+  %r = call ptr @foo0()
+  br label %end
+
+else:
+  %r2 = call ptr @foo0() readonly
+  br label %end
+end:
+  %pr = phi ptr [ %r, %if], [%r2, %else]
+  ret ptr %pr
+}
+
+define ptr @test_sink_no_args_oneside_fail(i1 %c) {
+; CHECK-LABEL: define {{[^@]+}}@test_sink_no_args_oneside_fail
+; CHECK-SAME: (i1 [[C:%.*]]) {
+; CHECK-NEXT:    br i1 [[C]], label [[IF:%.*]], label [[ELSE:%.*]]
+; CHECK:       if:
+; CHECK-NEXT:    call void @side.effect()
+; CHECK-NEXT:    [[R:%.*]] = call ptr @foo0()
+; CHECK-NEXT:    br label [[END:%.*]]
+; CHECK:       else:
+; CHECK-NEXT:    [[R2:%.*]] = call ptr @foo0() #[[ATTR0:[0-9]+]]
+; CHECK-NEXT:    br label [[END]]
+; CHECK:       end:
+; CHECK-NEXT:    [[PR:%.*]] = phi ptr [ [[R]], [[IF]] ], [ [[R2]], [[ELSE]] ]
+; CHECK-NEXT:    ret ptr [[PR]]
+;
+  br i1 %c, label %if, label %else
+if:
+  call void @side.effect()
+  %r = call ptr @foo0()
+  br label %end
+
+else:
+  %r2 = call ptr @foo0() readonly alwaysinline
+  br label %end
+end:
+  %pr = phi ptr [ %r, %if], [%r2, %else]
+  ret ptr %pr
+}
+
 define ptr @test_sink_int_attrs(i1 %c, ptr %p, ptr %p2, i64 %x) {
 ; CHECK-LABEL: define {{[^@]+}}@test_sink_int_attrs
 ; CHECK-SAME: (i1 [[C:%.*]], ptr [[P:%.*]], ptr [[P2:%.*]], i64 [[X:%.*]]) {
@@ -13,7 +68,7 @@ define ptr @test_sink_int_attrs(i1 %c, ptr %p, ptr %p2, i64 %x) {
 ; CHECK-NEXT:    call void @side.effect()
 ; CHECK-NEXT:    br label [[END]]
 ; CHECK:       end:
-; 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]]) #[[ATTR0:[0-9]+]]
+; 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]+]]
 ; CHECK-NEXT:    ret ptr [[R2]]
 ;
   br i1 %c, label %if, label %else
@@ -38,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]]) #[[ATTR1:[0-9]+]]
+; CHECK-NEXT:    [[R2:%.*]] = call ptr @foo(ptr dereferenceable(50) [[P]], i64 range(i64 10, 1000) [[X]]) #[[ATTR2:[0-9]+]]
 ; CHECK-NEXT:    ret ptr [[R2]]
 ;
   br i1 %c, label %if, label %else
@@ -63,7 +118,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]]) #[[ATTR2:[0-9]+]]
+; CHECK-NEXT:    [[R2:%.*]] = call noundef ptr @foo(ptr nonnull [[P]], i64 noundef [[X]]) #[[ATTR3:[0-9]+]]
 ; CHECK-NEXT:    ret ptr [[R2]]
 ;
   br i1 %c, label %if, label %else
@@ -88,7 +143,7 @@ define ptr @test_sink_bool_attrs3(i1 %c, ptr %p, i64 %x) {
 ; CHECK-NEXT:    call void @side.effect()
 ; CHECK-NEXT:    br label [[END]]
 ; CHECK:       end:
-; CHECK-NEXT:    [[R2:%.*]] = call nonnull ptr @foo(ptr [[P]], i64 noundef [[X]]) #[[ATTR3:[0-9]+]]
+; CHECK-NEXT:    [[R2:%.*]] = call nonnull ptr @foo(ptr [[P]], i64 noundef [[X]]) #[[ATTR4:[0-9]+]]
 ; CHECK-NEXT:    ret ptr [[R2]]
 ;
   br i1 %c, label %if, label %else
@@ -111,10 +166,10 @@ define ptr @test_sink_bool_attrs_fail_non_droppable(i1 %c, ptr %p, i64 %x) {
 ; CHECK-NEXT:    br i1 [[C]], label [[IF:%.*]], label [[ELSE:%.*]]
 ; CHECK:       if:
 ; CHECK-NEXT:    call void @side.effect()
-; CHECK-NEXT:    [[R:%.*]] = call nonnull ptr @foo(ptr noundef readonly [[P]], i64 noundef [[X]]) #[[ATTR4:[0-9]+]]
+; CHECK-NEXT:    [[R:%.*]] = call nonnull ptr @foo(ptr noundef readonly [[P]], i64 noundef [[X]]) #[[ATTR5:[0-9]+]]
 ; CHECK-NEXT:    br label [[END:%.*]]
 ; CHECK:       else:
-; CHECK-NEXT:    [[R2:%.*]] = call noundef nonnull ptr @foo(ptr nonnull writeonly [[P]], i64 noundef [[X]]) #[[ATTR5:[0-9]+]]
+; CHECK-NEXT:    [[R2:%.*]] = call noundef nonnull ptr @foo(ptr nonnull writeonly [[P]], i64 noundef [[X]]) #[[ATTR6:[0-9]+]]
 ; CHECK-NEXT:    br label [[END]]
 ; CHECK:       end:
 ; CHECK-NEXT:    [[PR:%.*]] = phi ptr [ [[R]], [[IF]] ], [ [[R2]], [[ELSE]] ]
@@ -140,10 +195,10 @@ define ptr @test_sink_bool_attrs_fail_non_droppable2(i1 %c, ptr %p, i64 %x) {
 ; CHECK-NEXT:    br i1 [[C]], label [[IF:%.*]], label [[ELSE:%.*]]
 ; CHECK:       if:
 ; CHECK-NEXT:    call void @side.effect()
-; CHECK-NEXT:    [[R:%.*]] = call nonnull ptr @foo(ptr noundef readonly [[P]], i64 noundef [[X]]) #[[ATTR6:[0-9]+]]
+; CHECK-NEXT:    [[R:%.*]] = call nonnull ptr @foo(ptr noundef readonly [[P]], i64 noundef [[X]]) #[[ATTR7:[0-9]+]]
 ; CHECK-NEXT:    br label [[END:%.*]]
 ; CHECK:       else:
-; CHECK-NEXT:    [[R2:%.*]] = call noundef nonnull ptr @foo(ptr nonnull writeonly byval(i64) [[P]], i64 noundef [[X]]) #[[ATTR5]]
+; CHECK-NEXT:    [[R2:%.*]] = call noundef nonnull ptr @foo(ptr nonnull writeonly byval(i64) [[P]], i64 noundef [[X]]) #[[ATTR6]]
 ; CHECK-NEXT:    br label [[END]]
 ; CHECK:       end:
 ; CHECK-NEXT:    [[PR:%.*]] = phi ptr [ [[R]], [[IF]] ], [ [[R2]], [[ELSE]] ]
@@ -169,10 +224,10 @@ define ptr @test_sink_bool_attrs_fail_non_droppable3(i1 %c, ptr %p, i64 %x) {
 ; CHECK-NEXT:    br i1 [[C]], label [[IF:%.*]], label [[ELSE:%.*]]
 ; CHECK:       if:
 ; CHECK-NEXT:    call void @side.effect()
-; CHECK-NEXT:    [[R:%.*]] = call nonnull ptr @foo(ptr noundef readonly byval(i32) [[P]], i64 noundef [[X]]) #[[ATTR6]]
+; CHECK-NEXT:    [[R:%.*]] = call nonnull ptr @foo(ptr noundef readonly byval(i32) [[P]], i64 noundef [[X]]) #[[ATTR7]]
 ; CHECK-NEXT:    br label [[END:%.*]]
 ; CHECK:       else:
-; CHECK-NEXT:    [[R2:%.*]] = call noundef nonnull ptr @foo(ptr nonnull writeonly byval(i64) [[P]], i64 noundef [[X]]) #[[ATTR7:[0-9]+]]
+; CHECK-NEXT:    [[R2:%.*]] = call noundef nonnull ptr @foo(ptr nonnull writeonly byval(i64) [[P]], i64 noundef [[X]]) #[[ATTR8:[0-9]+]]
 ; CHECK-NEXT:    br label [[END]]
 ; CHECK:       end:
 ; CHECK-NEXT:    [[PR:%.*]] = phi ptr [ [[R]], [[IF]] ], [ [[R2]], [[ELSE]] ]
@@ -200,7 +255,7 @@ define ptr @test_sink_bool_attrs4(i1 %c, ptr %p, i64 %x) {
 ; CHECK-NEXT:    call void @side.effect()
 ; CHECK-NEXT:    br label [[END]]
 ; CHECK:       end:
-; CHECK-NEXT:    [[R2:%.*]] = call nonnull ptr @foo(ptr byval(i64) [[P]], i64 noundef [[X]]) #[[ATTR7]]
+; CHECK-NEXT:    [[R2:%.*]] = call nonnull ptr @foo(ptr byval(i64) [[P]], i64 noundef [[X]]) #[[ATTR8]]
 ; CHECK-NEXT:    ret ptr [[R2]]
 ;
   br i1 %c, label %if, label %else
@@ -218,12 +273,13 @@ end:
 }
 
 ;.
-; CHECK: attributes #[[ATTR0]] = { memory(readwrite) }
-; CHECK: attributes #[[ATTR1]] = { memory(read) }
-; CHECK: attributes #[[ATTR2]] = { mustprogress nocallback nofree willreturn }
-; CHECK: attributes #[[ATTR3]] = { alwaysinline nosync willreturn }
-; CHECK: attributes #[[ATTR4]] = { alwaysinline cold nocallback nofree nosync willreturn }
-; CHECK: attributes #[[ATTR5]] = { nosync willreturn }
-; CHECK: attributes #[[ATTR6]] = { cold nocallback nofree nosync willreturn }
-; CHECK: attributes #[[ATTR7]] = { nosync }
+; CHECK: attributes #[[ATTR0]] = { alwaysinline memory(read) }
+; CHECK: attributes #[[ATTR1]] = { memory(readwrite) }
+; CHECK: attributes #[[ATTR2]] = { memory(read) }
+; CHECK: attributes #[[ATTR3]] = { mustprogress nocallback nofree willreturn }
+; CHECK: attributes #[[ATTR4]] = { alwaysinline nosync willreturn }
+; CHECK: attributes #[[ATTR5]] = { alwaysinline cold nocallback nofree nosync willreturn }
+; CHECK: attributes #[[ATTR6]] = { nosync willreturn }
+; CHECK: attributes #[[ATTR7]] = { cold nocallback nofree nosync willreturn }
+; CHECK: attributes #[[ATTR8]] = { nosync }
 ;.
diff --git a/llvm/unittests/IR/AttributesTest.cpp b/llvm/unittests/IR/AttributesTest.cpp
index 0fdb5a9c5a6886..316d7db924d05f 100644
--- a/llvm/unittests/IR/AttributesTest.cpp
+++ b/llvm/unittests/IR/AttributesTest.cpp
@@ -610,11 +610,16 @@ TEST(Attributes, ListIntersect) {
 
   AL0 = AL0.addParamAttribute(C, 1, Attribute::NoUndef);
   Res = AL0.intersectWith(C, AL1);
-  ASSERT_FALSE(Res.has_value());
+  ASSERT_TRUE(Res.has_value());
+  ASSERT_TRUE(Res->hasRetAttr(Attribute::NoUndef));
+  ASSERT_FALSE(Res->hasParamAttr(1, Attribute::NoUndef));
 
   AL1 = AL1.addParamAttribute(C, 2, Attribute::NoUndef);
   Res = AL0.intersectWith(C, AL1);
-  ASSERT_FALSE(Res.has_value());
+  ASSERT_TRUE(Res.has_value());
+  ASSERT_TRUE(Res->hasRetAttr(Attribute::NoUndef));
+  ASSERT_FALSE(Res->hasParamAttr(1, Attribute::NoUndef));
+  ASSERT_FALSE(Res->hasParamAttr(2, Attribute::NoUndef));
 
   AL0 = AL0.addParamAttribute(C, 2, Attribute::NoUndef);
   AL1 = AL1.addParamAttribute(C, 1, Attribute::NoUndef);
@@ -692,7 +697,17 @@ TEST(Attributes, ListIntersect) {
 
   AL1 = AL1.addParamAttribute(C, 3, Attribute::ReadNone);
   Res = AL0.intersectWith(C, AL1);
-  ASSERT_FALSE(Res.has_value());
+  ASSERT_TRUE(Res.has_value());
+  ASSERT_TRUE(Res.has_value());
+  ASSERT_TRUE(Res->hasFnAttr(Attribute::AlwaysInline));
+  ASSERT_TRUE(Res->hasFnAttr(Attribute::ReadOnly));
+  ASSERT_TRUE(Res->hasRetAttr(Attribute::NoUndef));
+  ASSERT_FALSE(Res->hasRetAttr(Attribute::NonNull));
+  ASSERT_TRUE(Res->hasParamAttr(1, Attribute::NoUndef));
+  ASSERT_TRUE(Res->hasParamAttr(2, Attribute::NoUndef));
+  ASSERT_FALSE(Res->hasParamAttr(2, Attribute::NonNull));
+  ASSERT_FALSE(Res->hasParamAttr(2, Attribute::ReadNone));
+  ASSERT_FALSE(Res->hasParamAttr(3, Attribute::ReadNone));
 
   AL0 = AL0.addParamAttribute(C, 3, Attribute::ReadNone);
   Res = AL0.intersectWith(C, AL1);

``````````

</details>


https://github.com/llvm/llvm-project/pull/110896


More information about the llvm-commits mailing list