[ARM][MC] Move information about variadic register defs into tablegen

[olista01]

Summary

Currently, variadic operands on an MCInst are assumed to be uses,
because they come after the defs. However, this is not always the case,
for example the Arm/Thumb LDM instructions write to a variable number of
registers.

This adds a property of instruction definitions which can be used to
mark variadic operands as defs. This only affects MCInst, because
MachineInstruction already tracks use/def per operand in each instance
of the instruction, so can already represent this.

This property can then be checked in MCInstrDesc, allowing us to remove
some special cases in ARMAsmParser::isITBlockTerminator.

[olista01]

Remove special case in MCInstrDesc::mayAffectControlFlow, which is no longer needed.

[rengolin]

Nice clean solution, thanks!

I wonder if there are tests that could expose the behaviour but is still consistent enough that won't change from other code-gen updates.

[olista01]

I can't think of any general way to test this, because these properties of MCInsts are not used much. The Thumb instructions which this applies to are tested by test/MC/ARM/implicit-it-generation.s, which has a big list of PC-writing instructions which cannot be used in the middle of an IT block, this patch is NFC for them.

[rengolin]

Right, that one is a hairy test, and I can see how this would break that test if not right. :)

LGTM, thanks!

llvm/trunk/include/llvm/MC/MCInstrDesc.h

enum Flag {
   CheapAsAMove,
   ExtraSrcRegAllocReq,
   ExtraDefRegAllocReq,
   RegSequence,
   ExtractSubreg,
   InsertSubreg,
   Convergent,
   Add,
-  Trap
+  Trap,
+  VariadicOpsAreDefs,
 };
 }
 
 /// Describe properties that are true of each instruction in the target
 /// description file.  This captures information about side effects, register
 /// use and many other things.  There is one instance of this struct for each
 /// target instruction class, and the MachineInstr class points to this struct
 /// directly to describe itself.
public:
 
 
   /// Return true if this instruction is convergent.
   ///
   /// Convergent instructions may not be made control-dependent on any
   /// additional values.
   bool isConvergent() const { return Flags & (1ULL << MCID::Convergent); }
 
+  /// Return true if variadic operands of this instruction are definitions.
+  bool variadicOpsAreDefs() const {
+    return Flags & (1ULL << MCID::VariadicOpsAreDefs);
+  }
+
   //===--------------------------------------------------------------------===//
   // Side Effect Analysis
   //===--------------------------------------------------------------------===//
 
   /// Return true if this instruction could possibly read memory.
   /// Instructions with this flag set are not necessarily simple load
   /// instructions, they may load a value and modify it, for example.
   bool mayLoad() const { return Flags & (1ULL << MCID::MayLoad); }

llvm/trunk/include/llvm/Target/Target.td

bit isPseudo     = 0;     // Is this instruction a pseudo-instruction?
                             // If so, won't have encoding information for
                             // the [MC]CodeEmitter stuff.
   bit isExtractSubreg = 0;  // Is this instruction a kind of extract subreg?
                              // If so, make sure to override
                              // TargetInstrInfo::getExtractSubregLikeInputs.
   bit isInsertSubreg = 0;   // Is this instruction a kind of insert subreg?
                             // If so, make sure to override
                             // TargetInstrInfo::getInsertSubregLikeInputs.
+  bit variadicOpsAreDefs = 0; // Are variadic operands definitions?
 
   // Does the instruction have side effects that are not captured by any
   // operands of the instruction or other flags?
   bit hasSideEffects = ?;
 
   // Is this instruction a "real" instruction (with a distinct machine
   // encoding), or is it a pseudo instruction used for codegen modeling
   // purposes.

llvm/trunk/lib/MC/MCInstrDesc.cpp

