[llvm] 2f5fdbf - [MergeFunc] Don't assume constant metadata operands

Nikita Popov via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 23 09:35:02 PDT 2023


Author: Nikita Popov
Date: 2023-03-23T17:34:53+01:00
New Revision: 2f5fdbfab8c63047bd4ebef154258868065168b3

URL: https://github.com/llvm/llvm-project/commit/2f5fdbfab8c63047bd4ebef154258868065168b3
DIFF: https://github.com/llvm/llvm-project/commit/2f5fdbfab8c63047bd4ebef154258868065168b3.diff

LOG: [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.

Added: 
    

Modified: 
    llvm/include/llvm/Transforms/Utils/FunctionComparator.h
    llvm/lib/Transforms/Utils/FunctionComparator.cpp
    llvm/test/Transforms/MergeFunc/mergefunc-preserve-nonnull.ll

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Transforms/Utils/FunctionComparator.h b/llvm/include/llvm/Transforms/Utils/FunctionComparator.h
index 400b9faa94c1b..78761fc78fee8 100644
--- a/llvm/include/llvm/Transforms/Utils/FunctionComparator.h
+++ b/llvm/include/llvm/Transforms/Utils/FunctionComparator.h
@@ -332,7 +332,8 @@ class FunctionComparator {
   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 af8bc8126160e..7fb6a7415a6fe 100644
--- 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 12bb0e8b38425..3481d53b626fc 100644
--- 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 
diff erent 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}


        


More information about the llvm-commits mailing list