[PATCH] [RFC] Deprecated feature in TableGen

Tom Stellard tom at stellard.net
Wed Aug 14 09:15:29 PDT 2013


On Wed, Aug 14, 2013 at 09:05:15AM -0700, Joey Gouly wrote:
> Hello all,
> 
> Attached is a quick patch I wrote, to show a simple way of supporting a Deprecated feature in TableGen.
> This patch allows you to specify for an Instruction a SubtargetFeature that will define whether an instruction is deprecated or not.
> I also added support for a CustomDeprecatedMethod, that allows you to operate on an MCInst directly, to decide if it is deprecated.
> 
> In the ARM backend v8 deprecates SETEND, v6+ deprecates SWP/SWPB and the CP15ISB/DSB instructions. Is there enough general use for this?
> 

Hi Joey,

Does deprecating an instruction mean the hardware doesn't support it or
does it mean the hardware supports it, but the compiler does not?

Thanks,
Tom
> 
> http://llvm-reviews.chandlerc.com/D1399
> 
> Files:
>   include/llvm/MC/MCInstrDesc.h
>   include/llvm/Target/Target.td
>   lib/Target/ARM/ARMInstrInfo.td
>   lib/Target/ARM/AsmParser/ARMAsmParser.cpp
>   lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp
>   test/MC/ARM/deprecated-v8.s
>   tools/llvm-mc/Disassembler.cpp
>   tools/llvm-mc/Disassembler.h
>   tools/llvm-mc/llvm-mc.cpp
>   utils/TableGen/CodeGenInstruction.cpp
>   utils/TableGen/CodeGenInstruction.h
>   utils/TableGen/InstrInfoEmitter.cpp