bool MCInstrDesc::mayAffectControlFlow(const MCInst &MI,
                                        const MCRegisterInfo &RI) const {
   if (isBranch() || isCall() || isReturn() || isIndirectBranch())
     return true;
   unsigned PC = RI.getProgramCounter();
   if (PC == 0)
     return false;
   if (hasDefOfPhysReg(MI, PC, RI))
     return true;
-  // A variadic instruction may define PC in the variable operand list.
-  // There's currently no indication of which entries in a variable
-  // list are defs and which are uses. While that's the case, this function
-  // needs to assume they're defs in order to be conservatively correct.
-  for (int i = NumOperands, e = MI.getNumOperands(); i != e; ++i) {
-    if (MI.getOperand(i).isReg() &&
-        RI.isSubRegisterEq(PC, MI.getOperand(i).getReg()))
-      return true;
-  }
   return false;
 }
 
 bool MCInstrDesc::hasImplicitDefOfPhysReg(unsigned Reg,
                                           const MCRegisterInfo *MRI) const {
   if (const MCPhysReg *ImpDefs = ImplicitDefs)
     for (; *ImpDefs; ++ImpDefs)
       if (*ImpDefs == Reg || (MRI && MRI->isSubRegister(Reg, *ImpDefs)))
         return true;
   return false;
 }
 
 bool MCInstrDesc::hasDefOfPhysReg(const MCInst &MI, unsigned Reg,
                                   const MCRegisterInfo &RI) const {
   for (int i = 0, e = NumDefs; i != e; ++i)
     if (MI.getOperand(i).isReg() &&
         RI.isSubRegisterEq(Reg, MI.getOperand(i).getReg()))
       return true;
+  if (variadicOpsAreDefs())
+    for (int i = NumOperands - 1, e = MI.getNumOperands(); i != e; ++i)
+      if (MI.getOperand(i).isReg() &&
+          RI.isSubRegisterEq(Reg, MI.getOperand(i).getReg()))
+        return true;
   return hasImplicitDefOfPhysReg(Reg, &RI);
 }

llvm/trunk/lib/Target/ARM/ARMInstrInfo.td

def IB_UPD :
     let Inst{20}    = L_bit;
 
     let DecoderMethod = "DecodeMemMultipleWritebackInstruction";
   }
 }
 
 let hasSideEffects = 0 in {
 
-let mayLoad = 1, hasExtraDefRegAllocReq = 1 in
+let mayLoad = 1, hasExtraDefRegAllocReq = 1, variadicOpsAreDefs = 1 in
 defm LDM : arm_ldst_mult<"ldm", "", 1, 0, LdStMulFrm, IIC_iLoad_m,
                          IIC_iLoad_mu>, ComplexDeprecationPredicate<"ARMLoad">;
 
 let mayStore = 1, hasExtraSrcRegAllocReq = 1 in
 defm STM : arm_ldst_mult<"stm", "", 0, 0, LdStMulFrm, IIC_iStore_m,
                          IIC_iStore_mu>,
            ComplexDeprecationPredicate<"ARMStore">;
 

llvm/trunk/lib/Target/ARM/ARMInstrThumb.td

 
 //===----------------------------------------------------------------------===//
 //  Load / store multiple Instructions.
 //
 
 // These require base address to be written back or one of the loaded regs.
 let hasSideEffects = 0 in {
 
-let mayLoad = 1, hasExtraDefRegAllocReq = 1 in
+let mayLoad = 1, hasExtraDefRegAllocReq = 1, variadicOpsAreDefs = 1 in
 def tLDMIA : T1I<(outs), (ins tGPR:$Rn, pred:$p, reglist:$regs, variable_ops),
         IIC_iLoad_m, "ldm${p}\t$Rn, $regs", []>, T1Encoding<{1,1,0,0,1,?}> {
   bits<3> Rn;
   bits<8> regs;
   let Inst{10-8} = Rn;
   let Inst{7-0}  = regs;
 }
 

 }
 
 } // hasSideEffects
 
 def : InstAlias<"ldm${p} $Rn!, $regs",
                 (tLDMIA tGPR:$Rn, pred:$p, reglist:$regs), 0>,
         Requires<[IsThumb, IsThumb1Only]>;
 
