Unify the target opcode enum in TargetOpcodes.h and the FixedInstrs array in CodeGenTarget.cpp to avoid the ordering dependence

[DavidKreitzer]

Summary

Quentin, is this what you had in mind with your FIXME's from r258333?

The one effect of this change worth noting is that PRE_ISEL_GENERIC_OPCODE_END no longer gets its own value in the TargetOpcode enum. I think that is harmless and probably desirable.

[qcolombet]

Hi David,

Thanks for doing this!
Yes, that was what I had in mind and that was on my todolist :).

LGTM.

Thanks,
-Quentin

include/llvm/Target/TargetOpcodes.h

 
 #ifndef LLVM_TARGET_TARGETOPCODES_H
 #define LLVM_TARGET_TARGETOPCODES_H
 
 namespace llvm {
 
 /// Invariant opcodes: All instruction sets have these as their low opcodes.
 ///
-/// Every instruction defined here must also appear in Target.td and the order
-/// must be the same as in CodeGenTarget.cpp.
-///
 namespace TargetOpcode {
 enum {
-  PHI = 0,
-  INLINEASM = 1,
-  CFI_INSTRUCTION = 2,
-  EH_LABEL = 3,
-  GC_LABEL = 4,
-
-  /// KILL - This instruction is a noop that is used only to adjust the
-  /// liveness of registers. This can be useful when dealing with
-  /// sub-registers.
-  KILL = 5,
-
-  /// EXTRACT_SUBREG - This instruction takes two operands: a register
-  /// that has subregisters, and a subregister index. It returns the
-  /// extracted subregister value. This is commonly used to implement
-  /// truncation operations on target architectures which support it.
-  EXTRACT_SUBREG = 6,
-
-  /// INSERT_SUBREG - This instruction takes three operands: a register that
-  /// has subregisters, a register providing an insert value, and a
-  /// subregister index. It returns the value of the first register with the
-  /// value of the second register inserted. The first register is often
-  /// defined by an IMPLICIT_DEF, because it is commonly used to implement
-  /// anyext operations on target architectures which support it.
-  INSERT_SUBREG = 7,
-
-  /// IMPLICIT_DEF - This is the MachineInstr-level equivalent of undef.
-  IMPLICIT_DEF = 8,
-
-  /// SUBREG_TO_REG - This instruction is similar to INSERT_SUBREG except that
-  /// the first operand is an immediate integer constant. This constant is
-  /// often zero, because it is commonly used to assert that the instruction
-  /// defining the register implicitly clears the high bits.
-  SUBREG_TO_REG = 9,
-
-  /// COPY_TO_REGCLASS - This instruction is a placeholder for a plain
-  /// register-to-register copy into a specific register class. This is only
-  /// used between instruction selection and MachineInstr creation, before
-  /// virtual registers have been created for all the instructions, and it's
-  /// only needed in cases where the register classes implied by the
-  /// instructions are insufficient. It is emitted as a COPY MachineInstr.
-  COPY_TO_REGCLASS = 10,
-
-  /// DBG_VALUE - a mapping of the llvm.dbg.value intrinsic
-  DBG_VALUE = 11,
-
-  /// REG_SEQUENCE - This variadic instruction is used to form a register that
-  /// represents a consecutive sequence of sub-registers. It's used as a
-  /// register coalescing / allocation aid and must be eliminated before code
-  /// emission.
-  // In SDNode form, the first operand encodes the register class created by
-  // the REG_SEQUENCE, while each subsequent pair names a vreg + subreg index
-  // pair.  Once it has been lowered to a MachineInstr, the regclass operand
-  // is no longer present.
-  /// e.g. v1027 = REG_SEQUENCE v1024, 3, v1025, 4, v1026, 5
-  /// After register coalescing references of v1024 should be replace with
-  /// v1027:3, v1025 with v1027:4, etc.
-  REG_SEQUENCE = 12,
-
-  /// COPY - Target-independent register copy. This instruction can also be
-  /// used to copy between subregisters of virtual registers.
-  COPY = 13,
-
-  /// BUNDLE - This instruction represents an instruction bundle. Instructions
-  /// which immediately follow a BUNDLE instruction which are marked with
-  /// 'InsideBundle' flag are inside the bundle.
-  BUNDLE = 14,
-
-  /// Lifetime markers.
-  LIFETIME_START = 15,
-  LIFETIME_END = 16,
-
-  /// A Stackmap instruction captures the location of live variables at its
-  /// position in the instruction stream. It is followed by a shadow of bytes
-  /// that must lie within the function and not contain another stackmap.
-  STACKMAP = 17,
-
-  /// Patchable call instruction - this instruction represents a call to a
-  /// constant address, followed by a series of NOPs. It is intended to
-  /// support optimizations for dynamic languages (such as javascript) that
-  /// rewrite calls to runtimes with more efficient code sequences.
-  /// This also implies a stack map.
-  PATCHPOINT = 18,
-
-  /// This pseudo-instruction loads the stack guard value. Targets which need
-  /// to prevent the stack guard value or address from being spilled to the
-  /// stack should override TargetLowering::emitLoadStackGuardNode and
-  /// additionally expand this pseudo after register allocation.
-  LOAD_STACK_GUARD = 19,
-
-  /// Call instruction with associated vm state for deoptimization and list
-  /// of live pointers for relocation by the garbage collector.  It is
-  /// intended to support garbage collection with fully precise relocating
-  /// collectors and deoptimizations in either the callee or caller.
-  STATEPOINT = 20,
-
-  /// Instruction that records the offset of a local stack allocation passed to
-  /// llvm.localescape. It has two arguments: the symbol for the label and the
-  /// frame index of the local stack allocation.
-  LOCAL_ESCAPE = 21,
-
-  /// Loading instruction that may page fault, bundled with associated
-  /// information on how to handle such a page fault.  It is intended to support
-  /// "zero cost" null checks in managed languages by allowing LLVM to fold
-  /// comparisons into existing memory operations.
-  FAULTING_LOAD_OP = 22,
-
-  /// The following generic opcodes are not supposed to appear after ISel.
-  /// This is something we might want to relax, but for now, this is convenient
-  /// to produce diagnostic.
-  PRE_ISEL_GENERIC_OPCODE_START,
-  // Generic opcodes used before ISel are here.
-
-  /// Generic ADD instruction. This is an integer add.
-  G_ADD = PRE_ISEL_GENERIC_OPCODE_START,
-  // TODO: Add more generic opcodes as we move along.
-  // FIXME: Right now, we have to manually add any new opcode in
-  // CodeGenTarget.cpp for TableGen to pick them up.
-  // Moreover the order must match.
-
-  /// Marker for the end of the generic opcode.
-  /// This is used to check if an opcode is in the range of the
-  /// generic opcodes.
-  PRE_ISEL_GENERIC_OPCODE_END,
-
-  /// BUILTIN_OP_END - This must be the last enum value in this list.
-  /// The target-specific post-isel opcode values start here.
-  GENERIC_OP_END = PRE_ISEL_GENERIC_OPCODE_END,
+#define HANDLE_TARGET_OPCODE(OPC, NUM) OPC = NUM,
+#define HANDLE_TARGET_OPCODE_MARKER(IDENT, OPC) IDENT = OPC,
+#include "llvm/Target/TargetOpcodes.def"
 };
 } // end namespace TargetOpcode
 } // end namespace llvm
 
 #endif