> Index: include/llvm/MC/MCInstrDesc.h
> ===================================================================
> --- include/llvm/MC/MCInstrDesc.h
> +++ include/llvm/MC/MCInstrDesc.h
> @@ -16,6 +16,7 @@
>  #define LLVM_MC_MCINSTRDESC_H
>  
>  #include "llvm/MC/MCInst.h"
> +#include "llvm/MC/MCSubtargetInfo.h"
>  #include "llvm/MC/MCRegisterInfo.h"
>  #include "llvm/Support/DataTypes.h"
>  
> @@ -145,6 +146,10 @@
>    const uint16_t *ImplicitUses;  // Registers implicitly read by this instr
>    const uint16_t *ImplicitDefs;  // Registers implicitly defined by this instr
>    const MCOperandInfo *OpInfo;   // 'NumOperands' entries about operands
> +  uint64_t Deprecated;           // Feature bits that this is deprecated on, if any
> +
> +  typedef bool (*DepFN)(MCInst&, MCSubtargetInfo&);
> +  DepFN CustomDeprecator;        // Holds an optional deprecator method
>  
>    /// \brief Returns the value of the specific constraint if
>    /// it is set. Returns -1 if it is not set.
> @@ -158,6 +163,13 @@
>      return -1;
>    }
>  
> +  // \brief Return if this Instruction is deprecated
> +  bool isDeprecated(MCInst &MI, MCSubtargetInfo &STI) const {
> +    if (CustomDeprecator)
> +      return CustomDeprecator(MI, STI);
> +    return (STI.getFeatureBits() & Deprecated) != 0;
> +  }
> +
>    /// \brief Return the opcode number for this descriptor.
>    unsigned getOpcode() const {
>      return Opcode;
> Index: include/llvm/Target/Target.td
> ===================================================================
> --- include/llvm/Target/Target.td
> +++ include/llvm/Target/Target.td
> @@ -450,6 +450,9 @@
>    /// can be queried via the getNamedOperandIdx() function which is generated
>    /// by TableGen.
>    bit UseNamedOperandTable = 0;
> +
> +  /// A custom method to determine if an instruction is deprecated or not.
> +  string CustomDeprecatedMethod = "";
>  }
>  
>  /// PseudoInstExpansion - Expansion information for a pseudo-instruction.
> @@ -1005,6 +1008,11 @@
>    list<SubtargetFeature> Implies = i;
>  }
>  
> +class Deprecated<SubtargetFeature dep> {
> +  SubtargetFeature Dep = dep;
> +}
> +
> +
>  //===----------------------------------------------------------------------===//
>  // Processor chip sets - These values represent each of the chip sets supported
>  // by the scheduler.  Each Processor definition requires corresponding
> Index: lib/Target/ARM/ARMInstrInfo.td
> ===================================================================
> --- lib/Target/ARM/ARMInstrInfo.td
> +++ lib/Target/ARM/ARMInstrInfo.td
> @@ -1820,7 +1820,7 @@
>  defm PLI  : APreLoad<1, 0, "pli">,  Requires<[IsARM,HasV7]>;
>  
>  def SETEND : AXI<(outs), (ins setend_op:$end), MiscFrm, NoItinerary,
> -                 "setend\t$end", []>, Requires<[IsARM]> {
> +                 "setend\t$end", []>, Requires<[IsARM]>, Deprecated<HasV8Ops> {
>    bits<1> end;
>    let Inst{31-10} = 0b1111000100000001000000;
>    let Inst{9} = end;
> @@ -4698,12 +4698,14 @@
>    let Inst{19-16} = CRn;
>  }
>  
> +let CustomDeprecatedMethod = "MCRIsDeprecated" in {
>  def MCR : MovRCopro<"mcr", 0 /* from ARM core register to coprocessor */,
>                      (outs),
>                      (ins p_imm:$cop, imm0_7:$opc1, GPR:$Rt, c_imm:$CRn,
>                           c_imm:$CRm, imm0_7:$opc2),
>                      [(int_arm_mcr imm:$cop, imm:$opc1, GPR:$Rt, imm:$CRn,
>                                    imm:$CRm, imm:$opc2)]>;
> +}
>  def : ARMInstAlias<"mcr${p} $cop, $opc1, $Rt, $CRn, $CRm",
>                     (MCR p_imm:$cop, imm0_7:$opc1, GPR:$Rt, c_imm:$CRn,
>                          c_imm:$CRm, 0, pred:$p)>;
> Index: lib/Target/ARM/AsmParser/ARMAsmParser.cpp
> ===================================================================
> --- lib/Target/ARM/AsmParser/ARMAsmParser.cpp
> +++ lib/Target/ARM/AsmParser/ARMAsmParser.cpp
> @@ -232,7 +232,7 @@
>                                SmallVectorImpl<MCParsedAsmOperand*> &Operands);
>    bool shouldOmitPredicateOperand(StringRef Mnemonic,
>                                SmallVectorImpl<MCParsedAsmOperand*> &Operands);
> -  bool isDeprecated(MCInst &Inst, StringRef &Info);
> +  bool isDeprecated(MCInst &Inst);
>  
>  public:
>    enum ARMMatchResultTy {
> @@ -4955,12 +4955,19 @@
>    return false;
>  }
>  
> -bool ARMAsmParser::isDeprecated(MCInst &Inst, StringRef &Info) {
> -  if (hasV8Ops() && Inst.getOpcode() == ARM::SETEND) {
> -    Info = "armv8";
> -    return true;
> -  }
> -  return false;
> +// FIXME: We would really prefer to have MCInstrInfo (the wrapper around
> +// the ARMInsts array) instead. Getting that here requires awkward
> +// API changes, though. Better way?
> +namespace llvm {
> +extern const MCInstrDesc ARMInsts[];
> +}
> +static const MCInstrDesc &getInstDesc(unsigned Opcode) {
> +  return ARMInsts[Opcode];
> +}
> +bool ARMAsmParser::isDeprecated(MCInst &Inst) {
> +  const unsigned Opc = Inst.getOpcode();
> +  const MCInstrDesc &MCID = getInstDesc(Opc);
> +  return MCID.isDeprecated(Inst, STI);
>  }
>  
>  static bool isDataTypeToken(StringRef Tok) {
> @@ -5278,15 +5285,6 @@
>    return false;
>  }
>  
> -// FIXME: We would really prefer to have MCInstrInfo (the wrapper around
> -// the ARMInsts array) instead. Getting that here requires awkward
> -// API changes, though. Better way?
> -namespace llvm {
> -extern const MCInstrDesc ARMInsts[];
> -}
> -static const MCInstrDesc &getInstDesc(unsigned Opcode) {
> -  return ARMInsts[Opcode];
> -}
>  
>  // FIXME: We would really like to be able to tablegen'erate this.
>  bool ARMAsmParser::
> @@ -5487,8 +5485,8 @@
>    }
>  
>    StringRef DepInfo;
> -  if (isDeprecated(Inst, DepInfo))
> -    Warning(Loc, "deprecated on " + DepInfo);
> +  if (isDeprecated(Inst))
> +    Warning(Loc, "deprecated instruction");
>  
>    return false;
>  }
> Index: lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp
> ===================================================================
> --- lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp
> +++ lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp
> @@ -18,6 +18,7 @@
>  #include "InstPrinter/ARMInstPrinter.h"
>  #include "llvm/ADT/Triple.h"
>  #include "llvm/MC/MCCodeGenInfo.h"
> +#include "llvm/MC/MCInst.h"
>  #include "llvm/MC/MCInstrAnalysis.h"
>  #include "llvm/MC/MCInstrInfo.h"
>  #include "llvm/MC/MCRegisterInfo.h"
> @@ -29,6 +30,18 @@
>  #define GET_REGINFO_MC_DESC
>  #include "ARMGenRegisterInfo.inc"
>  
> +static bool MCRIsDeprecated(llvm::MCInst &MI, llvm::MCSubtargetInfo &STI) {
> +  // Checks for the deprecated CP15ISB encoding:
> +  // mcr pX, #0, rX, c7, c5, #4
> +  if (STI.getFeatureBits() & llvm::ARM::HasV8Ops &&
> +      (MI.getOperand(1).isImm() && MI.getOperand(1).getImm() == 0) &&
> +      (MI.getOperand(3).isImm() && MI.getOperand(3).getImm() == 7) &&
> +      (MI.getOperand(4).isImm() && MI.getOperand(4).getImm() == 5) &&
> +      (MI.getOperand(5).isImm() && MI.getOperand(5).getImm() == 4))
> +    return true;
> +  return false;
> +}
> +
>  #define GET_INSTRINFO_MC_DESC
>  #include "ARMGenInstrInfo.inc"
>  
> Index: test/MC/ARM/deprecated-v8.s
> ===================================================================
> --- test/MC/ARM/deprecated-v8.s
> +++ test/MC/ARM/deprecated-v8.s
> @@ -1,3 +1,5 @@
>  @ RUN: llvm-mc -triple armv8 -show-encoding < %s 2>&1 | FileCheck %s
>  setend be
> -@ CHECK: warning: deprecated on armv8
> +@ CHECK: warning: deprecated instruction
> +mcr p8, #0, r5, c7, c5, #4
> +@ CHECK: warning: deprecated instruction
> Index: tools/llvm-mc/Disassembler.cpp
> ===================================================================
> --- tools/llvm-mc/Disassembler.cpp
> +++ tools/llvm-mc/Disassembler.cpp
> @@ -19,6 +19,7 @@
>  #include "llvm/MC/MCInst.h"
>  #include "llvm/MC/MCStreamer.h"
>  #include "llvm/MC/MCSubtargetInfo.h"
> +#include "llvm/MC/MCInstrInfo.h"
>  #include "llvm/Support/MemoryBuffer.h"
>  #include "llvm/Support/MemoryObject.h"
>  #include "llvm/Support/SourceMgr.h"
> @@ -48,10 +49,10 @@
>  };
>  }
>  
> -static bool PrintInsts(const MCDisassembler &DisAsm,
> -                       const ByteArrayTy &Bytes,
> -                       SourceMgr &SM, raw_ostream &Out,
> -                       MCStreamer &Streamer, bool InAtomicBlock) {
> +static bool PrintInsts(const MCDisassembler &DisAsm, const ByteArrayTy &Bytes,
> +                       SourceMgr &SM, raw_ostream &Out, MCStreamer &Streamer,
> +                       bool InAtomicBlock, MCSubtargetInfo &STI,
> +                       MCInstrInfo &MII) {
>    // Wrap the vector in a MemoryObject.
>    VectorMemoryObject memoryObject(Bytes);
>  
> @@ -86,6 +87,8 @@
>        // Fall through
>  
>      case MCDisassembler::Success:
> +      if (MII.get(Inst.getOpcode()).isDeprecated(Inst, STI))
> +        Streamer.AddComment(Twine("DEPRECATED"));
>        Streamer.EmitInstruction(Inst);
>        break;
>      }
> @@ -153,7 +156,7 @@
>  
>  int Disassembler::disassemble(const Target &T,
>                                const std::string &Triple,
> -                              MCSubtargetInfo &STI,
> +                              MCSubtargetInfo &STI, MCInstrInfo &MII,
>                                MCStreamer &Streamer,
>                                MemoryBuffer &Buffer,
>                                SourceMgr &SM,
> @@ -202,7 +205,7 @@
>  
>      if (!ByteArray.empty())
>        ErrorOccurred |= PrintInsts(*DisAsm, ByteArray, SM, Out, Streamer,
> -                                  InAtomicBlock);
> +                                  InAtomicBlock, STI, MII);
>    }
>  
>    if (InAtomicBlock) {
> Index: tools/llvm-mc/Disassembler.h
> ===================================================================
> --- tools/llvm-mc/Disassembler.h
> +++ tools/llvm-mc/Disassembler.h
> @@ -24,13 +24,14 @@
>  class raw_ostream;
>  class SourceMgr;
>  class MCSubtargetInfo;
> +class MCInstrInfo;
>  class MCStreamer;
>  
>  class Disassembler {
>  public:
>    static int disassemble(const Target &T,
>                           const std::string &Triple,
> -                         MCSubtargetInfo &STI,
> +                         MCSubtargetInfo &STI, MCInstrInfo &MII,
>                           MCStreamer &Streamer,
>                           MemoryBuffer &Buffer,
>                           SourceMgr &SM,
> Index: tools/llvm-mc/llvm-mc.cpp
> ===================================================================
> --- tools/llvm-mc/llvm-mc.cpp
> +++ tools/llvm-mc/llvm-mc.cpp
> @@ -476,7 +476,7 @@
>      break;
>    }
>    if (disassemble)
> -    Res = Disassembler::disassemble(*TheTarget, TripleName, *STI, *Str,
> +    Res = Disassembler::disassemble(*TheTarget, TripleName, *STI, *MCII, *Str,
>                                      *Buffer, SrcMgr, Out->os());
>  
>    // Keep output if no errors.
> Index: utils/TableGen/CodeGenInstruction.cpp
> ===================================================================
> --- utils/TableGen/CodeGenInstruction.cpp
> +++ utils/TableGen/CodeGenInstruction.cpp
> @@ -336,6 +336,18 @@
>  
>    // Parse the DisableEncoding field.
>    Operands.ProcessDisableEncoding(R->getValueAsString("DisableEncoding"));
> +
> +  if (R->getValueAsString("CustomDeprecatedMethod") != "") {
> +    CustomDeprecator = true;
> +    DeprecatedReason = R->getValueAsString("CustomDeprecatedMethod");
> +  } else {
> +    CustomDeprecator = false;
> +    if (R->getValue("Dep")) {
> +      RecordVal *Dep = R->getValue("Dep");
> +      DeprecatedReason = Dep->getValue()->getAsString();
> +    } else
> +      DeprecatedReason = "";
> +  }
>  }
>  
>  /// HasOneImplicitDefWithKnownVT - If the instruction has at least one
> Index: utils/TableGen/CodeGenInstruction.h
> ===================================================================
> --- utils/TableGen/CodeGenInstruction.h
> +++ utils/TableGen/CodeGenInstruction.h
> @@ -248,6 +248,9 @@
>      bool isCodeGenOnly;
>      bool isPseudo;
>  
> +    std::string DeprecatedReason;
> +    bool CustomDeprecator;
> +
>      /// Are there any undefined flags?
>      bool hasUndefFlags() const {
>        return mayLoad_Unset || mayStore_Unset || hasSideEffects_Unset;
> Index: utils/TableGen/InstrInfoEmitter.cpp
> ===================================================================
> --- utils/TableGen/InstrInfoEmitter.cpp
> +++ utils/TableGen/InstrInfoEmitter.cpp
> @@ -514,6 +514,16 @@
>    else
>      OS << "OperandInfo" << OpInfo.find(OperandInfo)->second;
>  
> +  CodeGenTarget &Target = CDP.getTargetInfo();
> +  if (Inst.CustomDeprecator) {
> +    OS << ",0,&" << Inst.DeprecatedReason;
> +  } else {
> +    if (Inst.DeprecatedReason != "")
> +      OS << "," << Target.getInstNamespace() << "::" << Inst.DeprecatedReason << ",0";
> +    else
> +      OS << ",0,0";
> +  }
> +
>    OS << " },  // Inst #" << Num << " = " << Inst.TheDef->getName() << "\n";
>  }
>  

> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits




More information about the llvm-commits mailing list