Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SimplifyCFG][Attributes] Enabling sinking calls with differing number of attrsets #110896

Closed

Conversation

goldsteinn
Copy link
Contributor

@goldsteinn goldsteinn commented Oct 2, 2024

  • [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.

…r 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.
@goldsteinn goldsteinn changed the title goldsteinn/simplify differing attr sets [SimplifyCFG][Attributes] Enabling sinking calls with differing number of attrsets Oct 2, 2024
@goldsteinn goldsteinn requested review from nikic, jryans and jrbyrnes and removed request for nikic and jryans October 2, 2024 17:16
@llvmbot
Copy link
Member

llvmbot commented Oct 2, 2024

@llvm/pr-subscribers-llvm-transforms

Author: None (goldsteinn)

Changes
  • [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 --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);

@llvmbot
Copy link
Member

llvmbot commented Oct 2, 2024

@llvm/pr-subscribers-llvm-ir

Author: None (goldsteinn)

Changes
  • [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 --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);

@goldsteinn
Copy link
Contributor Author

Noticed this while trying to support attr intersection in EarlyCSE.

llvm/lib/IR/Attributes.cpp Outdated Show resolved Hide resolved
llvm/lib/IR/Attributes.cpp Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Oct 2, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

llvm/lib/IR/Attributes.cpp Show resolved Hide resolved
@goldsteinn goldsteinn closed this in e343af7 Oct 2, 2024
Sterling-Augustine pushed a commit to Sterling-Augustine/llvm-project that referenced this pull request Oct 3, 2024
…r 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
xgupta pushed a commit to xgupta/llvm-project that referenced this pull request Oct 4, 2024
…r 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants