[globalisel][tablegen] Map ld and st to G_LOAD and G_STORE. NFC

[dsanders]

Summary

There is an important mismatch between ISD::LOAD and G_LOAD (and likewise for
ISD::STORE and G_STORE). In SelectionDAG, ISD::LOAD is a non-atomic load
and atomic loads are handled by a separate node. However, this is not true of
GlobalISel's G_LOAD. For G_LOAD, the MachineMemOperand indicates the atomicity
of the operation. As a result, this mapping must also add a predicate that
checks for non-atomic MachineMemOperands.

This is NFC since these nodes always have predicates in practice and are
therefore always rejected at the moment.

Depends on D37443

[dsanders]

Rebase and ping

[qcolombet]

The added predicate part looks fine, but I didn't get why we need to change CodeGenInstruction into record just yet.

include/llvm/Target/GlobalISel/SelectionDAGCompat.td

83

Although I get the comment, I don't see how this affect the following definition.
I.e., what in the following definition changed compared to the surrounding ones to take this into account?

88

Typo (G_STORE (G_TRUNCATE x))

utils/TableGen/GlobalISelEmitter.cpp

1996

I don't get why this change is required yet. G_LOAD and G_STORE records are still CodeGenInstruction, aren't they? (I haven't checked)

[dsanders]

include/llvm/Target/GlobalISel/SelectionDAGCompat.td

83

I think I've missed a 'For example' in the comment but it would be clearer to explain the Atomic bit as well. I'll update the comment.

88

Thanks for spotting that. I'm not sure what I was thinking there.

utils/TableGen/GlobalISelEmitter.cpp

1996

I think this is another confusing comment. It's still technically correct but it makes it sound like it's a DenseMap<SDNode, Instruction> when it's now DenseMap<SDNode, GINodeEquiv>. I had to change the values of the map to the GINodeEquiv so I could access the new CheckMMOIsNonAtomic field.

[qcolombet]

LGTM with the previous nitpicks fixed.

utils/TableGen/GlobalISelEmitter.cpp

1996

Make sense.
I thought we could have kept CodeGenInstruction and then access the definition of the record to read the CheckMMOIsNonAtomic field, but that wouldn't work given the record would give us the instruction, not the GINodeEquiv.

Thanks for the clarifications.

include/llvm/CodeGen/GlobalISel/InstructionSelector.h

enum {
   /// Check an immediate predicate on the specified instruction via an APInt.
   /// - InsnID - Instruction ID
   /// - The predicate to test
   GIM_CheckAPIntImmPredicate,
   /// Check a floating point immediate predicate on the specified instruction.
   /// - InsnID - Instruction ID
   /// - The predicate to test
   GIM_CheckAPFloatImmPredicate,
+  /// Check a memory operation is non-atomic.
+  /// - InsnID - Instruction ID
+  GIM_CheckNonAtomic,
 
   /// Check the type for the specified operand
   /// - InsnID - Instruction ID
   /// - OpIdx - Operand index
   /// - Expected type
   GIM_CheckType,
   /// Check the register bank for the specified operand
   /// - InsnID - Instruction ID

include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h

case GIM_CheckAPFloatImmPredicate: {
       assert(Predicate > GIPFP_APFloat_Invalid && "Expected a valid predicate");
       APFloat Value = State.MIs[InsnID]->getOperand(1).getFPImm()->getValueAPF();
 
       if (!MatcherInfo.APFloatImmPredicateFns[Predicate](Value))
         if (handleReject() == RejectAndGiveUp)
           return false;
       break;
     }
+    case GIM_CheckNonAtomic: {
+      int64_t InsnID = MatchTable[CurrentIdx++];
+      DEBUG(dbgs() << CurrentIdx << ": GIM_CheckNonAtomic(MIs[" << InsnID
+                   << "])\n");
+      assert(State.MIs[InsnID] != nullptr && "Used insn before defined");
+      assert((State.MIs[InsnID]->getOpcode() == TargetOpcode::G_LOAD ||
+              State.MIs[InsnID]->getOpcode() == TargetOpcode::G_STORE) &&
+             "Expected G_LOAD/G_STORE");
+
+      if (!State.MIs[InsnID]->hasOneMemOperand())
+        if (handleReject() == RejectAndGiveUp)
+          return false;
+
+      for (const auto &MMO : State.MIs[InsnID]->memoperands())
+        if (MMO->getOrdering() != AtomicOrdering::NotAtomic)
+          if (handleReject() == RejectAndGiveUp)
+            return false;
+      break;
+    }
 
     case GIM_CheckType: {
       int64_t InsnID = MatchTable[CurrentIdx++];
       int64_t OpIdx = MatchTable[CurrentIdx++];
       int64_t TypeID = MatchTable[CurrentIdx++];
       DEBUG(dbgs() << CurrentIdx << ": GIM_CheckType(MIs[" << InsnID
                    << "]->getOperand(" << OpIdx << "), TypeID=" << TypeID
                    << ")\n");

include/llvm/Target/GlobalISel/SelectionDAGCompat.td

 //===----------------------------------------------------------------------===//
 
 // Declare that a generic Instruction is 'equivalent' to an SDNode, that is,
 // SelectionDAG patterns involving the SDNode can be transformed to match the
 // Instruction instead.
 class GINodeEquiv<Instruction i, SDNode node> {
   Instruction I = i;
   SDNode Node = node;
+
+  // SelectionDAG has separate nodes for atomic and non-atomic memory operations
+  // (ISD::LOAD, ISD::ATOMIC_LOAD, ISD::STORE, ISD::ATOMIC_STORE) but GlobalISel
+  // stores this information in the MachineMemoryOperand.
+  bit CheckMMOIsNonAtomic = 0;
 }
 
 // These are defined in the same order as the G_* instructions.
 def : GINodeEquiv<G_ANYEXT, anyext>;
 def : GINodeEquiv<G_SEXT, sext>;
 def : GINodeEquiv<G_ZEXT, zext>;
 def : GINodeEquiv<G_TRUNC, trunc>;
 def : GINodeEquiv<G_BITCAST, bitconvert>;

 def : GINodeEquiv<G_FLOG2, flog2>;
 def : GINodeEquiv<G_INTRINSIC, intrinsic_wo_chain>;
 // ISD::INTRINSIC_VOID can also be handled with G_INTRINSIC_W_SIDE_EFFECTS.
 def : GINodeEquiv<G_INTRINSIC_W_SIDE_EFFECTS, intrinsic_void>;
 def : GINodeEquiv<G_INTRINSIC_W_SIDE_EFFECTS, intrinsic_w_chain>;
 def : GINodeEquiv<G_BR, br>;
 def : GINodeEquiv<G_BSWAP, bswap>;
 
+// Broadly speaking G_LOAD is equivalent to ISD::LOAD but there are some
+// complications that tablegen must take care of. Predicates such as
+// isSignExtLoad require that this is not a perfect 1:1 mapping since a
+// sign-extending load is (G_SEXT (G_LOAD x)) in GlobalISel.
+def : GINodeEquiv<G_LOAD, ld> { let CheckMMOIsNonAtomic = 1; }
+// Broadly speaking G_STORE is equivalent to ISD::STORE but there are some
+// complications that tablegen must take care of. Predicates such as
+// isTruncStore require that this is not a perfect 1:1 mapping since a
+// truncating store is (G_TRUNCATE (G_LOAD x)) in GlobalISel.
+def : GINodeEquiv<G_STORE, st> { let CheckMMOIsNonAtomic = 1; }
+
 // Specifies the GlobalISel equivalents for SelectionDAG's ComplexPattern.
 // Should be used on defs that subclass GIComplexOperandMatcher<>.
 class GIComplexPatternEquiv<ComplexPattern seldag> {
   ComplexPattern SelDAGEquivalent = seldag;
 }

utils/TableGen/GlobalISelEmitter.cpp

 class InstructionPredicateMatcher {
 protected:
   /// This enum is used for RTTI and also defines the priority that is given to
   /// the predicate when generating the matcher code. Kinds with higher priority
   /// must be tested first.
   enum PredicateKind {
     IPM_Opcode,
     IPM_ImmPredicate,
+    IPM_NonAtomicMMO,
   };
 
   PredicateKind Kind;
 
 public:
   InstructionPredicateMatcher(PredicateKind Kind) : Kind(Kind) {}
   virtual ~InstructionPredicateMatcher() {}
 
void emitPredicateOpcodes(MatchTable &Table, RuleMatcher &Rule,
     Table << MatchTable::Opcode(Predicate.getGlobalISelMatchOpcode())
           << MatchTable::Comment("MI") << MatchTable::IntValue(InsnVarID)
           << MatchTable::Comment("Predicate")
           << MatchTable::NamedValue(Predicate.getGlobalISelEnumName())
           << MatchTable::LineBreak;
   }
 };
 
+/// Generates code to check that a memory instruction has a non-atomic MachineMemoryOperand.
+class NonAtomicMMOPredicateMatcher : public InstructionPredicateMatcher {
+public:
+  NonAtomicMMOPredicateMatcher()
+      : InstructionPredicateMatcher(IPM_NonAtomicMMO) {}
+
+  static bool classof(const InstructionPredicateMatcher *P) {
+    return P->getKind() == IPM_NonAtomicMMO;
+  }
+
+  void emitPredicateOpcodes(MatchTable &Table, RuleMatcher &Rule,
+                            unsigned InsnVarID) const override {
+    Table << MatchTable::Opcode("GIM_CheckNonAtomic")
+          << MatchTable::Comment("MI") << MatchTable::IntValue(InsnVarID)
+          << MatchTable::LineBreak;
+  }
+};
+
 /// Generates code to check that a set of predicates and operands match for a
 /// particular instruction.
 ///
 /// Typical predicates include:
 /// * Has a specific opcode.
 /// * Has an nsw/nuw flag or doesn't.
 class InstructionMatcher
     : public PredicateListMatcher<InstructionPredicateMatcher> {

 private:
   const RecordKeeper &RK;
   const CodeGenDAGPatterns CGP;
   const CodeGenTarget &Target;
   CodeGenRegBank CGRegs;
 
   /// Keep track of the equivalence between SDNodes and Instruction.
   /// This is defined using 'GINodeEquiv' in the target description.
-  DenseMap<Record *, const CodeGenInstruction *> NodeEquivs;
+  DenseMap<Record *, Record *> NodeEquivs;
 
   /// Keep track of the equivalence between ComplexPattern's and
   /// GIComplexOperandMatcher. Map entries are specified by subclassing
   /// GIComplexPatternEquiv.
   DenseMap<const Record *, const Record *> ComplexPatternEquivs;
 
   // Map of predicates to their subtarget features.
   SubtargetFeatureInfoMap SubtargetFeatures;
 
   void gatherNodeEquivs();
-  const CodeGenInstruction *findNodeEquiv(Record *N) const;
+  Record *findNodeEquiv(Record *N) const;
 
   Error importRulePredicates(RuleMatcher &M, ArrayRef<Predicate> Predicates);
   Expected<InstructionMatcher &>
   createAndImportSelDAGMatcher(InstructionMatcher &InsnMatcher,
                                const TreePatternNode *Src,
                                unsigned &TempOpIdx) const;
   Error importChildMatcher(InstructionMatcher &InsnMatcher,
                            const TreePatternNode *SrcChild, unsigned OpIdx,
private:
   Expected<RuleMatcher> runOnPattern(const PatternToMatch &P);
 
   void declareSubtargetFeature(Record *Predicate);
 };
 
 void GlobalISelEmitter::gatherNodeEquivs() {
   assert(NodeEquivs.empty());
   for (Record *Equiv : RK.getAllDerivedDefinitions("GINodeEquiv"))
-    NodeEquivs[Equiv->getValueAsDef("Node")] =
-        &Target.getInstruction(Equiv->getValueAsDef("I"));
+    NodeEquivs[Equiv->getValueAsDef("Node")] = Equiv;
 
   assert(ComplexPatternEquivs.empty());
   for (Record *Equiv : RK.getAllDerivedDefinitions("GIComplexPatternEquiv")) {
     Record *SelDAGEquiv = Equiv->getValueAsDef("SelDAGEquivalent");
     if (!SelDAGEquiv)
       continue;
     ComplexPatternEquivs[SelDAGEquiv] = Equiv;
  }
 }
 
-const CodeGenInstruction *GlobalISelEmitter::findNodeEquiv(Record *N) const {
+Record *GlobalISelEmitter::findNodeEquiv(Record *N) const {
   return NodeEquivs.lookup(N);
 }
 
 GlobalISelEmitter::GlobalISelEmitter(RecordKeeper &RK)
     : RK(RK), CGP(RK), Target(CGP.getTargetInfo()),
       CGRegs(RK, Target.getHwModes()) {}
 
 //===- Emitter ------------------------------------------------------------===//
GlobalISelEmitter::importRulePredicates(RuleMatcher &M,
 
   return Error::success();
 }
 
 Expected<InstructionMatcher &>
 GlobalISelEmitter::createAndImportSelDAGMatcher(InstructionMatcher &InsnMatcher,
                                                 const TreePatternNode *Src,
                                                 unsigned &TempOpIdx) const {
+  Record *SrcGIEquivOrNull = nullptr;
   const CodeGenInstruction *SrcGIOrNull = nullptr;
 
   // Start with the defined operands (i.e., the results of the root operator).
   if (Src->getExtTypes().size() > 1)
     return failedImport("Src pattern has multiple results");
 
   if (Src->isLeaf()) {
     Init *SrcInit = Src->getLeafValue();
     if (isa<IntInit>(SrcInit)) {
       InsnMatcher.addPredicate<InstructionOpcodeMatcher>(
           &Target.getInstruction(RK.getDef("G_CONSTANT")));
     } else
       return failedImport(
           "Unable to deduce gMIR opcode to handle Src (which is a leaf)");
   } else {
-    SrcGIOrNull = findNodeEquiv(Src->getOperator());
-    if (!SrcGIOrNull)
+    SrcGIEquivOrNull = findNodeEquiv(Src->getOperator());
+    if (!SrcGIEquivOrNull)
       return failedImport("Pattern operator lacks an equivalent Instruction" +
                           explainOperator(Src->getOperator()));
-    auto &SrcGI = *SrcGIOrNull;
+    SrcGIOrNull = &Target.getInstruction(SrcGIEquivOrNull->getValueAsDef("I"));
 
     // The operators look good: match the opcode
-    InsnMatcher.addPredicate<InstructionOpcodeMatcher>(&SrcGI);
+    InsnMatcher.addPredicate<InstructionOpcodeMatcher>(SrcGIOrNull);
   }
 
   unsigned OpIdx = 0;
   for (const TypeSetByHwMode &VTy : Src->getExtTypes()) {
     auto OpTyOrNone = VTy.isMachineValueType()
                           ? MVTToLLT(VTy.getMachineValueType().SimpleTy)
                           : None;
     if (!OpTyOrNone)
for (const auto &Predicate : Src->getPredicateFns()) {
     if (Predicate.isImmediatePattern()) {
       InsnMatcher.addPredicate<InstructionImmPredicateMatcher>(Predicate);
       continue;
     }
 
     return failedImport("Src pattern child has predicate (" +
                         explainPredicates(Src) + ")");
   }
+  if (SrcGIEquivOrNull && SrcGIEquivOrNull->getValueAsBit("CheckMMOIsNonAtomic"))
+    InsnMatcher.addPredicate<NonAtomicMMOPredicateMatcher>();
 
   if (Src->isLeaf()) {
     Init *SrcInit = Src->getLeafValue();
     if (IntInit *SrcIntInit = dyn_cast<IntInit>(SrcInit)) {
       OperandMatcher &OM =
           InsnMatcher.addOperand(OpIdx++, Src->getName(), TempOpIdx);
       if (!OM.isTied())
         OM.addPredicate<LiteralIntOperandMatcher>(SrcIntInit->getValue());