include/llvm/Target/TargetOpcodes.def

+//===-- llvm/Target/TargetOpcodes.def - Target Indep Opcodes ------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines the target independent instruction opcodes.
+//
+//===----------------------------------------------------------------------===//
+
+// NOTE: NO INCLUDE GUARD DESIRED!
+
+/// HANDLE_TARGET_OPCODE defines an opcode and its associated enum value.
+///
+#ifndef HANDLE_TARGET_OPCODE
+#define HANDLE_TARGET_OPCODE(OPC, NUM)
+#endif
+
+/// HANDLE_TARGET_OPCODE_MARKER defines an alternative identifier for an opcode.
+///
+#ifndef HANDLE_TARGET_OPCODE_MARKER
+#define HANDLE_TARGET_OPCODE_MARKER(IDENT, OPC)
+#endif
+
+/// Every instruction defined here must also appear in Target.td.
+///
+HANDLE_TARGET_OPCODE(PHI, 0)
+HANDLE_TARGET_OPCODE(INLINEASM, 1)
+HANDLE_TARGET_OPCODE(CFI_INSTRUCTION, 2)
+HANDLE_TARGET_OPCODE(EH_LABEL, 3)
+HANDLE_TARGET_OPCODE(GC_LABEL, 4)
+
+/// KILL - This instruction is a noop that is used only to adjust the
+/// liveness of registers. This can be useful when dealing with
+/// sub-registers.
+HANDLE_TARGET_OPCODE(KILL, 5)
+
+/// EXTRACT_SUBREG - This instruction takes two operands: a register
+/// that has subregisters, and a subregister index. It returns the
+/// extracted subregister value. This is commonly used to implement
+/// truncation operations on target architectures which support it.
+HANDLE_TARGET_OPCODE(EXTRACT_SUBREG, 6)
+
+/// INSERT_SUBREG - This instruction takes three operands: a register that
+/// has subregisters, a register providing an insert value, and a
+/// subregister index. It returns the value of the first register with the
+/// value of the second register inserted. The first register is often
+/// defined by an IMPLICIT_DEF, because it is commonly used to implement
+/// anyext operations on target architectures which support it.
+HANDLE_TARGET_OPCODE(INSERT_SUBREG, 7)
+
+/// IMPLICIT_DEF - This is the MachineInstr-level equivalent of undef.
+HANDLE_TARGET_OPCODE(IMPLICIT_DEF, 8)
+
+/// SUBREG_TO_REG - This instruction is similar to INSERT_SUBREG except that
+/// the first operand is an immediate integer constant. This constant is
+/// often zero, because it is commonly used to assert that the instruction
+/// defining the register implicitly clears the high bits.
+HANDLE_TARGET_OPCODE(SUBREG_TO_REG, 9)
+
+/// COPY_TO_REGCLASS - This instruction is a placeholder for a plain
+/// register-to-register copy into a specific register class. This is only
+/// used between instruction selection and MachineInstr creation, before
+/// virtual registers have been created for all the instructions, and it's
+/// only needed in cases where the register classes implied by the
+/// instructions are insufficient. It is emitted as a COPY MachineInstr.
+HANDLE_TARGET_OPCODE(COPY_TO_REGCLASS, 10)
+
+/// DBG_VALUE - a mapping of the llvm.dbg.value intrinsic
+HANDLE_TARGET_OPCODE(DBG_VALUE, 11)
+
+/// REG_SEQUENCE - This variadic instruction is used to form a register that
+/// represents a consecutive sequence of sub-registers. It's used as a
+/// register coalescing / allocation aid and must be eliminated before code
+/// emission.
+// In SDNode form, the first operand encodes the register class created by
+// the REG_SEQUENCE, while each subsequent pair names a vreg + subreg index
+// pair.  Once it has been lowered to a MachineInstr, the regclass operand
+// is no longer present.
+/// e.g. v1027 = REG_SEQUENCE v1024, 3, v1025, 4, v1026, 5
+/// After register coalescing references of v1024 should be replace with
+/// v1027:3, v1025 with v1027:4, etc.
+HANDLE_TARGET_OPCODE(REG_SEQUENCE, 12)
+
+/// COPY - Target-independent register copy. This instruction can also be
+/// used to copy between subregisters of virtual registers.
+HANDLE_TARGET_OPCODE(COPY, 13)
+
+/// BUNDLE - This instruction represents an instruction bundle. Instructions
+/// which immediately follow a BUNDLE instruction which are marked with
+/// 'InsideBundle' flag are inside the bundle.
+HANDLE_TARGET_OPCODE(BUNDLE, 14)
+
+/// Lifetime markers.
+HANDLE_TARGET_OPCODE(LIFETIME_START, 15)
+HANDLE_TARGET_OPCODE(LIFETIME_END, 16)
+
+/// A Stackmap instruction captures the location of live variables at its
+/// position in the instruction stream. It is followed by a shadow of bytes
+/// that must lie within the function and not contain another stackmap.
+HANDLE_TARGET_OPCODE(STACKMAP, 17)
+
+/// Patchable call instruction - this instruction represents a call to a
+/// constant address, followed by a series of NOPs. It is intended to
+/// support optimizations for dynamic languages (such as javascript) that
+/// rewrite calls to runtimes with more efficient code sequences.
+/// This also implies a stack map.
+HANDLE_TARGET_OPCODE(PATCHPOINT, 18)
+
+/// This pseudo-instruction loads the stack guard value. Targets which need
+/// to prevent the stack guard value or address from being spilled to the
+/// stack should override TargetLowering::emitLoadStackGuardNode and
+/// additionally expand this pseudo after register allocation.
+HANDLE_TARGET_OPCODE(LOAD_STACK_GUARD, 19)
+
+/// Call instruction with associated vm state for deoptimization and list
+/// of live pointers for relocation by the garbage collector.  It is
+/// intended to support garbage collection with fully precise relocating
+/// collectors and deoptimizations in either the callee or caller.
+HANDLE_TARGET_OPCODE(STATEPOINT, 20)
+
+/// Instruction that records the offset of a local stack allocation passed to
+/// llvm.localescape. It has two arguments: the symbol for the label and the
+/// frame index of the local stack allocation.
+HANDLE_TARGET_OPCODE(LOCAL_ESCAPE, 21)
+
+/// Loading instruction that may page fault, bundled with associated
+/// information on how to handle such a page fault.  It is intended to support
+/// "zero cost" null checks in managed languages by allowing LLVM to fold
+/// comparisons into existing memory operations.
+HANDLE_TARGET_OPCODE(FAULTING_LOAD_OP, 22)
+
+/// The following generic opcodes are not supposed to appear after ISel.
+/// This is something we might want to relax, but for now, this is convenient
+/// to produce diagnostics.
+
+/// Generic ADD instruction. This is an integer add.
+HANDLE_TARGET_OPCODE(G_ADD, 23)
+HANDLE_TARGET_OPCODE_MARKER(PRE_ISEL_GENERIC_OPCODE_START, G_ADD)
+
+// TODO: Add more generic opcodes as we move along.
+
+/// Marker for the end of the generic opcode.
+/// This is used to check if an opcode is in the range of the
+/// generic opcodes.
+HANDLE_TARGET_OPCODE_MARKER(PRE_ISEL_GENERIC_OPCODE_END, G_ADD)
+
+/// BUILTIN_OP_END - This must be the last enum value in this list.
+/// The target-specific post-isel opcode values start here.
+HANDLE_TARGET_OPCODE_MARKER(GENERIC_OP_END, PRE_ISEL_GENERIC_OPCODE_END)

