[llvm] 8325d46 - [MergeFuncs] Compare load instruction metadata

[Nikita Popov via llvm-commits]

Author: Ding Xiang Fei
Date: 2023-03-21T09:45:51+01:00
New Revision: 8325d46a4ab803c6eeff28bf1bdafb43287ce557

URL: https://github.com/llvm/llvm-project/commit/8325d46a4ab803c6eeff28bf1bdafb43287ce557
DIFF: https://github.com/llvm/llvm-project/commit/8325d46a4ab803c6eeff28bf1bdafb43287ce557.diff

LOG: [MergeFuncs] Compare load instruction metadata

MergeFuncs currently merges load instructions with differing
semantically-relevant metadata, e.g. a load that has !nonnull
with one that does not.

Update FunctionComparator to make sure that metadata of both
loads is the same. Alternatively, it would be possilbe to ignore
the metadata during comparison, and then drop it during merging.

Differential Revision: https://reviews.llvm.org/D144682

Added: 
    

Modified: 
    llvm/include/llvm/Transforms/Utils/FunctionComparator.h
    llvm/lib/Transforms/Utils/FunctionComparator.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Transforms/Utils/FunctionComparator.h b/llvm/include/llvm/Transforms/Utils/FunctionComparator.h
index b6b53d0f10cb9..400b9faa94c1b 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 cmpRangeMetadata(const MDNode *L, const MDNode *R) const;
+  int cmpMetadata(const MDNode *L, const MDNode *R) const;
+  int cmpInstMetadata(Instruction const *L, Instruction const *R) const;
   int cmpOperandBundlesSchema(const CallBase &LCS, const CallBase &RCS) const;
 
   /// Compare two GEPs for equivalent pointer arithmetic.

diff  --git a/llvm/lib/Transforms/Utils/FunctionComparator.cpp b/llvm/lib/Transforms/Utils/FunctionComparator.cpp
index 3fa61ec68cd30..af8bc8126160e 100644
--- a/llvm/lib/Transforms/Utils/FunctionComparator.cpp
+++ b/llvm/lib/Transforms/Utils/FunctionComparator.cpp
@@ -157,16 +157,13 @@ int FunctionComparator::cmpAttrs(const AttributeList L,
   return 0;
 }
 
-int FunctionComparator::cmpRangeMetadata(const MDNode *L,
-                                         const MDNode *R) const {
+int FunctionComparator::cmpMetadata(const MDNode *L, const MDNode *R) const {
   if (L == R)
     return 0;
   if (!L)
     return -1;
   if (!R)
     return 1;
-  // Range metadata is a sequence of numbers. Make sure they are the same
-  // sequence.
   // TODO: Note that as this is metadata, it is possible to drop and/or merge
   // this data when considering functions to merge. Thus this comparison would
   // return 0 (i.e. equivalent), but merging would become more complicated
@@ -176,14 +173,48 @@ int FunctionComparator::cmpRangeMetadata(const MDNode *L,
   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()))
       return Res;
   }
   return 0;
 }
 
+int FunctionComparator::cmpInstMetadata(Instruction const *L,
+                                        Instruction const *R) const {
+  /// These metadata affects the other optimization passes by making assertions
+  /// or constraints.
+  /// Values that carry 
diff erent expectations should be considered 
diff erent.
+  SmallVector<std::pair<unsigned, MDNode *>> MDL, MDR;
+  L->getAllMetadataOtherThanDebugLoc(MDL);
+  R->getAllMetadataOtherThanDebugLoc(MDR);
+  if (MDL.size() > MDR.size())
+    return 1;
+  else if (MDL.size() < MDR.size())
+    return -1;
+  for (size_t I = 0, N = MDL.size(); I < N; ++I) {
+    auto const [KeyL, ML] = MDL[I];
+    auto const [KeyR, MR] = MDR[I];
+    if (int Res = cmpNumbers(KeyL, KeyR))
+      return Res;
+    if (int Res = cmpMetadata(ML, MR))
+      return Res;
+  }
+  return 0;
+}
+
 int FunctionComparator::cmpOperandBundlesSchema(const CallBase &LCS,
                                                 const CallBase &RCS) const {
   assert(LCS.getOpcode() == RCS.getOpcode() && "Can't compare otherwise!");
@@ -586,9 +617,7 @@ int FunctionComparator::cmpOperations(const Instruction *L,
     if (int Res = cmpNumbers(LI->getSyncScopeID(),
                              cast<LoadInst>(R)->getSyncScopeID()))
       return Res;
-    return cmpRangeMetadata(
-        LI->getMetadata(LLVMContext::MD_range),
-        cast<LoadInst>(R)->getMetadata(LLVMContext::MD_range));
+    return cmpInstMetadata(L, R);
   }
   if (const StoreInst *SI = dyn_cast<StoreInst>(L)) {
     if (int Res =
@@ -616,8 +645,8 @@ int FunctionComparator::cmpOperations(const Instruction *L,
       if (int Res = cmpNumbers(CI->getTailCallKind(),
                                cast<CallInst>(R)->getTailCallKind()))
         return Res;
-    return cmpRangeMetadata(L->getMetadata(LLVMContext::MD_range),
-                            R->getMetadata(LLVMContext::MD_range));
+    return cmpMetadata(L->getMetadata(LLVMContext::MD_range),
+                       R->getMetadata(LLVMContext::MD_range));
   }
   if (const InsertValueInst *IVI = dyn_cast<InsertValueInst>(L)) {
     ArrayRef<unsigned> LIndices = IVI->getIndices();