[MergeFunc] Don't assume constant metadata operands

We should not call mdconst::extract, unless we know that the
metadata in question is ConstantAsMetadata.

For now we consider all other metadata as equal. The noalias test
shows that this is not correct, but at least it doesn't crash
anymore.

diff --git a/llvm/include/llvm/Transforms/Utils/FunctionComparator.h b/llvm/include/llvm/Transforms/Utils/FunctionComparator.h
index 400b9fa..78761fc 100644 (file)
--- a/llvm/include/llvm/Transforms/Utils/FunctionComparator.h
+++ b/llvm/include/llvm/Transforms/Utils/FunctionComparator.h
@@ -332,7 +332,8 @@ private:
   int cmpOrderings(AtomicOrdering L, AtomicOrdering R) const;
   int cmpInlineAsm(const InlineAsm *L, const InlineAsm *R) const;
   int cmpAttrs(const AttributeList L, const AttributeList R) const;
-  int cmpMetadata(const MDNode *L, const MDNode *R) const;
+  int cmpMDNode(const MDNode *L, const MDNode *R) const;
+  int cmpMetadata(const Metadata *L, const Metadata *R) const;
   int cmpInstMetadata(Instruction const *L, Instruction const *R) const;
   int cmpOperandBundlesSchema(const CallBase &LCS, const CallBase &RCS) const;
 

diff --git a/llvm/lib/Transforms/Utils/FunctionComparator.cpp b/llvm/lib/Transforms/Utils/FunctionComparator.cpp
index af8bc81..7fb6a74 100644 (file)
--- a/llvm/lib/Transforms/Utils/FunctionComparator.cpp
+++ b/llvm/lib/Transforms/Utils/FunctionComparator.cpp
@@ -157,7 +157,25 @@ int FunctionComparator::cmpAttrs(const AttributeList L,
   return 0;
 }
 
