[llvm] Handle dso_local_equivalent in FunctionComparator

[leonardchan]

Summary

This addresses https://github.com/llvm/llvm-project/issues/51066.

Prior to this, dso_local_equivalent would lead to an llvm_unreachable in a switch in the FunctionComparator. This adds a conservative case in that switch that just compares the underlying functions.

[leonardchan]

@phosek feel free to add who you think is appropriate for reviewing this.

This is a conservative comparison of dso_local_equivalents where two are the same only if they point to the same underlying function. It could be argued that a dso_local_equivalent @func could be "the same" as just @func, but I think that can be argued/implemented separately. This is mainly for addressing the crash in pr51066.

[nikic]

Please drop the clang test and use update_test_checks.py for LLVM tests.

llvm/test/Transforms/MergeFunc/dso_local_equivalent_merged.ll

2

Based on your check lines, it doesn't look like the functions actually get merged? You probably need to add some dummy instructions to pass the instruction threshold, or use internal linkage.

[leonardchan]

llvm/test/Transforms/MergeFunc/dso_local_equivalent_merged.ll

2

Woops. Yeah you're right. Updating the tests.

[phosek]

LGTM

[leonardchan]

Will wait until EOD before committing this unless others have more comments.

llvm/lib/Transforms/Utils/FunctionComparator.cpp

if (LBA->getFunction() == RBA->getFunction()) {
       // literally the same pointer, they must respectively be the left and
       // right functions.
       assert(LBA->getFunction() == FnL && RBA->getFunction() == FnR);
       // cmpValues will tell us if these are equivalent BasicBlocks, in the
       // context of their respective functions.
       return cmpValues(LBA->getBasicBlock(), RBA->getBasicBlock());
     }
   }
+  case Value::DSOLocalEquivalentVal: {
+    // dso_local_equivalent is functionally equivalent to whatever it points to.
+    // This means the behavior of the IR should be the exact same as if the
+    // function was referenced directly rather than through a
+    // dso_local_equivalent.
+    const auto *LEquiv = cast<DSOLocalEquivalent>(L);
+    const auto *REquiv = cast<DSOLocalEquivalent>(R);
+    return cmpGlobalValues(LEquiv->getGlobalValue(), REquiv->getGlobalValue());
+  }
   default: // Unknown constant, abort.
     LLVM_DEBUG(dbgs() << "Looking at valueID " << L->getValueID() << "\n");
     llvm_unreachable("Constant ValueID not recognized.");
     return -1;
   }
 }
 
 int FunctionComparator::cmpGlobalValues(GlobalValue *L, GlobalValue *R) const {

llvm/test/Transforms/MergeFunc/dso_local_equivalent_merged.ll

+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+;; Check the cases involving dso_local_equivalent where we do expect functions to be merged.
+; RUN: opt -S -mergefunc < %s | FileCheck %s
+
+; CHECK-NOT: @b
+
+@x = constant { i32 ()*, i32 ()* } { i32 ()* @a, i32 ()* @b }
+; CHECK: { i32 ()* @a, i32 ()* @a }
+
+define i32 @func() {
+; CHECK-LABEL: @func(
+; CHECK-NEXT:    ret i32 0
+;
+  ret i32 0
+}
+
+define internal i32 @a() unnamed_addr {
+; CHECK-LABEL: @a(
+; CHECK-NEXT:    [[TMP1:%.*]] = call i32 dso_local_equivalent @func()
+; CHECK-NEXT:    ret i32 [[TMP1]]
+;
+  %1 = call i32 dso_local_equivalent @func()
+  ret i32 %1
+}
+
+define internal i32 @b() unnamed_addr {
+  %1 = call i32 dso_local_equivalent @func()
+  ret i32 %1
+}

llvm/test/Transforms/MergeFunc/dso_local_equivalent_unmerged.ll

+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+;; Check the cases involving dso_local_equivalent where we do not expect functions to be merged.
+; RUN: opt -S -mergefunc < %s | FileCheck %s
+
+@x = constant { i32 ()*, i32 ()* } { i32 ()* @a, i32 ()* @b }
+; CHECK: { i32 ()* @a, i32 ()* @b }
+
+@x2 = constant { i32 ()*, i32 ()* } { i32 ()* @c, i32 ()* @d }
+; CHECK: { i32 ()* @c, i32 ()* @d }
+
+;; func1 and func2 are different functions.
+declare i32 @func1()
+define i32 @func2() {
+; CHECK-LABEL: @func2(
+; CHECK-NEXT:    ret i32 0
+;
+  ret i32 0
+}
+
+define internal i32 @a() unnamed_addr {
+; CHECK-LABEL: @a(
+; CHECK-NEXT:    [[TMP1:%.*]] = call i32 dso_local_equivalent @func1()
+; CHECK-NEXT:    ret i32 [[TMP1]]
+;
+  %1 = call i32 dso_local_equivalent @func1()
+  ret i32 %1
+}
+
+define internal i32 @b() unnamed_addr {
+; CHECK-LABEL: @b(
+; CHECK-NEXT:    [[TMP1:%.*]] = call i32 dso_local_equivalent @func2()
+; CHECK-NEXT:    ret i32 [[TMP1]]
+;
+  %1 = call i32 dso_local_equivalent @func2()
+  ret i32 %1
+}
+
+;; func3 and func4 have the same body and signature but do not have merged
+;; callers because they are different functions.
+define i32 @func3() {
+; CHECK-LABEL: @func3(
+; CHECK-NEXT:    ret i32 0
+;
+  ret i32 0
+}
+define i32 @func4() {
+; CHECK-LABEL: @func4(
+; CHECK-NEXT:    ret i32 0
+;
+  ret i32 0
+}
+
+define internal i32 @c() unnamed_addr {
+; CHECK-LABEL: @c(
+; CHECK-NEXT:    [[TMP1:%.*]] = call i32 dso_local_equivalent @func3()
+; CHECK-NEXT:    ret i32 [[TMP1]]
+;
+  %1 = call i32 dso_local_equivalent @func3()
+  ret i32 %1
+}
+
+define internal i32 @d() unnamed_addr {
+; CHECK-LABEL: @d(
+; CHECK-NEXT:    [[TMP1:%.*]] = call i32 dso_local_equivalent @func4()
+; CHECK-NEXT:    ret i32 [[TMP1]]
+;
+  %1 = call i32 dso_local_equivalent @func4()
+  ret i32 %1
+}