comments

Author: Pierre-vh

llvm/include/llvm/IR/MemoryModelRelaxationAnnotations.h

@@ -19,13 +19,13 @@
 
 #include <set>
 #include <string>
-#include <tuple>
 #include <vector>
 
 namespace llvm {
 
 class MDNode;
 class MDTuple;
+class Metadata;
 class StringRef;
 class raw_ostream;
 class LLVMContext;
@@ -57,6 +57,9 @@ class MMRAMetadata {
     return MMRAMetadata(A).isCompatibleWith(B);
   }
 
+  /// \returns true if \p MD is a well-formed MMRA tag.
+  static bool isTagMD(const Metadata *MD);
+
   /// \returns whether this set of tags is compatible with \p Other.
   bool isCompatibleWith(const MMRAMetadata &Other) const;
 

llvm/lib/Analysis/VectorUtils.cpp

@@ -796,12 +796,10 @@ Instruction *llvm::propagateMetadata(Instruction *Inst, ArrayRef<Value *> VL) {
                     LLVMContext::MD_nontemporal, LLVMContext::MD_invariant_load,
                     LLVMContext::MD_access_group, LLVMContext::MD_mmra}) {
     MDNode *MD = I0->getMetadata(Kind);
-    if (Kind == LLVMContext::MD_mmra && !MD)
-      continue;
-
     for (int J = 1, E = VL.size(); MD && J != E; ++J) {
       const Instruction *IJ = cast<Instruction>(VL[J]);
       MDNode *IMD = IJ->getMetadata(Kind);
+
       switch (Kind) {
       case LLVMContext::MD_mmra: {
         auto Tags = MMRAMetadata(dyn_cast_or_null<MDTuple>(MD));

llvm/lib/IR/MemoryModelRelaxationAnnotations.cpp

@@ -34,8 +34,7 @@ MMRAMetadata::MMRAMetadata(MDNode *MD) {
   // CTor can use the tryParse & just fatal on err.
 
   MDTuple *Tuple = dyn_cast<MDTuple>(MD);
-  if (!Tuple)
-    report_fatal_error("MMRAs should always be MDTuples!");
+  assert(Tuple && "Invalid MMRA structure");
 
   const auto HandleTagMD = [this](MDNode *TagMD) {
     addTag(cast<MDString>(TagMD->getOperand(0))->getString(),
@@ -49,18 +48,20 @@ MMRAMetadata::MMRAMetadata(MDNode *MD) {
 
   for (const MDOperand &Op : Tuple->operands()) {
     MDNode *MDOp = cast<MDNode>(Op.get());
-    if (!isTagMD(MDOp)) {
-      errs() << "MD Node:\n";
-      MD->print(errs());
-      errs() << "Operand:\n";
-      Op->print(errs());
-      report_fatal_error("Invalid MMRA Metadata Structure!");
-    }
-
+    assert(isTagMD(MDOp));
     HandleTagMD(MDOp);
   }
 }
 
+bool MMRAMetadata::isTagMD(const Metadata *MD) {
+  if (auto *Tuple = dyn_cast<MDTuple>(MD)) {
+    return Tuple->getNumOperands() == 2 &&
+           isa<MDString>(Tuple->getOperand(0)) &&
+           isa<MDString>(Tuple->getOperand(1));
+  }
+  return false;
+}
+
 bool MMRAMetadata::isCompatibleWith(const MMRAMetadata &Other) const {
   // Two sets of tags are compatible iff, for every unique tag prefix P
   // present in at least one set:

llvm/lib/IR/Verifier.cpp

@@ -4829,14 +4829,18 @@ void Verifier::visitMMRAMetadata(Instruction &I, MDNode *MD) {
            isa<MDString>(Tuple->getOperand(1));
   };
 
-  // Simple MMRA metadata like !{!"foo", "!bar"} -> ok.
-  if (IsLeaf(MD))
+  // MMRA Metadata should either be a tag, e.g. !{!"foo", !"bar"}, or a
+  // list of tags such as !2 in the following example:
+  //    !0 = !{!"a", !"b"}
+  //    !1 = !{!"c", !"d"}
+  //    !2 = !{!0, !1}
+  if (MMRAMetadata::isTagMD(MD))
     return;
 
   Check(isa<MDTuple>(MD), "!mmra expected to be a metadata tuple", I, MD);
   for (const MDOperand &MDOp : MD->operands())
-    Check(IsLeaf(MDOp), "!mmra metadata tuple operand is not an MMRA tag", I,
-          MDOp.get());
+    Check(MMRAMetadata::isTagMD(MDOp.get()),
+          "!mmra metadata tuple operand is not an MMRA tag", I, MDOp.get());
 }
 
 void Verifier::visitCallStackMetadata(MDNode *MD) {

llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp

@@ -20,7 +20,6 @@
 #include "llvm/IR/DebugInfoMetadata.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/LLVMContext.h"
-#include "llvm/IR/MemoryModelRelaxationAnnotations.h"
 #include "llvm/IR/PatternMatch.h"
 #include "llvm/Transforms/InstCombine/InstCombiner.h"
 #include "llvm/Transforms/Utils/Local.h"
@@ -1522,8 +1521,7 @@ bool InstCombinerImpl::mergeStoreIntoSuccessor(StoreInst &SI) {
     auto *SIVTy = SI.getValueOperand()->getType();
     auto *OSVTy = OtherStore->getValueOperand()->getType();
     return CastInst::isBitOrNoopPointerCastable(OSVTy, SIVTy, DL) &&
-           SI.hasSameSpecialState(OtherStore) &&
-           MMRAMetadata(SI) == MMRAMetadata(*OtherStore);
+           SI.hasSameSpecialState(OtherStore);
   };
 
   // If the other block ends in an unconditional branch, check for the 'if then

llvm/lib/Transforms/Scalar/EarlyCSE.cpp

@@ -36,7 +36,6 @@
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/LLVMContext.h"
-#include "llvm/IR/MemoryModelRelaxationAnnotations.h"
 #include "llvm/IR/PassManager.h"
 #include "llvm/IR/PatternMatch.h"
 #include "llvm/IR/Type.h"
@@ -363,11 +362,6 @@ static bool isEqualImpl(SimpleValue LHS, SimpleValue RHS) {
 
   if (LHSI->getOpcode() != RHSI->getOpcode())
     return false;
-
-  // Avoid aggressively combining MMRAs.
-  if (MMRAMetadata(*LHSI) != MMRAMetadata(*RHSI))
-    return false;
-
   if (LHSI->isIdenticalToWhenDefined(RHSI)) {
     // Convergent calls implicitly depend on the set of threads that is
     // currently executing, so conservatively return false if they are in
@@ -1591,10 +1585,6 @@ bool EarlyCSE::processNode(DomTreeNode *Node) {
           LLVM_DEBUG(dbgs() << "Skipping due to debug counter\n");
           continue;
         }
-        if (MMRAMetadata(Inst) != MMRAMetadata(*InVal.DefInst)) {
-          LLVM_DEBUG(dbgs() << "Skipping due to MMRAs being different\n");
-          continue;
-        }
         if (InVal.IsLoad)
           if (auto *I = dyn_cast<Instruction>(Op))
             combineMetadataForCSE(I, &Inst, false);
@@ -1642,10 +1632,6 @@ bool EarlyCSE::processNode(DomTreeNode *Node) {
           LLVM_DEBUG(dbgs() << "Skipping due to debug counter\n");
           continue;
         }
-        if (MMRAMetadata(Inst) != MMRAMetadata(*InVal.first)) {
-          LLVM_DEBUG(dbgs() << "Skipping due to MMRAs being different\n");
-          continue;
-        }
         if (!Inst.use_empty())
           Inst.replaceAllUsesWith(InVal.first);
         salvageKnowledge(&Inst, &AC);

llvm/lib/Transforms/Scalar/GVNHoist.cpp

@@ -246,8 +246,7 @@ static void combineKnownMetadata(Instruction *ReplInst, Instruction *I) {
                                       LLVMContext::MD_fpmath,
                                       LLVMContext::MD_invariant_load,
                                       LLVMContext::MD_invariant_group,
-                                      LLVMContext::MD_access_group,
-                                      LLVMContext::MD_mmra};
+                                      LLVMContext::MD_access_group};
   combineMetadata(ReplInst, I, KnownIDs, true);
 }
 

llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp

@@ -80,7 +80,6 @@
 #include "llvm/Analysis/GlobalsModRef.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/Instructions.h"
-#include "llvm/IR/MemoryModelRelaxationAnnotations.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Transforms/Scalar.h"
@@ -197,11 +196,10 @@ StoreInst *MergedLoadStoreMotion::canSinkFromBlock(BasicBlock *BB1,
         !isStoreSinkBarrierInRange(*Store1->getNextNode(), BB1->back(), Loc1) &&
         !isStoreSinkBarrierInRange(*Store0->getNextNode(), BB0->back(), Loc0) &&
         Store0->hasSameSpecialState(Store1) &&
-        MMRAMetadata(*Store0) == MMRAMetadata(*Store1) &
-            CastInst::isBitOrNoopPointerCastable(
-                Store0->getValueOperand()->getType(),
-                Store1->getValueOperand()->getType(),
-                Store0->getModule()->getDataLayout()))
+        CastInst::isBitOrNoopPointerCastable(
+            Store0->getValueOperand()->getType(),
+            Store1->getValueOperand()->getType(),
+            Store0->getModule()->getDataLayout()))
       return Store1;
   }
   return nullptr;

llvm/test/Transforms/EarlyCSE/mmra.ll

@@ -157,35 +157,6 @@ return:                                           ; preds = %if.end, %if.then
   ret ptr %retval.0
 }
 
-; TODO: We might want to disable GVN if MMRAs differ.
-define void @test6(i1 %b, ptr %x) {
-; CHECK-LABEL: define void @test6
-; CHECK-SAME: (i1 [[B:%.*]], ptr [[X:%.*]]) {
-; CHECK-NEXT:  entry:
-; CHECK-NEXT:    store i32 2, ptr [[X]], align 4, !mmra [[META4:![0-9]+]]
-; CHECK-NEXT:    br i1 [[B]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
-; CHECK:       if.then:
-; CHECK-NEXT:    br label [[IF_END:%.*]]
-; CHECK:       if.else:
-; CHECK-NEXT:    br label [[IF_END]]
-; CHECK:       if.end:
-; CHECK-NEXT:    ret void
-;
-entry:
-  br i1 %b, label %if.then, label %if.else
-
-if.then:                                          ; preds = %entry
-  store i32 2, ptr %x, align 4, !mmra !13
-  br label %if.end
-
-if.else:                                          ; preds = %entry
-  store i32 2, ptr %x, align 4, !mmra !14
-  br label %if.end
-
-if.end:                                           ; preds = %if.else, %if.then
-  ret void
-}
-
 !1 = !{!2, !2, i64 0}
 !2 = !{!"int", !3, i64 0}
 !3 = !{!"omnipotent char", !4, i64 0}
@@ -195,19 +166,9 @@ if.end:                                           ; preds = %if.else, %if.then
 !7 = !{i32 0, i32 2}
 !8 = !{i32 3, i32 4}
 !9 = !{}
-
-!10 = !{!"foo", !"bar"}
-!11 = !{!"foo", !"bux"}
-!12 = !{!"bar", !"baz"}
-!13 = !{!10, !11}
-!14 = !{!11, !12}
-
 ;.
-; CHECK: [[TBAA0]] = !{[[META1:![0-9]+]], [[META1]], i64 0}
-; CHECK: [[META1]] = !{!"omnipotent char", [[META2:![0-9]+]], i64 0}
-; CHECK: [[META2]] = !{!"Simple C++ TBAA"}
+; CHECK: [[TBAA0]] = !{!1, !1, i64 0}
+; CHECK: [[META1:![0-9]+]] = !{!"omnipotent char", !2, i64 0}
+; CHECK: [[META2:![0-9]+]] = !{!"Simple C++ TBAA"}
 ; CHECK: [[RNG3]] = !{i32 0, i32 2, i32 3, i32 4}
-; CHECK: [[META4]] = !{[[META5:![0-9]+]], [[META6:![0-9]+]]}
-; CHECK: [[META5]] = !{!"foo", !"bar"}
-; CHECK: [[META6]] = !{!"foo", !"bux"}
 ;.