-int FunctionComparator::cmpMetadata(const MDNode *L, const MDNode *R) const {
+int FunctionComparator::cmpMetadata(const Metadata *L,
+                                    const Metadata *R) const {
+  // TODO: the following routine coerce the metadata contents into constants
+  // before comparison.
+  // It ignores any other cases, so that the metadata nodes are considered
+  // equal even though this is not correct.
+  // We should structurally compare the metadata nodes to be perfect here.
+  auto *CL = dyn_cast<ConstantAsMetadata>(L);
+  auto *CR = dyn_cast<ConstantAsMetadata>(R);
+  if (CL == CR)
+    return 0;
+  if (!CL)
+    return -1;
+  if (!CR)
+    return 1;
+  return cmpConstants(CL->getValue(), CR->getValue());
+}
+
+int FunctionComparator::cmpMDNode(const MDNode *L, const MDNode *R) const {
   if (L == R)
     return 0;
   if (!L)
@@ -172,23 +190,9 @@ int FunctionComparator::cmpMetadata(const MDNode *L, const MDNode *R) const {
   // function semantically.
   if (int Res = cmpNumbers(L->getNumOperands(), R->getNumOperands()))
     return Res;
-  for (size_t I = 0; I < L->getNumOperands(); ++I) {
-    // TODO: the following routine coerce the metadata contents into numbers
-    // before comparison.
-    // It ignores any other cases, so that the metadata nodes are considered
-    // equal even though this is not correct.
-    // We should structurally compare the metadata nodes to be perfect here.
-    ConstantInt *LLow = mdconst::extract<ConstantInt>(L->getOperand(I));
-    ConstantInt *RLow = mdconst::extract<ConstantInt>(R->getOperand(I));
-    if (LLow == RLow)
-      continue;
-    if (!LLow)
-      return -1;
-    if (!RLow)
-      return 1;
-    if (int Res = cmpAPInts(LLow->getValue(), RLow->getValue()))
+  for (size_t I = 0; I < L->getNumOperands(); ++I)
+    if (int Res = cmpMetadata(L->getOperand(I), R->getOperand(I)))
       return Res;
-  }
   return 0;
 }
 
@@ -209,7 +213,7 @@ int FunctionComparator::cmpInstMetadata(Instruction const *L,
     auto const [KeyR, MR] = MDR[I];
     if (int Res = cmpNumbers(KeyL, KeyR))
       return Res;
-    if (int Res = cmpMetadata(ML, MR))
+    if (int Res = cmpMDNode(ML, MR))
       return Res;
   }
   return 0;
@@ -645,8 +649,8 @@ int FunctionComparator::cmpOperations(const Instruction *L,
       if (int Res = cmpNumbers(CI->getTailCallKind(),
                                cast<CallInst>(R)->getTailCallKind()))
         return Res;
-    return cmpMetadata(L->getMetadata(LLVMContext::MD_range),
-                       R->getMetadata(LLVMContext::MD_range));
+    return cmpMDNode(L->getMetadata(LLVMContext::MD_range),
+                     R->getMetadata(LLVMContext::MD_range));
   }
   if (const InsertValueInst *IVI = dyn_cast<InsertValueInst>(L)) {
     ArrayRef<unsigned> LIndices = IVI->getIndices();

diff --git a/llvm/test/Transforms/MergeFunc/mergefunc-preserve-nonnull.ll b/llvm/test/Transforms/MergeFunc/mergefunc-preserve-nonnull.ll
index 12bb0e8..3481d53 100644 (file)
--- a/llvm/test/Transforms/MergeFunc/mergefunc-preserve-nonnull.ll
+++ b/llvm/test/Transforms/MergeFunc/mergefunc-preserve-nonnull.ll
@@ -28,8 +28,8 @@ define void @f2(ptr %0, ptr %1) {
   ret void
 }
 
-define void @f3(ptr %0, ptr %1) {
-; CHECK-LABEL: @f3(
+define void @noundef(ptr %0, ptr %1) {
+; CHECK-LABEL: @noundef(
 ; CHECK-NEXT:    [[TMP3:%.*]] = load ptr, ptr [[TMP1:%.*]], align 8, !noundef !0
 ; CHECK-NEXT:    store ptr [[TMP3]], ptr [[TMP0:%.*]], align 8
 ; CHECK-NEXT:    ret void
@@ -39,9 +39,20 @@ define void @f3(ptr %0, ptr %1) {
   ret void
 }
 
-define void @f4(ptr %0, ptr %1) {
-; CHECK-LABEL: @f4(
-; CHECK-NEXT:    tail call void @f3(ptr [[TMP0:%.*]], ptr [[TMP1:%.*]])
+define void @noalias_1(ptr %0, ptr %1) {
+; CHECK-LABEL: @noalias_1(
+; CHECK-NEXT:    [[TMP3:%.*]] = load ptr, ptr [[TMP1:%.*]], align 8, !noalias !1
+; CHECK-NEXT:    store ptr [[TMP3]], ptr [[TMP0:%.*]], align 8, !alias.scope !1
+; CHECK-NEXT:    ret void
+;
+  %3 = load ptr, ptr %1, align 8, !noalias !4
+  store ptr %3, ptr %0, align 8, !alias.scope !4
+  ret void
+}
+
+define void @noundef_dbg(ptr %0, ptr %1) {
+; CHECK-LABEL: @noundef_dbg(
+; CHECK-NEXT:    tail call void @noundef(ptr [[TMP0:%.*]], ptr [[TMP1:%.*]])
 ; CHECK-NEXT:    ret void
 ;
   %3 = load ptr, ptr %1, align 8, !noundef !0, !dbg !1
@@ -49,5 +60,22 @@ define void @f4(ptr %0, ptr %1) {
   ret void
 }
 
+; FIXME: This is merged despite different noalias metadata.
+define void @noalias_2(ptr %0, ptr %1) {
+; CHECK-LABEL: @noalias_2(
+; CHECK-NEXT:    tail call void @noalias_1(ptr [[TMP0:%.*]], ptr [[TMP1:%.*]])
+; CHECK-NEXT:    ret void
+;
+  %3 = load ptr, ptr %1, align 8, !noalias !7
+  store ptr %3, ptr %0, align 8, !alias.scope !7
+  ret void
+}
+
 !0 = !{}
 !1 = !{}
+!2 = !{!2}
+!3 = !{!3, !2}
+!4 = !{!3}
+!5 = !{!5}
+!6 = !{!6, !5}
+!7 = !{!6}