-let mayLoad = 1, Uses = [SP], Defs = [SP], hasExtraDefRegAllocReq = 1 in
+let mayLoad = 1, Uses = [SP], Defs = [SP], hasExtraDefRegAllocReq = 1,
+    variadicOpsAreDefs = 1 in
 def tPOP : T1I<(outs), (ins pred:$p, reglist:$regs, variable_ops),
                IIC_iPop,
                "pop${p}\t$regs", []>,
            T1Misc<{1,1,0,?,?,?,?}> {
   bits<16> regs;
   let Inst{8}   = regs{15};
   let Inst{7-0} = regs{7-0};
 }

llvm/trunk/lib/Target/ARM/ARMInstrThumb2.td

def DB_UPD :
     let Inst{20}    = L_bit;
     let Inst{19-16} = Rn;
     let Inst{15-0}  = regs;
   }
 }
 
 let hasSideEffects = 0 in {
 
-let mayLoad = 1, hasExtraDefRegAllocReq = 1 in
+let mayLoad = 1, hasExtraDefRegAllocReq = 1, variadicOpsAreDefs = 1 in
 defm t2LDM : thumb2_ld_mult<"ldm", IIC_iLoad_m, IIC_iLoad_mu, 1>;
 
 multiclass thumb2_st_mult<string asm, InstrItinClass itin,
                             InstrItinClass itin_upd, bit L_bit> {
   def IA :
     T2XI<(outs), (ins GPR:$Rn, pred:$p, reglist:$regs, variable_ops),
          itin, !strconcat(asm, "${p}.w\t$Rn, $regs"), []> {
     bits<4>  Rn;

llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp

bool ARMAsmParser::isITBlockTerminator(MCInst &Inst) const {
   // All branch & call instructions terminate IT blocks with the exception of
   // SVC.
   if (MCID.isTerminator() || (MCID.isCall() && Inst.getOpcode() != ARM::tSVC) ||
       MCID.isReturn() || MCID.isBranch() || MCID.isIndirectBranch())
     return true;
 
   // Any arithmetic instruction which writes to the PC also terminates the IT
   // block.
-  for (unsigned OpIdx = 0; OpIdx < MCID.getNumDefs(); ++OpIdx) {
-    MCOperand &Op = Inst.getOperand(OpIdx);
-    if (Op.isReg() && Op.getReg() == ARM::PC)
-      return true;
-  }
-
-  if (MCID.hasImplicitDefOfPhysReg(ARM::PC, MRI))
-    return true;
-
-  // Instructions with variable operand lists, which write to the variable
-  // operands. We only care about Thumb instructions here, as ARM instructions
-  // obviously can't be in an IT block.
-  switch (Inst.getOpcode()) {
-  case ARM::tLDMIA:
-  case ARM::t2LDMIA:
-  case ARM::t2LDMIA_UPD:
-  case ARM::t2LDMDB:
-  case ARM::t2LDMDB_UPD:
-    if (listContainsReg(Inst, 3, ARM::PC))
-      return true;
-    break;
-  case ARM::tPOP:
-    if (listContainsReg(Inst, 2, ARM::PC))
-      return true;
-    break;
-  }
+  if (MCID.hasDefOfPhysReg(Inst, ARM::PC, *MRI))
 
   return false;
 }
 
 unsigned ARMAsmParser::MatchInstruction(OperandVector &Operands, MCInst &Inst,
                                           SmallVectorImpl<NearMissInfo> &NearMisses,
                                           bool MatchingInlineAsm,
                                           bool &EmitInITBlock,

llvm/trunk/utils/TableGen/CodeGenInstruction.h

public:
     bool isRegSequence : 1;
     bool isExtractSubreg : 1;
     bool isInsertSubreg : 1;
     bool isConvergent : 1;
     bool hasNoSchedulingInfo : 1;
     bool FastISelShouldIgnore : 1;
     bool hasChain : 1;
     bool hasChain_Inferred : 1;
+    bool variadicOpsAreDefs : 1;
 
     std::string DeprecatedReason;
     bool HasComplexDeprecationPredicate;
 
     /// Are there any undefined flags?
     bool hasUndefFlags() const {
       return mayLoad_Unset || mayStore_Unset || hasSideEffects_Unset;
     }

llvm/trunk/utils/TableGen/CodeGenInstruction.cpp

CodeGenInstruction::CodeGenInstruction(Record *R)
   hasCtrlDep   = R->getValueAsBit("hasCtrlDep");
   isNotDuplicable = R->getValueAsBit("isNotDuplicable");
   isRegSequence = R->getValueAsBit("isRegSequence");
   isExtractSubreg = R->getValueAsBit("isExtractSubreg");
   isInsertSubreg = R->getValueAsBit("isInsertSubreg");
   isConvergent = R->getValueAsBit("isConvergent");
   hasNoSchedulingInfo = R->getValueAsBit("hasNoSchedulingInfo");
   FastISelShouldIgnore = R->getValueAsBit("FastISelShouldIgnore");
+  variadicOpsAreDefs = R->getValueAsBit("variadicOpsAreDefs");
 
   bool Unset;
   mayLoad      = R->getValueAsBitOrUnset("mayLoad", Unset);
   mayLoad_Unset = Unset;
   mayStore     = R->getValueAsBitOrUnset("mayStore", Unset);
   mayStore_Unset = Unset;
   hasSideEffects = R->getValueAsBitOrUnset("hasSideEffects", Unset);
   hasSideEffects_Unset = Unset;

llvm/trunk/utils/TableGen/InstrDocsEmitter.cpp

#define FLAG(f) if (II->f) { FlagStrings.push_back(str(f)); }
     FLAG(hasExtraDefRegAllocReq)
     FLAG(isCodeGenOnly)
     FLAG(isPseudo)
     FLAG(isRegSequence)
     FLAG(isExtractSubreg)
     FLAG(isInsertSubreg)
     FLAG(isConvergent)
     FLAG(hasNoSchedulingInfo)
+    FLAG(variadicOpsAreDefs)
     if (!FlagStrings.empty()) {
       OS << "Flags: ";
       bool IsFirst = true;
       for (auto FlagString : FlagStrings) {
         if (!IsFirst)
           OS << ", ";
         OS << "``" << FlagString << "``";
         IsFirst = false;

llvm/trunk/utils/TableGen/InstrInfoEmitter.cpp

void InstrInfoEmitter::emitRecord(const CodeGenInstruction &Inst, unsigned Num,
   if (!Target.getAllowRegisterRenaming() || Inst.hasExtraSrcRegAllocReq)
     OS << "|(1ULL<<MCID::ExtraSrcRegAllocReq)";
   if (!Target.getAllowRegisterRenaming() || Inst.hasExtraDefRegAllocReq)
     OS << "|(1ULL<<MCID::ExtraDefRegAllocReq)";
   if (Inst.isRegSequence) OS << "|(1ULL<<MCID::RegSequence)";
   if (Inst.isExtractSubreg) OS << "|(1ULL<<MCID::ExtractSubreg)";
   if (Inst.isInsertSubreg) OS << "|(1ULL<<MCID::InsertSubreg)";
   if (Inst.isConvergent) OS << "|(1ULL<<MCID::Convergent)";
+  if (Inst.variadicOpsAreDefs) OS << "|(1ULL<<MCID::VariadicOpsAreDefs)";
 
   // Emit all of the target-specific flags...
   BitsInit *TSF = Inst.TheDef->getValueAsBitsInit("TSFlags");
   if (!TSF)
     PrintFatalError("no TSFlags?");
   uint64_t Value = 0;
   for (unsigned i = 0, e = TSF->getNumBits(); i != e; ++i) {
     if (const auto *Bit = dyn_cast<BitInit>(TSF->getBit(i)))