[TargetLowering] Add hook for adding target MMO flags when doing ISel.

[gberry]

Summary

Also add support for printing and parsing target MMO flags.

[hfinkel]

Given that I understand the use case here to be the desire to translate metadata on loads into something at the MI level:

Did you consider just adding the metadata operands as MI-level metadata operands to the load? I believe that MI-level metadata operands are only currently used for DBG_VALUEs, but I can imagine using them for more-general information passing. I question came up just the other day (again) about preserving the !unpredictable metadata on branches for MI-level transformations, and so I'm wondering whether it is worth exploring a solution that can handle more than memory accesses.

Alternatively, MMOs currently hold a bunch of pointers to various kinds of Metadata. Should we extend this in some way? One option, for example, would be to allow targets to subclass MachinePointerInfo (and, so, we'd always construct and free these via some target callback), thus allowing the target to add whatever it wants.

All that having been said, AArch64 already uses target MMO flags (for suppressLdStPair), and I'm completely in favor of adding some way to get these in and out of MIR. It would be nice to also let the target callbacks provide names. The syntax could just be to use a quoted string, for example. I believe that's unambiguous.

[gberry]

In D34962#798587, @hfinkel wrote:

Given that I understand the use case here to be the desire to translate metadata on loads into something at the MI level:

Did you consider just adding the metadata operands as MI-level metadata operands to the load? I believe that MI-level metadata operands are only currently used for DBG_VALUEs, but I can imagine using them for more-general information passing. I question came up just the other day (again) about preserving the !unpredictable metadata on branches for MI-level transformations, and so I'm wondering whether it is worth exploring a solution that can handle more than memory accesses.

Alternatively, MMOs currently hold a bunch of pointers to various kinds of Metadata. Should we extend this in some way? One option, for example, would be to allow targets to subclass MachinePointerInfo (and, so, we'd always construct and free these via some target callback), thus allowing the target to add whatever it wants.

All that having been said, AArch64 already uses target MMO flags (for suppressLdStPair), and I'm completely in favor of adding some way to get these in and out of MIR. It would be nice to also let the target callbacks provide names. The syntax could just be to use a quoted string, for example. I believe that's unambiguous.

Since I only needed one bit on some loads, the target MMO flags seemed like a perfect fit. I did consider adding target hooks for printing/parsing these flag values, but it didn't seem worth the effort, since I think it will make parsing these particularly complex (e.g. we would need custom token types per target etc.). If we made them quoted strings, that might make it simple enough to be worth while.

[hfinkel]

In D34962#799842, @gberry wrote:

In D34962#798587, @hfinkel wrote:

Given that I understand the use case here to be the desire to translate metadata on loads into something at the MI level:

Did you consider just adding the metadata operands as MI-level metadata operands to the load? I believe that MI-level metadata operands are only currently used for DBG_VALUEs, but I can imagine using them for more-general information passing. I question came up just the other day (again) about preserving the !unpredictable metadata on branches for MI-level transformations, and so I'm wondering whether it is worth exploring a solution that can handle more than memory accesses.

Alternatively, MMOs currently hold a bunch of pointers to various kinds of Metadata. Should we extend this in some way? One option, for example, would be to allow targets to subclass MachinePointerInfo (and, so, we'd always construct and free these via some target callback), thus allowing the target to add whatever it wants.

All that having been said, AArch64 already uses target MMO flags (for suppressLdStPair), and I'm completely in favor of adding some way to get these in and out of MIR. It would be nice to also let the target callbacks provide names. The syntax could just be to use a quoted string, for example. I believe that's unambiguous.

Since I only needed one bit on some loads, the target MMO flags seemed like a perfect fit. I did consider adding target hooks for printing/parsing these flag values, but it didn't seem worth the effort, since I think it will make parsing these particularly complex (e.g. we would need custom token types per target etc.). If we made them quoted strings, that might make it simple enough to be worth while.

I'd prefer that they be quoted strings; otherwise, I'm happy with the direction.

[MatzeB]

There is already precedent with
TargetInstrInfo::getSerializableTargetIndices(), getSerializableDirectMachineOperandTargetFlags() and getSerializableBitmaskMachineOperandTargetFlags()

Maybe you can add something for MMO flags in a similar way?

[gberry]

Update to add target hook to print/parse target MMO flags as string literals in MIR text

[hfinkel]

This LGTM.

The getMMOFlags addition can be a separate patch, right? If so, please separate it.

I'd also really like a comment added to include/llvm/CodeGen/MachineMemOperand.h, around here:

// Reserved for use by target-specific passes.
MOTargetFlag1 = 1u << 6,
MOTargetFlag2 = 1u << 7,
MOTargetFlag3 = 1u << 8,

reminding us that if we add more flags here we also need to update the MIR printing/parsing code.

llvm/trunk/include/llvm/CodeGen/MachineMemOperand.h

enum Flags : uint16_t {
     /// The memory access is non-temporal.
     MONonTemporal = 1u << 3,
     /// The memory access is dereferenceable (i.e., doesn't trap).
     MODereferenceable = 1u << 4,
     /// The memory access always returns the same value (or traps).
     MOInvariant = 1u << 5,
 
     // Reserved for use by target-specific passes.
+    // Targets may override getSerializableMachineMemOperandTargetFlags() to
+    // enable MIR serialization/parsing of these flags.  If more of these flags
+    // are added, the MIR printing/parsing code will need to be updated as well.
     MOTargetFlag1 = 1u << 6,
     MOTargetFlag2 = 1u << 7,
     MOTargetFlag3 = 1u << 8,
 
     LLVM_MARK_AS_BITMASK_ENUM(/* LargestFlag = */ MOTargetFlag3)
   };
 
 private:

llvm/trunk/include/llvm/Target/TargetInstrInfo.h

public:
   ///
   /// MIR Serialization is able to serialize only the target flags that are
   /// defined by this method.
   virtual ArrayRef<std::pair<unsigned, const char *>>
   getSerializableBitmaskMachineOperandTargetFlags() const {
     return None;
   }
 
+  /// Return an array that contains the MMO target flag values and their
+  /// names.
+  ///
+  /// MIR Serialization is able to serialize only the MMO target flags that are
+  /// defined by this method.
+  virtual ArrayRef<std::pair<MachineMemOperand::Flags, const char *>>
+  getSerializableMachineMemOperandTargetFlags() const {
+    return None;
+  }
+
   /// Determines whether \p Inst is a tail call instruction. Override this
   /// method on targets that do not properly set MCID::Return and MCID::Call on
   /// tail call instructions."
   virtual bool isTailCall(const MachineInstr &Inst) const {
     return Inst.isReturn() && Inst.isCall();
   }
 
   /// True if the instruction is bound to the top of its basic block and no

llvm/trunk/lib/CodeGen/MIRParser/MILexer.h

bool isRegisterFlag() const {
     return Kind == kw_implicit || Kind == kw_implicit_define ||
            Kind == kw_def || Kind == kw_dead || Kind == kw_killed ||
            Kind == kw_undef || Kind == kw_internal ||
            Kind == kw_early_clobber || Kind == kw_debug_use;
   }
 
   bool isMemoryOperandFlag() const {
     return Kind == kw_volatile || Kind == kw_non_temporal ||
-           Kind == kw_dereferenceable || Kind == kw_invariant;
+           Kind == kw_dereferenceable || Kind == kw_invariant ||
+           Kind == StringConstant;
   }
 
   bool is(TokenKind K) const { return Kind == K; }
 
   bool isNot(TokenKind K) const { return Kind != K; }
 
   StringRef::iterator location() const { return Range.begin(); }
 

llvm/trunk/lib/CodeGen/MIRParser/MIParser.cpp

class MIParser {
   /// Maps from slot numbers to function's unnamed values.
   DenseMap<unsigned, const Value *> Slots2Values;
   /// Maps from target index names to target indices.
   StringMap<int> Names2TargetIndices;
   /// Maps from direct target flag names to the direct target flag values.
   StringMap<unsigned> Names2DirectTargetFlags;
   /// Maps from direct target flag names to the bitmask target flag values.
   StringMap<unsigned> Names2BitmaskTargetFlags;
+  /// Maps from MMO target flag names to MMO target flag values.
+  StringMap<MachineMemOperand::Flags> Names2MMOTargetFlags;
 
 public:
   MIParser(PerFunctionMIParsingState &PFS, SMDiagnostic &Error,
            StringRef Source);
 
   /// \p SkipChar gives the number of characters to skip before looking
   /// for the next token.
   void lex(unsigned SkipChar = 0);
private:
   void initNames2BitmaskTargetFlags();
 
   /// Try to convert a name of a bitmask target flag to the corresponding
   /// target flag.
   ///
   /// Return true if the name isn't a name of a bitmask target flag.
   bool getBitmaskTargetFlag(StringRef Name, unsigned &Flag);
 
+  void initNames2MMOTargetFlags();
+
+  /// Try to convert a name of a MachineMemOperand target flag to the
+  /// corresponding target flag.
+  ///
+  /// Return true if the name isn't a name of a target MMO flag.
+  bool getMMOTargetFlag(StringRef Name, MachineMemOperand::Flags &Flag);
+
   /// parseStringConstant
   ///   ::= StringConstant
   bool parseStringConstant(std::string &Result);
 };
 
 } // end anonymous namespace
 
 MIParser::MIParser(PerFunctionMIParsingState &PFS, SMDiagnostic &Error,
case MIToken::kw_non_temporal:
     Flags |= MachineMemOperand::MONonTemporal;
     break;
   case MIToken::kw_dereferenceable:
     Flags |= MachineMemOperand::MODereferenceable;
     break;
   case MIToken::kw_invariant:
     Flags |= MachineMemOperand::MOInvariant;
     break;
-  // TODO: parse the target specific memory operand flags.
+  case MIToken::StringConstant: {
+    MachineMemOperand::Flags TF;
+    if (getMMOTargetFlag(Token.stringValue(), TF))
+      return error("use of undefined target MMO flag '" + Token.stringValue() +
+                   "'");
+    Flags |= TF;
+    break;
+  }
   default:
     llvm_unreachable("The current token should be a memory operand flag");
   }
   if (OldFlags == Flags)
     // We know that the same flag is specified more than once when the flags
     // weren't modified.
     return error("duplicate '" + Token.stringValue() + "' memory operand flag");
   lex();
bool MIParser::getBitmaskTargetFlag(StringRef Name, unsigned &Flag) {
   initNames2BitmaskTargetFlags();
   auto FlagInfo = Names2BitmaskTargetFlags.find(Name);
   if (FlagInfo == Names2BitmaskTargetFlags.end())
     return true;
   Flag = FlagInfo->second;
   return false;
 }
 
+void MIParser::initNames2MMOTargetFlags() {
+  if (!Names2MMOTargetFlags.empty())
+    return;
+  const auto *TII = MF.getSubtarget().getInstrInfo();
+  assert(TII && "Expected target instruction info");
+  auto Flags = TII->getSerializableMachineMemOperandTargetFlags();
+  for (const auto &I : Flags)
+    Names2MMOTargetFlags.insert(
+        std::make_pair(StringRef(I.second), I.first));
+}
+
+bool MIParser::getMMOTargetFlag(StringRef Name,
+                                MachineMemOperand::Flags &Flag) {
+  initNames2MMOTargetFlags();
+  auto FlagInfo = Names2MMOTargetFlags.find(Name);
+  if (FlagInfo == Names2MMOTargetFlags.end())
+    return true;
+  Flag = FlagInfo->second;
+  return false;
+}
+
 bool MIParser::parseStringConstant(std::string &Result) {
   if (Token.isNot(MIToken::StringConstant))
     return error("expected string constant");
   Result = Token.stringValue();
   lex();
   return false;
 }
 

llvm/trunk/lib/CodeGen/MIRPrinter.cpp

public:
   void printIRBlockReference(const BasicBlock &BB);
   void printIRValueReference(const Value &V);
   void printStackObjectReference(int FrameIndex);
   void printOffset(int64_t Offset);
   void printTargetFlags(const MachineOperand &Op);
   void print(const MachineOperand &Op, const TargetRegisterInfo *TRI,
              unsigned I, bool ShouldPrintRegisterTies,
              LLT TypeToPrint, bool IsDef = false);
-  void print(const LLVMContext &Context, const MachineMemOperand &Op);
+  void print(const LLVMContext &Context, const TargetInstrInfo &TII,
+             const MachineMemOperand &Op);
   void printSyncScope(const LLVMContext &Context, SyncScope::ID SSID);
 
   void print(const MCCFIInstruction &CFI, const TargetRegisterInfo *TRI);
 };
 
 } // end namespace llvm
 
 namespace llvm {
void MIPrinter::print(const MachineInstr &MI) {
 
   if (!MI.memoperands_empty()) {
     OS << " :: ";
     const LLVMContext &Context = MF->getFunction()->getContext();
     bool NeedComma = false;
     for (const auto *Op : MI.memoperands()) {
       if (NeedComma)
         OS << ", ";
-      print(Context, *Op);
+      print(Context, *TII, *Op);
       NeedComma = true;
     }
   }
 }
 
 void MIPrinter::printMBBReference(const MachineBasicBlock &MBB) {
   OS << "%bb." << MBB.getNumber();
   if (const auto *BB = MBB.getBasicBlock()) {
case MachineOperand::MO_Predicate: {
     auto Pred = static_cast<CmpInst::Predicate>(Op.getPredicate());
     OS << (CmpInst::isIntPredicate(Pred) ? "int" : "float") << "pred("
        << CmpInst::getPredicateName(Pred) << ')';
     break;
   }
   }
 }
 
-void MIPrinter::print(const LLVMContext &Context, const MachineMemOperand &Op) {
+static const char *getTargetMMOFlagName(const TargetInstrInfo &TII,
+                                        unsigned TMMOFlag) {
+  auto Flags = TII.getSerializableMachineMemOperandTargetFlags();
+  for (const auto &I : Flags) {
+    if (I.first == TMMOFlag) {
+      return I.second;
+    }
+  }
+  return nullptr;
+}
+
+void MIPrinter::print(const LLVMContext &Context, const TargetInstrInfo &TII,
+                      const MachineMemOperand &Op) {
   OS << '(';
-  // TODO: Print operand's target specific flags.
   if (Op.isVolatile())
     OS << "volatile ";
   if (Op.isNonTemporal())
     OS << "non-temporal ";
   if (Op.isDereferenceable())
     OS << "dereferenceable ";
   if (Op.isInvariant())
     OS << "invariant ";
+  if (Op.getFlags() & MachineMemOperand::MOTargetFlag1)
+    OS << '"' << getTargetMMOFlagName(TII, MachineMemOperand::MOTargetFlag1)
+       << "\" ";
+  if (Op.getFlags() & MachineMemOperand::MOTargetFlag2)
+    OS << '"' << getTargetMMOFlagName(TII, MachineMemOperand::MOTargetFlag2)
+       << "\" ";
+  if (Op.getFlags() & MachineMemOperand::MOTargetFlag3)
+    OS << '"' << getTargetMMOFlagName(TII, MachineMemOperand::MOTargetFlag3)
+       << "\" ";
   if (Op.isLoad())
     OS << "load ";
   else {
     assert(Op.isStore() && "Non load machine operand must be a store");
     OS << "store ";
   }
 
   printSyncScope(Context, Op.getSyncScopeID());

llvm/trunk/lib/CodeGen/MachineInstr.cpp

void MachineMemOperand::print(raw_ostream &OS, ModuleSlotTracker &MST) const {
   }
 
   if (isNonTemporal())
     OS << "(nontemporal)";
   if (isDereferenceable())
     OS << "(dereferenceable)";
   if (isInvariant())
     OS << "(invariant)";
+  if (getFlags() & MOTargetFlag1)
+    OS << "(flag1)";
+  if (getFlags() & MOTargetFlag2)
+    OS << "(flag2)";
+  if (getFlags() & MOTargetFlag3)
+    OS << "(flag3)";
 }
 
 //===----------------------------------------------------------------------===//
 // MachineInstr Implementation
 //===----------------------------------------------------------------------===//
 
 void MachineInstr::addImplicitDefUseOperands(MachineFunction &MF) {
   if (MCID->ImplicitDefs)

llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.h

public:
   bool expandPostRAPseudo(MachineInstr &MI) const override;
 
   std::pair<unsigned, unsigned>
   decomposeMachineOperandsTargetFlags(unsigned TF) const override;
   ArrayRef<std::pair<unsigned, const char *>>
   getSerializableDirectMachineOperandTargetFlags() const override;
   ArrayRef<std::pair<unsigned, const char *>>
   getSerializableBitmaskMachineOperandTargetFlags() const override;
+  ArrayRef<std::pair<MachineMemOperand::Flags, const char *>>
+  getSerializableMachineMemOperandTargetFlags() const override;
 
   bool isFunctionSafeToOutlineFrom(MachineFunction &MF) const override;
   unsigned getOutliningBenefit(size_t SequenceSize, size_t Occurrences,
                                bool CanBeTailCall) const override;
   AArch64GenInstrInfo::MachineOutlinerInstrType
   getOutliningType(MachineInstr &MI) const override;
   void insertOutlinerEpilogue(MachineBasicBlock &MBB,
                               MachineFunction &MF,

llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.cpp

AArch64InstrInfo::getSerializableBitmaskMachineOperandTargetFlags() const {
 
   static const std::pair<unsigned, const char *> TargetFlags[] = {
       {MO_GOT, "aarch64-got"},
       {MO_NC, "aarch64-nc"},
       {MO_TLS, "aarch64-tls"}};
   return makeArrayRef(TargetFlags);
 }
 
+ArrayRef<std::pair<MachineMemOperand::Flags, const char *>>
+AArch64InstrInfo::getSerializableMachineMemOperandTargetFlags() const {
+  static const std::pair<MachineMemOperand::Flags, const char *> TargetFlags[] =
+      {{MOSuppressPair, "aarch64-suppress-pair"}};
+  return makeArrayRef(TargetFlags);
+}
+
 unsigned AArch64InstrInfo::getOutliningBenefit(size_t SequenceSize,
                                                size_t Occurrences,
                                                bool CanBeTailCall) const {
   unsigned NotOutlinedSize = SequenceSize * Occurrences;
   unsigned OutlinedSize;
 
   // Is this candidate something we can outline as a tail call?
   if (CanBeTailCall) {

llvm/trunk/test/CodeGen/MIR/AArch64/invalid-target-memoperands.mir

+# RUN: not llc -mtriple=aarch64-none-linux-gnu -run-pass none -o /dev/null %s 2>&1 | FileCheck %s
+
+--- |
+
+  define void @target_memoperands_error() {
+    ret void
+  }
+
+...
+---
+name:            target_memoperands_error
+body: |
+  bb.0:
+
+    %0:_(p0) = COPY %x0
+  ; CHECK: [[@LINE+1]]:35: use of undefined target MMO flag 'aarch64-invalid'
+    %1:_(s64) = G_LOAD %0(p0) :: ("aarch64-invalid" load 8)
+    RET_ReallyLR
+...

llvm/trunk/test/CodeGen/MIR/AArch64/target-memoperands.mir

+# RUN: llc -mtriple=aarch64-none-linux-gnu -run-pass none -o - %s | FileCheck %s
+
+--- |
+
+  define void @target_memoperands() {
+    ret void
+  }
+
+...
+---
+# CHECK-LABEL: name: target_memoperands
+# CHECK: %1(s64) = G_LOAD %0(p0) :: ("aarch64-suppress-pair" load 8)
+# CHECK: G_STORE %1(s64), %0(p0) :: ("aarch64-suppress-pair" store 8)
+name:            target_memoperands
+body: |
+  bb.0:
+
+    %0:_(p0) = COPY %x0
+    %1:_(s64) = G_LOAD %0(p0) :: ("aarch64-suppress-pair" load 8)
+    G_STORE %1(s64), %0(p0) :: ("aarch64-suppress-pair" store 8)
+    RET_ReallyLR
+...