[PATCH] D112870: [MergeFunctions] Don't merge functions that use @llvm.type.checked.load instrinsics

[Kuba (Brecka) Mracek via Phabricator via llvm-commits]

kubamracek created this revision.
kubamracek added reviewers: pcc, fhahn, MaskRay, davide.
Herald added subscribers: ormris, hiraditya.
kubamracek requested review of this revision.
Herald added a project: LLVM.

See attached testcase that shows the miscompile / mis-merge. Comparing functions for equality is done via FunctionComparator, which only compares whether the two functions will produce the same machine code:

  /// FunctionComparator - Compares two functions to determine whether or not
  /// they will generate machine code with the same behaviour. 

Namely, this means that metadata attachments and metadata arguments of instructions are ignored, and that's a problem for @llvm.type.checked.load which does actually have an important meaningful metadata reference (type id).

This PR opts out functions that use @llvm.type.checked.load from merging.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112870

Files:
  llvm/lib/Transforms/IPO/MergeFunctions.cpp
  llvm/test/Transforms/MergeFunc/merge-type-metadata.ll


Index: llvm/test/Transforms/MergeFunc/merge-type-metadata.ll
===================================================================
--- /dev/null
+++ llvm/test/Transforms/MergeFunc/merge-type-metadata.ll
@@ -0,0 +1,23 @@
+; RUN: opt -S -mergefunc < %s | FileCheck %s
+
+; CHECK: @call_vfunc1
+define internal i32 @call_vfunc1(i8* %vtable) unnamed_addr {
+  %a = tail call { i8*, i1 } @llvm.type.checked.load(i8* %vtable, i32 0, metadata !"vtype1")
+  ret i32 0
+}
+
+; CHECK: @call_vfunc2
+define internal i32 @call_vfunc2(i8* %vtable) unnamed_addr {
+  %a = tail call { i8*, i1 } @llvm.type.checked.load(i8* %vtable, i32 0, metadata !"vtype2")
+  ret i32 0
+}
+
+declare { i8*, i1 } @llvm.type.checked.load(i8*, i32, metadata)
+
+define void @main() {
+  %a = call i32 @call_vfunc1(i8* null)
+  ; CHECK: call i32 @call_vfunc1
+  %b = call i32 @call_vfunc2(i8* null)
+  ; CHECK: call i32 @call_vfunc2
+  ret void
+}
Index: llvm/lib/Transforms/IPO/MergeFunctions.cpp
===================================================================
--- llvm/lib/Transforms/IPO/MergeFunctions.cpp
+++ llvm/lib/Transforms/IPO/MergeFunctions.cpp
@@ -403,19 +403,42 @@
 #endif
 
 /// Check whether \p F is eligible for function merging.
-static bool isEligibleForMerging(Function &F) {
-  return !F.isDeclaration() && !F.hasAvailableExternallyLinkage();
+static bool
+isEligibleForMerging(Function &F,
+                     std::set<Function *> TypeCheckedLoadUsingFuncs) {
+  if (F.isDeclaration() || F.hasAvailableExternallyLinkage())
+    return false;
+
+  if (TypeCheckedLoadUsingFuncs.count(&F))
+    return false;
+
+  return true;
 }
 
 bool MergeFunctions::runOnModule(Module &M) {
   bool Changed = false;
 
+  // Functions that use @llvm.type.checked.load intrinsic calls are not eligible
+  // for merging because the type metadata is not considered meaningful when
+  // comparing functions for equality. Let's find such functions.
+  Function *TypeCheckedLoadFunc =
+      M.getFunction(Intrinsic::getName(Intrinsic::type_checked_load));
+  std::set<Function *> TypeCheckedLoadUsingFuncs;
+  if (TypeCheckedLoadFunc) {
+    for (auto U : TypeCheckedLoadFunc->users()) {
+      auto CI = dyn_cast<CallInst>(U);
+      if (!CI)
+        continue;
+      TypeCheckedLoadUsingFuncs.insert(CI->getFunction());
+    }
+  }
+
   // All functions in the module, ordered by hash. Functions with a unique
   // hash value are easily eliminated.
   std::vector<std::pair<FunctionComparator::FunctionHash, Function *>>
     HashedFuncs;
   for (Function &Func : M) {
-    if (isEligibleForMerging(Func)) {
+    if (isEligibleForMerging(Func, TypeCheckedLoadUsingFuncs)) {
       HashedFuncs.push_back({FunctionComparator::functionHash(Func), &Func});
     }
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D112870.383589.patch
Type: text/x-patch
Size: 2743 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211030/0a97600c/attachment.bin>