[llvm] Fix: Distinguish CFI Metadata Checks in MergeFunctions Pass (PR #65963)

[Oskar Wirga via llvm-commits]

https://github.com/oskarwirga updated https://github.com/llvm/llvm-project/pull/65963

>From 9e10c30dd4a969924d3804c34f833120d989b624 Mon Sep 17 00:00:00 2001
From: Oskar Wirga <10386631+oskarwirga at users.noreply.github.com>
Date: Thu, 21 Sep 2023 17:56:01 -0700
Subject: [PATCH 1/2] Fix: Distinguish CFI Metadata Checks in MergeFunctions
 Pass

---
 llvm/lib/Transforms/IPO/MergeFunctions.cpp    | 28 +++++++++++++-
 .../MergeFunc/cfi-function-merging.ll         | 38 +++++++++++++++++++
 .../MergeFunc/merge-fp-intrinsics.ll          | 22 +++++++++++
 3 files changed, 87 insertions(+), 1 deletion(-)
 create mode 100644 llvm/test/Transforms/MergeFunc/cfi-function-merging.ll
 create mode 100644 llvm/test/Transforms/MergeFunc/merge-fp-intrinsics.ll

diff --git a/llvm/lib/Transforms/IPO/MergeFunctions.cpp b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
index 312a8df440bf1e3..35d33811fbb6b6e 100644
--- a/llvm/lib/Transforms/IPO/MergeFunctions.cpp
+++ b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
@@ -375,9 +375,35 @@ bool MergeFunctions::doFunctionalCheck(std::vector<WeakTrackingVH> &Worklist) {
 }
 #endif
 
+/// Check whether \p F has an intrinsic which references
+/// distinct metadata as an operand. The most common
+/// instance of this would be CFI checks for function-local types.
+static bool hasDistinctMetadataIntrinsic(const Function &F) {
+  for (const BasicBlock &BB : F) {
+    for (const Instruction &I : BB.instructionsWithoutDebug()) {
+      if (!isa<IntrinsicInst>(&I))
+        continue;
+
+      for (unsigned i = 0, e = I.getNumOperands(); i != e; ++i) {
+        MetadataAsValue *MDL = dyn_cast<MetadataAsValue>(I.getOperand(i));
+        if (!MDL)
+          continue;
+        MDNode *node = dyn_cast<MDNode>(MDL->getMetadata());
+        if (!node)
+          continue;
+        if (!node->isDistinct())
+          continue;
+        return true;
+      }
+    }
+  }
+  return false;
+}
+
 /// Check whether \p F is eligible for function merging.
 static bool isEligibleForMerging(Function &F) {
-  return !F.isDeclaration() && !F.hasAvailableExternallyLinkage();
+  return !F.isDeclaration() && !F.hasAvailableExternallyLinkage() &&
+         !hasDistinctMetadataIntrinsic(F);
 }
 
 bool MergeFunctions::runOnModule(Module &M) {
diff --git a/llvm/test/Transforms/MergeFunc/cfi-function-merging.ll b/llvm/test/Transforms/MergeFunc/cfi-function-merging.ll
new file mode 100644
index 000000000000000..a3d81f156f753b4
--- /dev/null
+++ b/llvm/test/Transforms/MergeFunc/cfi-function-merging.ll
@@ -0,0 +1,38 @@
+;; Check the cases involving internal CFI instrumented functions where we do not expect functions to be merged.
+; RUN: opt -S -passes=mergefunc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-none-linux-android28"
+
+; Function Attrs: nocallback nofree nosync nounwind readnone speculatable willreturn
+declare i1 @llvm.type.test(ptr, metadata) #6
+
+define internal void @A__on_zero_sharedEv(ptr noundef nonnull align 8 dereferenceable(32) %this) {
+; CHECK-LABEL: @A__on_zero_sharedEv
+entry:
+  %this.addr = alloca ptr, align 8
+  store ptr %this, ptr %this.addr, align 8
+  %this1 = load ptr, ptr %this.addr, align 8
+  %vtable = load ptr, ptr %this1, align 8
+  %0 = call i1 @llvm.type.test(ptr %vtable, metadata !11), !nosanitize !47
+  ret void
+}
+
+; Function Attrs: mustprogress noinline nounwind optnone uwtable
+define internal void @B__on_zero_sharedEv(ptr noundef nonnull align 8 dereferenceable(32) %this) {
+; CHECK-LABEL: @B__on_zero_sharedEv
+entry:
+  %this.addr = alloca ptr, align 8
+  store ptr %this, ptr %this.addr, align 8
+  %this1 = load ptr, ptr %this.addr, align 8
+  %vtable = load ptr, ptr %this1, align 8
+  %0 = call i1 @llvm.type.test(ptr %vtable, metadata !22), !nosanitize !47
+  ret void
+}
+
+!10 = !{i64 16, !11}
+!11 = distinct !{}
+!21 = !{i64 16, !22}
+!22 = distinct !{}
+!47 = !{}
diff --git a/llvm/test/Transforms/MergeFunc/merge-fp-intrinsics.ll b/llvm/test/Transforms/MergeFunc/merge-fp-intrinsics.ll
new file mode 100644
index 000000000000000..21a8f0242aeb485
--- /dev/null
+++ b/llvm/test/Transforms/MergeFunc/merge-fp-intrinsics.ll
@@ -0,0 +1,22 @@
+;; Make sure internal constrained FP intrinsics still merge properly
+; RUN: opt -passes=mergefunc -S < %s | FileCheck %s
+; RUN: opt -passes=mergefunc -S < %s | FileCheck -check-prefix=MERGE %s
+
+target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
+
+declare float @llvm.experimental.constrained.fadd.f32(float, float, metadata, metadata)
+
+!round.dynamic = !{}
+!fpexcept.strict = !{}
+
+define internal float @func1(float %a, float %b) {
+; CHECK-LABEL: define internal float @func1(float %a, float %b)
+  %result = call float @llvm.experimental.constrained.fadd.f32(float %a, float %b, metadata !"round.dynamic", metadata !"fpexcept.strict")
+  ret float %result
+}
+
+define internal float @func2(float %a, float %b) {
+; MERGE-NOT: define internal float @func2(float %a, float %b)
+  %result = call float @llvm.experimental.constrained.fadd.f32(float %a, float %b, metadata !"round.dynamic", metadata !"fpexcept.strict")
+  ret float %result
+}

>From c13a50b7b4f6e030286652660bbbcfd820706fc0 Mon Sep 17 00:00:00 2001
From: Oskar Wirga <10386631+oskarwirga at users.noreply.github.com>
Date: Thu, 21 Sep 2023 17:56:12 -0700
Subject: [PATCH 2/2] NFC: clang-format

---
 llvm/lib/Transforms/IPO/MergeFunctions.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/llvm/lib/Transforms/IPO/MergeFunctions.cpp b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
index 35d33811fbb6b6e..f5a24ee1c81b3c6 100644
--- a/llvm/lib/Transforms/IPO/MergeFunctions.cpp
+++ b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
@@ -430,7 +430,7 @@ bool MergeFunctions::runOnModule(Module &M) {
     // If the hash value matches the previous value or the next one, we must
     // consider merging it. Otherwise it is dropped and never considered again.
     if ((I != S && std::prev(I)->first == I->first) ||
-        (std::next(I) != IE && std::next(I)->first == I->first) ) {
+        (std::next(I) != IE && std::next(I)->first == I->first)) {
       Deferred.push_back(WeakTrackingVH(I->second));
     }
   }