[llvm] [EarlyCSE] De-Duplicate callsites with differing attrs (PR #110929)

via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 3 09:34:19 PDT 2024


https://github.com/goldsteinn updated https://github.com/llvm/llvm-project/pull/110929

>From 2cf7c3a2c4116b6c9dfe43dc4292d56959e11fb3 Mon Sep 17 00:00:00 2001
From: Noah Goldstein <goldstein.w.n at gmail.com>
Date: Wed, 2 Oct 2024 15:53:27 -0500
Subject: [PATCH 1/3] [EarlyCSE] Add tests for de-duplication of callsites with
 differing attrs; NFC

---
 .../EarlyCSE/replace-calls-def-attrs.ll       | 231 ++++++++++++++++++
 1 file changed, 231 insertions(+)
 create mode 100644 llvm/test/Transforms/EarlyCSE/replace-calls-def-attrs.ll

diff --git a/llvm/test/Transforms/EarlyCSE/replace-calls-def-attrs.ll b/llvm/test/Transforms/EarlyCSE/replace-calls-def-attrs.ll
new file mode 100644
index 00000000000000..4e35c18cd8901c
--- /dev/null
+++ b/llvm/test/Transforms/EarlyCSE/replace-calls-def-attrs.ll
@@ -0,0 +1,231 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-attributes --check-globals all --version 5
+; RUN: opt -S -passes=early-cse  < %s | FileCheck %s
+
+declare i8 @baz(i8, i8)
+declare i8 @baz_side_effects(i8, i8)
+declare i8 @buz(i8, i8)
+
+declare ptr @baz.ptr(i8, i8)
+declare i8 @buz.ptr(ptr, ptr)
+define i8 @same_parent_combine_diff_attrs(i8 %x, i8 %y) {
+; CHECK-LABEL: define i8 @same_parent_combine_diff_attrs(
+; CHECK-SAME: i8 [[X:%.*]], i8 [[Y:%.*]]) {
+; CHECK-NEXT:    [[C2:%.*]] = call i8 @baz(i8 noundef [[X]], i8 noundef [[Y]]) #[[ATTR0:[0-9]+]]
+; CHECK-NEXT:    [[C1:%.*]] = call i8 @baz(i8 [[X]], i8 noundef [[Y]]) #[[ATTR0]]
+; CHECK-NEXT:    [[R:%.*]] = call i8 @buz(i8 [[C1]], i8 [[C2]])
+; CHECK-NEXT:    ret i8 [[R]]
+;
+  %c1 = call i8 @baz(i8 noundef %x, i8 noundef %y) readnone
+  %c0 = call i8 @baz(i8 %x, i8 noundef %y) readnone
+  %r = call i8 @buz(i8 %c0, i8 %c1)
+  ret i8 %r
+
+}
+
+define i8 @same_parent_combine_diff_attrs_needs_intersect(i8 %x, i8 %y) {
+; CHECK-LABEL: define i8 @same_parent_combine_diff_attrs_needs_intersect(
+; CHECK-SAME: i8 [[X:%.*]], i8 [[Y:%.*]]) {
+; CHECK-NEXT:    [[C1:%.*]] = call nonnull ptr @baz.ptr(i8 noundef [[X]], i8 noundef [[Y]]) #[[ATTR0]]
+; CHECK-NEXT:    [[C0:%.*]] = call ptr @baz.ptr(i8 [[X]], i8 noundef [[Y]]) #[[ATTR0]]
+; CHECK-NEXT:    [[R:%.*]] = call i8 @buz.ptr(ptr [[C0]], ptr [[C1]])
+; CHECK-NEXT:    ret i8 [[R]]
+;
+  %c1 = call nonnull ptr @baz.ptr(i8 noundef %x, i8 noundef %y) readnone
+  %c0 = call ptr @baz.ptr(i8 %x, i8 noundef %y) readnone
+  %r = call i8 @buz.ptr(ptr %c0, ptr %c1)
+  ret i8 %r
+
+}
+
+define i8 @same_parent_combine_diff_attrs_needs_intersect2(i8 %x, i8 %y) {
+; CHECK-LABEL: define i8 @same_parent_combine_diff_attrs_needs_intersect2(
+; CHECK-SAME: i8 [[X:%.*]], i8 [[Y:%.*]]) {
+; CHECK-NEXT:    [[C1:%.*]] = call nonnull ptr @baz.ptr(i8 noundef [[X]], i8 noundef [[Y]]) #[[ATTR1:[0-9]+]]
+; CHECK-NEXT:    [[C0:%.*]] = call ptr @baz.ptr(i8 [[X]], i8 noundef [[Y]]) #[[ATTR1]]
+; CHECK-NEXT:    [[R:%.*]] = call i8 @buz.ptr(ptr [[C0]], ptr [[C1]])
+; CHECK-NEXT:    ret i8 [[R]]
+;
+  %c1 = call nonnull ptr @baz.ptr(i8 noundef %x, i8 noundef %y) readonly
+  %c0 = call ptr @baz.ptr(i8 %x, i8 noundef %y) readonly
+  %r = call i8 @buz.ptr(ptr %c0, ptr %c1)
+  ret i8 %r
+
+}
+
+define i8 @same_parent_combine_diff_attrs_really_needs_intersect(i8 %x, i8 %y) {
+; CHECK-LABEL: define i8 @same_parent_combine_diff_attrs_really_needs_intersect(
+; CHECK-SAME: i8 [[X:%.*]], i8 [[Y:%.*]]) {
+; CHECK-NEXT:    [[C1:%.*]] = call nonnull ptr @baz.ptr(i8 noundef [[X]], i8 noundef [[Y]]) #[[ATTR1]]
+; CHECK-NEXT:    [[C0:%.*]] = call ptr @baz.ptr(i8 [[X]], i8 noundef [[Y]]) #[[ATTR1]]
+; CHECK-NEXT:    [[R:%.*]] = call i8 @buz.ptr(ptr [[C0]], ptr noundef [[C1]])
+; CHECK-NEXT:    ret i8 [[R]]
+;
+  %c1 = call nonnull ptr @baz.ptr(i8 noundef %x, i8 noundef %y) readonly
+  %c0 = call ptr @baz.ptr(i8 %x, i8 noundef %y) readonly
+  %r = call i8 @buz.ptr(ptr %c0, ptr noundef %c1)
+  ret i8 %r
+
+}
+
+define i8 @same_parent_combine_diff_attrs_fail_side_effects(i8 %x, i8 %y) {
+; CHECK-LABEL: define i8 @same_parent_combine_diff_attrs_fail_side_effects(
+; CHECK-SAME: i8 [[X:%.*]], i8 [[Y:%.*]]) {
+; CHECK-NEXT:    [[C1:%.*]] = call i8 @baz(i8 noundef [[X]], i8 noundef [[Y]])
+; CHECK-NEXT:    [[C0:%.*]] = call i8 @baz(i8 [[X]], i8 noundef [[Y]])
+; CHECK-NEXT:    [[R:%.*]] = call i8 @buz(i8 [[C0]], i8 [[C1]])
+; CHECK-NEXT:    ret i8 [[R]]
+;
+  %c1 = call i8 @baz(i8 noundef %x, i8 noundef %y)
+  %c0 = call i8 @baz(i8 %x, i8 noundef %y)
+  %r = call i8 @buz(i8 %c0, i8 %c1)
+  ret i8 %r
+
+}
+
+define i8 @same_parent_combine_diff_attrs_quasi_side_effects2(i8 %x, i8 %y) {
+; CHECK-LABEL: define i8 @same_parent_combine_diff_attrs_quasi_side_effects2(
+; CHECK-SAME: i8 [[X:%.*]], i8 [[Y:%.*]]) {
+; CHECK-NEXT:    [[C1:%.*]] = call i8 @baz(i8 noundef [[X]], i8 noundef [[Y]]) #[[ATTR0]]
+; CHECK-NEXT:    [[C0:%.*]] = call i8 @baz(i8 [[X]], i8 noundef [[Y]])
+; CHECK-NEXT:    [[R:%.*]] = call i8 @buz(i8 [[C0]], i8 [[C1]])
+; CHECK-NEXT:    ret i8 [[R]]
+;
+  %c1 = call i8 @baz(i8 noundef %x, i8 noundef %y) readnone
+  %c0 = call i8 @baz(i8 %x, i8 noundef %y)
+  %r = call i8 @buz(i8 %c0, i8 %c1)
+  ret i8 %r
+
+}
+
+
+define i8 @diff_parent_combine_diff_attrs(i1 %c, i8 %x, i8 %y) {
+; CHECK-LABEL: define i8 @diff_parent_combine_diff_attrs(
+; CHECK-SAME: i1 [[C:%.*]], i8 [[X:%.*]], i8 [[Y:%.*]]) {
+; CHECK-NEXT:    [[C1:%.*]] = call i8 @baz(i8 [[X]], i8 noundef [[Y]]) #[[ATTR0]]
+; CHECK-NEXT:    br i1 [[C]], label %[[T:.*]], label %[[F:.*]]
+; CHECK:       [[T]]:
+; CHECK-NEXT:    [[C0:%.*]] = call i8 @baz(i8 noundef [[X]], i8 noundef [[Y]]) #[[ATTR1]]
+; CHECK-NEXT:    [[R:%.*]] = call i8 @buz(i8 [[C0]], i8 [[C1]])
+; CHECK-NEXT:    ret i8 [[R]]
+; CHECK:       [[F]]:
+; CHECK-NEXT:    [[R2:%.*]] = add i8 [[C1]], 4
+; CHECK-NEXT:    ret i8 [[R2]]
+;
+  %c1 = call i8 @baz(i8 %x, i8 noundef %y) readnone
+  br i1 %c, label %T, label %F
+T:
+  %c0 = call i8 @baz(i8 noundef %x, i8 noundef %y) readonly
+  %r = call i8 @buz(i8 %c0, i8 %c1)
+  ret i8 %r
+F:
+  %r2 = add i8 %c1, 4
+  ret i8 %r2
+}
+
+define i8 @diff_parent_combine_diff_attrs_preserves_return_attrs(i1 %c, i8 %x, i8 %y) {
+; CHECK-LABEL: define i8 @diff_parent_combine_diff_attrs_preserves_return_attrs(
+; CHECK-SAME: i1 [[C:%.*]], i8 [[X:%.*]], i8 [[Y:%.*]]) {
+; CHECK-NEXT:    [[C1:%.*]] = call nonnull ptr @baz.ptr(i8 [[X]], i8 noundef [[Y]]) #[[ATTR1]]
+; CHECK-NEXT:    br i1 [[C]], label %[[T:.*]], label %[[F:.*]]
+; CHECK:       [[T]]:
+; CHECK-NEXT:    [[C0:%.*]] = call nonnull ptr @baz.ptr(i8 noundef [[X]], i8 noundef [[Y]]) #[[ATTR1]]
+; CHECK-NEXT:    [[R:%.*]] = call i8 @buz.ptr(ptr [[C0]], ptr noundef [[C1]])
+; CHECK-NEXT:    ret i8 [[R]]
+; CHECK:       [[F]]:
+; CHECK-NEXT:    ret i8 9
+;
+  %c1 = call nonnull ptr @baz.ptr(i8 %x, i8 noundef %y) readonly
+  br i1 %c, label %T, label %F
+T:
+  %c0 = call nonnull ptr @baz.ptr(i8 noundef %x, i8 noundef %y) readonly
+  %r = call i8 @buz.ptr(ptr %c0, ptr noundef %c1)
+  ret i8 %r
+F:
+  ret i8 9
+}
+
+define i8 @same_parent_combine_diff_attrs_todo(i8 %x, i8 %y) {
+; CHECK-LABEL: define i8 @same_parent_combine_diff_attrs_todo(
+; CHECK-SAME: i8 [[X:%.*]], i8 [[Y:%.*]]) {
+; CHECK-NEXT:    [[C1:%.*]] = call i8 @baz(i8 [[X]], i8 noundef [[Y]]) #[[ATTR0]]
+; CHECK-NEXT:    [[C0:%.*]] = call i8 @baz(i8 noundef [[X]], i8 noundef [[Y]]) #[[ATTR2:[0-9]+]]
+; CHECK-NEXT:    [[R:%.*]] = call i8 @buz(i8 [[C0]], i8 [[C1]])
+; CHECK-NEXT:    ret i8 [[R]]
+;
+  %c1 = call i8 @baz(i8 %x, i8 noundef %y) readnone
+  %c0 = call i8 @baz(i8 noundef %x, i8 noundef %y) readnone alwaysinline
+  %r = call i8 @buz(i8 %c0, i8 %c1)
+  ret i8 %r
+
+}
+
+define i8 @same_parent_combine_diff_attrs_fail(i8 %x, i8 %y) {
+; CHECK-LABEL: define i8 @same_parent_combine_diff_attrs_fail(
+; CHECK-SAME: i8 [[X:%.*]], i8 [[Y:%.*]]) {
+; CHECK-NEXT:    [[C1:%.*]] = call i8 @baz(i8 [[X]], i8 noundef [[Y]]) #[[ATTR0]]
+; CHECK-NEXT:    [[C0:%.*]] = call i8 @baz(i8 noundef [[X]], i8 noundef [[Y]]) #[[ATTR3:[0-9]+]]
+; CHECK-NEXT:    [[R:%.*]] = call i8 @buz(i8 [[C0]], i8 [[C1]])
+; CHECK-NEXT:    ret i8 [[R]]
+;
+  %c1 = call i8 @baz(i8 %x, i8 noundef %y) readnone
+  %c0 = call i8 @baz(i8 noundef %x, i8 noundef %y) readnone strictfp
+  %r = call i8 @buz(i8 %c0, i8 %c1)
+  ret i8 %r
+
+}
+
+define i8 @diff_parent_combine_diff_attrs_todo(i1 %c, i8 %x, i8 %y) {
+; CHECK-LABEL: define i8 @diff_parent_combine_diff_attrs_todo(
+; CHECK-SAME: i1 [[C:%.*]], i8 [[X:%.*]], i8 [[Y:%.*]]) {
+; CHECK-NEXT:    [[C1:%.*]] = call i8 @baz(i8 [[X]], i8 noundef [[Y]]) #[[ATTR0]]
+; CHECK-NEXT:    br i1 [[C]], label %[[T:.*]], label %[[F:.*]]
+; CHECK:       [[T]]:
+; CHECK-NEXT:    [[C0:%.*]] = call i8 @baz(i8 noundef [[X]], i8 noundef [[Y]]) #[[ATTR4:[0-9]+]]
+; CHECK-NEXT:    [[R:%.*]] = call i8 @buz(i8 [[C0]], i8 [[C1]])
+; CHECK-NEXT:    ret i8 [[R]]
+; CHECK:       [[F]]:
+; CHECK-NEXT:    [[R2:%.*]] = add i8 [[C1]], 4
+; CHECK-NEXT:    ret i8 [[R2]]
+;
+  %c1 = call i8 @baz(i8 %x, i8 noundef %y) readnone
+  br i1 %c, label %T, label %F
+T:
+  %c0 = call i8 @baz(i8 noundef %x, i8 noundef %y) readnone optnone noinline
+  %r = call i8 @buz(i8 %c0, i8 %c1)
+  ret i8 %r
+F:
+  %r2 = add i8 %c1, 4
+  ret i8 %r2
+}
+
+define i8 @diff_parent_combine_diff_attrs_fail(i1 %c, i8 %x, i8 %y) {
+; CHECK-LABEL: define i8 @diff_parent_combine_diff_attrs_fail(
+; CHECK-SAME: i1 [[C:%.*]], i8 [[X:%.*]], i8 [[Y:%.*]]) {
+; CHECK-NEXT:    [[C1:%.*]] = call i8 @baz(i8 [[X]], i8 noundef [[Y]]) #[[ATTR0]]
+; CHECK-NEXT:    br i1 [[C]], label %[[T:.*]], label %[[F:.*]]
+; CHECK:       [[T]]:
+; CHECK-NEXT:    [[C0:%.*]] = call i8 @baz(i8 noundef [[X]], i8 noundef [[Y]]) #[[ATTR3]]
+; CHECK-NEXT:    [[R:%.*]] = call i8 @buz(i8 [[C0]], i8 [[C1]])
+; CHECK-NEXT:    ret i8 [[R]]
+; CHECK:       [[F]]:
+; CHECK-NEXT:    [[R2:%.*]] = add i8 [[C1]], 4
+; CHECK-NEXT:    ret i8 [[R2]]
+;
+  %c1 = call i8 @baz(i8 %x, i8 noundef %y) readnone
+  br i1 %c, label %T, label %F
+T:
+  %c0 = call i8 @baz(i8 noundef %x, i8 noundef %y) readnone strictfp
+  %r = call i8 @buz(i8 %c0, i8 %c1)
+  ret i8 %r
+F:
+  %r2 = add i8 %c1, 4
+  ret i8 %r2
+}
+
+;.
+; CHECK: attributes #[[ATTR0]] = { memory(none) }
+; CHECK: attributes #[[ATTR1]] = { memory(read) }
+; CHECK: attributes #[[ATTR2]] = { alwaysinline memory(none) }
+; CHECK: attributes #[[ATTR3]] = { strictfp memory(none) }
+; CHECK: attributes #[[ATTR4]] = { noinline optnone memory(none) }
+;.

>From 2fbda15fd4c521c9a79f8362ceb1941b3a72730f Mon Sep 17 00:00:00 2001
From: Noah Goldstein <goldstein.w.n at gmail.com>
Date: Wed, 2 Oct 2024 15:53:17 -0500
Subject: [PATCH 2/3] [EarlyCSE] De-Duplicate callsites with differing attrs

We only do this if the attributes of the two callsites are compatible
(intersectable) which is probably not in fact necessary.
---
 llvm/include/llvm/IR/Instruction.h            |  3 ++-
 llvm/lib/IR/Instruction.cpp                   |  5 ++--
 llvm/lib/Transforms/Scalar/EarlyCSE.cpp       | 23 +++++++++++++++++--
 .../EarlyCSE/replace-calls-def-attrs.ll       | 23 ++++++++-----------
 4 files changed, 35 insertions(+), 19 deletions(-)

diff --git a/llvm/include/llvm/IR/Instruction.h b/llvm/include/llvm/IR/Instruction.h
index 61dba265dc948b..c6084fa762be08 100644
--- a/llvm/include/llvm/IR/Instruction.h
+++ b/llvm/include/llvm/IR/Instruction.h
@@ -876,7 +876,8 @@ class Instruction : public User,
   /// Return true if the specified instruction is exactly identical to the
   /// current one. This means that all operands match and any extra information
   /// (e.g. load is volatile) agree.
-  bool isIdenticalTo(const Instruction *I) const LLVM_READONLY;
+  bool isIdenticalTo(const Instruction *I,
+                     bool IntersectAttrs = false) const LLVM_READONLY;
 
   /// This is like isIdenticalTo, except that it ignores the
   /// SubclassOptionalData flags, which may specify conditions under which the
diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp
index eec751078698b2..d9b47351bf543d 100644
--- a/llvm/lib/IR/Instruction.cpp
+++ b/llvm/lib/IR/Instruction.cpp
@@ -862,8 +862,9 @@ bool Instruction::hasSameSpecialState(const Instruction *I2,
   return true;
 }
 
-bool Instruction::isIdenticalTo(const Instruction *I) const {
-  return isIdenticalToWhenDefined(I) &&
+bool Instruction::isIdenticalTo(const Instruction *I,
+                                bool IntersectAttrs) const {
+  return isIdenticalToWhenDefined(I, IntersectAttrs) &&
          SubclassOptionalData == I->SubclassOptionalData;
 }
 
diff --git a/llvm/lib/Transforms/Scalar/EarlyCSE.cpp b/llvm/lib/Transforms/Scalar/EarlyCSE.cpp
index cf11f5bc885a75..7d482a7d52385a 100644
--- a/llvm/lib/Transforms/Scalar/EarlyCSE.cpp
+++ b/llvm/lib/Transforms/Scalar/EarlyCSE.cpp
@@ -362,7 +362,7 @@ static bool isEqualImpl(SimpleValue LHS, SimpleValue RHS) {
 
   if (LHSI->getOpcode() != RHSI->getOpcode())
     return false;
-  if (LHSI->isIdenticalToWhenDefined(RHSI)) {
+  if (LHSI->isIdenticalToWhenDefined(RHSI, /*IntersectAttrs=*/true)) {
     // Convergent calls implicitly depend on the set of threads that is
     // currently executing, so conservatively return false if they are in
     // different basic blocks.
@@ -551,7 +551,7 @@ bool DenseMapInfo<CallValue>::isEqual(CallValue LHS, CallValue RHS) {
   if (LHSI->isConvergent() && LHSI->getParent() != RHSI->getParent())
     return false;
 
-  return LHSI->isIdenticalTo(RHSI);
+  return LHSI->isIdenticalTo(RHSI, /*IntersectAttrs=*/true);
 }
 
 //===----------------------------------------------------------------------===//
@@ -1534,6 +1534,13 @@ bool EarlyCSE::processNode(DomTreeNode *Node) {
           LLVM_DEBUG(dbgs() << "Skipping due to debug counter\n");
           continue;
         }
+        if (auto *CB = dyn_cast<CallBase>(V)) {
+          bool Success = CB->tryIntersectAttributes(cast<CallBase>(&Inst));
+          assert(Success && "Failed to intersect attributes in callsites that "
+                            "passed identical check");
+          // For NDEBUG Compile.
+          (void)Success;
+        }
         combineIRFlags(Inst, V);
         Inst.replaceAllUsesWith(V);
         salvageKnowledge(&Inst, &AC);
@@ -1632,6 +1639,18 @@ bool EarlyCSE::processNode(DomTreeNode *Node) {
           LLVM_DEBUG(dbgs() << "Skipping due to debug counter\n");
           continue;
         }
+        // We probably don't need a complete itersection of attrs between
+        // InVal.first and Inst.
+        // We only CSE readonly functions that have the same memory state and
+        // its not clear that any non-return *callsite* attribute can create
+        // side-effects. Likewise this implies when checking equality of
+        // callsite for CSEing, we can probably ignore all non-return attrs.
+        bool Success = cast<CallBase>(InVal.first)
+                           ->tryIntersectAttributes(cast<CallBase>(&Inst));
+        assert(Success && "Failed to intersect attributes in callsites that "
+                          "passed identical check");
+        // For NDEBUG Compile.
+        (void)Success;
         if (!Inst.use_empty())
           Inst.replaceAllUsesWith(InVal.first);
         salvageKnowledge(&Inst, &AC);
diff --git a/llvm/test/Transforms/EarlyCSE/replace-calls-def-attrs.ll b/llvm/test/Transforms/EarlyCSE/replace-calls-def-attrs.ll
index 4e35c18cd8901c..cfc05c3da38c1b 100644
--- a/llvm/test/Transforms/EarlyCSE/replace-calls-def-attrs.ll
+++ b/llvm/test/Transforms/EarlyCSE/replace-calls-def-attrs.ll
@@ -10,9 +10,8 @@ declare i8 @buz.ptr(ptr, ptr)
 define i8 @same_parent_combine_diff_attrs(i8 %x, i8 %y) {
 ; CHECK-LABEL: define i8 @same_parent_combine_diff_attrs(
 ; CHECK-SAME: i8 [[X:%.*]], i8 [[Y:%.*]]) {
-; CHECK-NEXT:    [[C2:%.*]] = call i8 @baz(i8 noundef [[X]], i8 noundef [[Y]]) #[[ATTR0:[0-9]+]]
-; CHECK-NEXT:    [[C1:%.*]] = call i8 @baz(i8 [[X]], i8 noundef [[Y]]) #[[ATTR0]]
-; CHECK-NEXT:    [[R:%.*]] = call i8 @buz(i8 [[C1]], i8 [[C2]])
+; CHECK-NEXT:    [[C1:%.*]] = call i8 @baz(i8 [[X]], i8 noundef [[Y]]) #[[ATTR0:[0-9]+]]
+; CHECK-NEXT:    [[R:%.*]] = call i8 @buz(i8 [[C1]], i8 [[C1]])
 ; CHECK-NEXT:    ret i8 [[R]]
 ;
   %c1 = call i8 @baz(i8 noundef %x, i8 noundef %y) readnone
@@ -25,9 +24,8 @@ define i8 @same_parent_combine_diff_attrs(i8 %x, i8 %y) {
 define i8 @same_parent_combine_diff_attrs_needs_intersect(i8 %x, i8 %y) {
 ; CHECK-LABEL: define i8 @same_parent_combine_diff_attrs_needs_intersect(
 ; CHECK-SAME: i8 [[X:%.*]], i8 [[Y:%.*]]) {
-; CHECK-NEXT:    [[C1:%.*]] = call nonnull ptr @baz.ptr(i8 noundef [[X]], i8 noundef [[Y]]) #[[ATTR0]]
-; CHECK-NEXT:    [[C0:%.*]] = call ptr @baz.ptr(i8 [[X]], i8 noundef [[Y]]) #[[ATTR0]]
-; CHECK-NEXT:    [[R:%.*]] = call i8 @buz.ptr(ptr [[C0]], ptr [[C1]])
+; CHECK-NEXT:    [[C1:%.*]] = call ptr @baz.ptr(i8 [[X]], i8 noundef [[Y]]) #[[ATTR0]]
+; CHECK-NEXT:    [[R:%.*]] = call i8 @buz.ptr(ptr [[C1]], ptr [[C1]])
 ; CHECK-NEXT:    ret i8 [[R]]
 ;
   %c1 = call nonnull ptr @baz.ptr(i8 noundef %x, i8 noundef %y) readnone
@@ -40,9 +38,8 @@ define i8 @same_parent_combine_diff_attrs_needs_intersect(i8 %x, i8 %y) {
 define i8 @same_parent_combine_diff_attrs_needs_intersect2(i8 %x, i8 %y) {
 ; CHECK-LABEL: define i8 @same_parent_combine_diff_attrs_needs_intersect2(
 ; CHECK-SAME: i8 [[X:%.*]], i8 [[Y:%.*]]) {
-; CHECK-NEXT:    [[C1:%.*]] = call nonnull ptr @baz.ptr(i8 noundef [[X]], i8 noundef [[Y]]) #[[ATTR1:[0-9]+]]
-; CHECK-NEXT:    [[C0:%.*]] = call ptr @baz.ptr(i8 [[X]], i8 noundef [[Y]]) #[[ATTR1]]
-; CHECK-NEXT:    [[R:%.*]] = call i8 @buz.ptr(ptr [[C0]], ptr [[C1]])
+; CHECK-NEXT:    [[C1:%.*]] = call ptr @baz.ptr(i8 [[X]], i8 noundef [[Y]]) #[[ATTR1:[0-9]+]]
+; CHECK-NEXT:    [[R:%.*]] = call i8 @buz.ptr(ptr [[C1]], ptr [[C1]])
 ; CHECK-NEXT:    ret i8 [[R]]
 ;
   %c1 = call nonnull ptr @baz.ptr(i8 noundef %x, i8 noundef %y) readonly
@@ -55,9 +52,8 @@ define i8 @same_parent_combine_diff_attrs_needs_intersect2(i8 %x, i8 %y) {
 define i8 @same_parent_combine_diff_attrs_really_needs_intersect(i8 %x, i8 %y) {
 ; CHECK-LABEL: define i8 @same_parent_combine_diff_attrs_really_needs_intersect(
 ; CHECK-SAME: i8 [[X:%.*]], i8 [[Y:%.*]]) {
-; CHECK-NEXT:    [[C1:%.*]] = call nonnull ptr @baz.ptr(i8 noundef [[X]], i8 noundef [[Y]]) #[[ATTR1]]
-; CHECK-NEXT:    [[C0:%.*]] = call ptr @baz.ptr(i8 [[X]], i8 noundef [[Y]]) #[[ATTR1]]
-; CHECK-NEXT:    [[R:%.*]] = call i8 @buz.ptr(ptr [[C0]], ptr noundef [[C1]])
+; CHECK-NEXT:    [[C1:%.*]] = call ptr @baz.ptr(i8 [[X]], i8 noundef [[Y]]) #[[ATTR1]]
+; CHECK-NEXT:    [[R:%.*]] = call i8 @buz.ptr(ptr [[C1]], ptr noundef [[C1]])
 ; CHECK-NEXT:    ret i8 [[R]]
 ;
   %c1 = call nonnull ptr @baz.ptr(i8 noundef %x, i8 noundef %y) readonly
@@ -128,8 +124,7 @@ define i8 @diff_parent_combine_diff_attrs_preserves_return_attrs(i1 %c, i8 %x, i
 ; CHECK-NEXT:    [[C1:%.*]] = call nonnull ptr @baz.ptr(i8 [[X]], i8 noundef [[Y]]) #[[ATTR1]]
 ; CHECK-NEXT:    br i1 [[C]], label %[[T:.*]], label %[[F:.*]]
 ; CHECK:       [[T]]:
-; CHECK-NEXT:    [[C0:%.*]] = call nonnull ptr @baz.ptr(i8 noundef [[X]], i8 noundef [[Y]]) #[[ATTR1]]
-; CHECK-NEXT:    [[R:%.*]] = call i8 @buz.ptr(ptr [[C0]], ptr noundef [[C1]])
+; CHECK-NEXT:    [[R:%.*]] = call i8 @buz.ptr(ptr [[C1]], ptr noundef [[C1]])
 ; CHECK-NEXT:    ret i8 [[R]]
 ; CHECK:       [[F]]:
 ; CHECK-NEXT:    ret i8 9

>From ada9f69a1134ff83441f387dfdd0bae9e84856cc Mon Sep 17 00:00:00 2001
From: Noah Goldstein <goldstein.w.n at gmail.com>
Date: Thu, 3 Oct 2024 11:33:53 -0500
Subject: [PATCH 3/3] Improve comment

---
 llvm/lib/Transforms/Scalar/EarlyCSE.cpp | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/llvm/lib/Transforms/Scalar/EarlyCSE.cpp b/llvm/lib/Transforms/Scalar/EarlyCSE.cpp
index 7d482a7d52385a..27593de0cdb955 100644
--- a/llvm/lib/Transforms/Scalar/EarlyCSE.cpp
+++ b/llvm/lib/Transforms/Scalar/EarlyCSE.cpp
@@ -1639,12 +1639,15 @@ bool EarlyCSE::processNode(DomTreeNode *Node) {
           LLVM_DEBUG(dbgs() << "Skipping due to debug counter\n");
           continue;
         }
-        // We probably don't need a complete itersection of attrs between
-        // InVal.first and Inst.
-        // We only CSE readonly functions that have the same memory state and
-        // its not clear that any non-return *callsite* attribute can create
-        // side-effects. Likewise this implies when checking equality of
-        // callsite for CSEing, we can probably ignore all non-return attrs.
+        // NB: Intersection of attrs between InVal.first and Inst is overly
+        // conservative. Since we only CSE readonly functions that have the same
+        // memory state, we can preserve (or possibly in some cases combine)
+        // more attributes. Likewise this implies when checking equality of
+        // callsite for CSEing, we can probably ignore more attributes.
+        // Generally poison generating attributes need to be handled with more
+        // care as they can create *new* UB if preserved/combined and violated.
+        // Attributes that imply immediate UB on the otherhand would have been
+        // violated either way.
         bool Success = cast<CallBase>(InVal.first)
                            ->tryIntersectAttributes(cast<CallBase>(&Inst));
         assert(Success && "Failed to intersect attributes in callsites that "



More information about the llvm-commits mailing list