utils/TableGen/CodeGenTarget.cpp

GetInstByName(const char *Name,
   if (!Rec || I == Insts.end())
     PrintFatalError(Twine("Could not find '") + Name + "' instruction!");
   return I->second.get();
 }
 
 /// \brief Return all of the instructions defined by the target, ordered by
 /// their enum value.
 void CodeGenTarget::ComputeInstrsByEnum() const {
-  // The ordering here must match the ordering in TargetOpcodes.h.
-  // FIXME: It would be nice to have the opcode directly extracted
-  // to avoid potential errors. At the very, least a compile time
-  // error would be appreciated if the order does not match.
   static const char *const FixedInstrs[] = {
-      "PHI",          "INLINEASM",     "CFI_INSTRUCTION",  "EH_LABEL",
-      "GC_LABEL",     "KILL",          "EXTRACT_SUBREG",   "INSERT_SUBREG",
-      "IMPLICIT_DEF", "SUBREG_TO_REG", "COPY_TO_REGCLASS", "DBG_VALUE",
-      "REG_SEQUENCE", "COPY",          "BUNDLE",           "LIFETIME_START",
-      "LIFETIME_END", "STACKMAP",      "PATCHPOINT",       "LOAD_STACK_GUARD",
-      "STATEPOINT",   "LOCAL_ESCAPE",   "FAULTING_LOAD_OP",
-      // Generic opcodes for GlobalISel start here.
-      "G_ADD",
+#define HANDLE_TARGET_OPCODE(OPC, NUM) #OPC,
+#include "llvm/Target/TargetOpcodes.def"
       nullptr};
   const auto &Insts = getInstructions();
   for (const char *const *p = FixedInstrs; *p; ++p) {
     const CodeGenInstruction *Instr = GetInstByName(*p, Insts, Records);
     assert(Instr && "Missing target independent instruction");
     assert(Instr->Namespace == "TargetOpcode" && "Bad namespace");
     InstrsByEnum.push_back(Instr);
   }