[llvm] d8ce50e - [MC] Store number of implicit operands in MCInstrDesc. NFC.

Jay Foad via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 24 13:23:34 PST 2023


Author: Jay Foad
Date: 2023-01-24T21:23:27Z
New Revision: d8ce50e3c25f667a10750d1129a1b8a060d43492

URL: https://github.com/llvm/llvm-project/commit/d8ce50e3c25f667a10750d1129a1b8a060d43492
DIFF: https://github.com/llvm/llvm-project/commit/d8ce50e3c25f667a10750d1129a1b8a060d43492.diff

LOG: [MC] Store number of implicit operands in MCInstrDesc. NFC.

Combine the implicit uses and defs lists into a single list of uses
followed by defs. Instead of 0-terminating the list, store the number
of uses and defs. This avoids having to scan the whole list to find the
length and removes one pointer from MCInstrDesc (although it does not
get any smaller due to alignment issues).

Remove the old accessor methods getImplicitUses, getNumImplicitUses,
getImplicitDefs and getNumImplicitDefs as all clients are using the new
implicit_uses and implicit_defs.

Differential Revision: https://reviews.llvm.org/D142216

Added: 
    

Modified: 
    llvm/include/llvm/MC/MCInstrDesc.h
    llvm/unittests/CodeGen/MachineInstrTest.cpp
    llvm/unittests/CodeGen/RegAllocScoreTest.cpp
    llvm/utils/TableGen/InstrInfoEmitter.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/MC/MCInstrDesc.h b/llvm/include/llvm/MC/MCInstrDesc.h
index 3bba10501b68d..7719cd0815307 100644
--- a/llvm/include/llvm/MC/MCInstrDesc.h
+++ b/llvm/include/llvm/MC/MCInstrDesc.h
@@ -207,10 +207,11 @@ class MCInstrDesc {
   unsigned char NumDefs;         // Num of args that are definitions
   unsigned char Size;            // Number of bytes in encoding.
   unsigned short SchedClass;     // enum identifying instr sched class
+  unsigned char NumImplicitUses; // Num of regs implicitly used
+  unsigned char NumImplicitDefs; // Num of regs implicitly defined
   uint64_t Flags;                // Flags identifying machine instr class
   uint64_t TSFlags;              // Target Specific Flag values
-  const MCPhysReg *ImplicitUses; // Registers implicitly read by this instr
-  const MCPhysReg *ImplicitDefs; // Registers implicitly defined by this instr
+  const MCPhysReg *ImplicitOps;  // List of implicit uses followed by defs
   const MCOperandInfo *OpInfo;   // 'NumOperands' entries about operands
 
   /// Returns the value of the specified operand constraint if
@@ -566,21 +567,8 @@ class MCInstrDesc {
   /// flags register.  In this case, the instruction is marked as implicitly
   /// reading the flags.  Likewise, the variable shift instruction on X86 is
   /// marked as implicitly reading the 'CL' register, which it always does.
-  ///
-  /// This method returns null if the instruction has no implicit uses.
-  const MCPhysReg *getImplicitUses() const { return ImplicitUses; }
   ArrayRef<MCPhysReg> implicit_uses() const {
-    return {ImplicitUses, getNumImplicitUses()};
-  }
-
-  /// Return the number of implicit uses this instruction has.
-  unsigned getNumImplicitUses() const {
-    if (!ImplicitUses)
-      return 0;
-    unsigned i = 0;
-    for (; ImplicitUses[i]; ++i) /*empty*/
-      ;
-    return i;
+    return {ImplicitOps, NumImplicitUses};
   }
 
   /// Return a list of registers that are potentially written by any
@@ -591,21 +579,8 @@ class MCInstrDesc {
   /// instruction always deposits the quotient and remainder in the EAX/EDX
   /// registers.  For that instruction, this will return a list containing the
   /// EAX/EDX/EFLAGS registers.
-  ///
-  /// This method returns null if the instruction has no implicit defs.
-  const MCPhysReg *getImplicitDefs() const { return ImplicitDefs; }
   ArrayRef<MCPhysReg> implicit_defs() const {
-    return {ImplicitDefs, getNumImplicitDefs()};
-  }
-
-  /// Return the number of implicit defs this instruct has.
-  unsigned getNumImplicitDefs() const {
-    if (!ImplicitDefs)
-      return 0;
-    unsigned i = 0;
-    for (; ImplicitDefs[i]; ++i) /*empty*/
-      ;
-    return i;
+    return {ImplicitOps + NumImplicitUses, NumImplicitDefs};
   }
 
   /// Return true if this instruction implicitly

diff  --git a/llvm/unittests/CodeGen/MachineInstrTest.cpp b/llvm/unittests/CodeGen/MachineInstrTest.cpp
index 9313bb699d4c4..10c29b5f837cb 100644
--- a/llvm/unittests/CodeGen/MachineInstrTest.cpp
+++ b/llvm/unittests/CodeGen/MachineInstrTest.cpp
@@ -54,9 +54,9 @@ TEST(IsIdenticalToTest, DifferentDefs) {
   MCOperandInfo OpInfo[] = {
       {0, 0, MCOI::OPERAND_REGISTER, 0},
       {0, 1 << MCOI::OptionalDef, MCOI::OPERAND_REGISTER, 0}};
-  MCInstrDesc MCID = {
-      0, NumOps,  NumDefs, 0,     0, 1ULL << MCID::HasOptionalDef,
-      0, nullptr, nullptr, OpInfo};
+  MCInstrDesc MCID = {0, NumOps,  NumDefs, 0,
+                      0, 0,       0,       1ULL << MCID::HasOptionalDef,
+                      0, nullptr, OpInfo};
 
   // Create two MIs with 
diff erent virtual reg defs and the same uses.
   unsigned VirtualDef1 = -42; // The value doesn't matter, but the sign does.
@@ -125,9 +125,9 @@ TEST(MachineInstrExpressionTraitTest, IsEqualAgreesWithGetHashValue) {
   MCOperandInfo OpInfo[] = {
       {0, 0, MCOI::OPERAND_REGISTER, 0},
       {0, 1 << MCOI::OptionalDef, MCOI::OPERAND_REGISTER, 0}};
-  MCInstrDesc MCID = {
-      0, NumOps,  NumDefs, 0,     0, 1ULL << MCID::HasOptionalDef,
-      0, nullptr, nullptr, OpInfo};
+  MCInstrDesc MCID = {0, NumOps,  NumDefs, 0,
+                      0, 0,       0,       1ULL << MCID::HasOptionalDef,
+                      0, nullptr, OpInfo};
 
   // Define a series of instructions with 
diff erent kinds of operands and make
   // sure that the hash function is consistent with isEqual for various
@@ -201,7 +201,7 @@ TEST(MachineInstrPrintingTest, DebugLocPrinting) {
   auto MF = createMachineFunction(Ctx, Mod);
 
   MCOperandInfo OpInfo{0, 0, MCOI::OPERAND_REGISTER, 0};
-  MCInstrDesc MCID = {0, 1, 1, 0, 0, 0, 0, nullptr, nullptr, &OpInfo};
+  MCInstrDesc MCID = {0, 1, 1, 0, 0, 0, 0, 0, 0, nullptr, &OpInfo};
 
   DIFile *DIF = DIFile::getDistinct(Ctx, "filename", "");
   DISubprogram *DIS = DISubprogram::getDistinct(
@@ -228,7 +228,7 @@ TEST(MachineInstrSpan, DistanceBegin) {
   auto MF = createMachineFunction(Ctx, Mod);
   auto MBB = MF->CreateMachineBasicBlock();
 
-  MCInstrDesc MCID = {0, 0, 0, 0, 0, 0, 0, nullptr, nullptr, nullptr};
+  MCInstrDesc MCID = {0, 0, 0, 0, 0, 0, 0, 0, 0, nullptr, nullptr};
 
   auto MII = MBB->begin();
   MachineInstrSpan MIS(MII, MBB);
@@ -245,7 +245,7 @@ TEST(MachineInstrSpan, DistanceEnd) {
   auto MF = createMachineFunction(Ctx, Mod);
   auto MBB = MF->CreateMachineBasicBlock();
 
-  MCInstrDesc MCID = {0, 0, 0, 0, 0, 0, 0, nullptr, nullptr, nullptr};
+  MCInstrDesc MCID = {0, 0, 0, 0, 0, 0, 0, 0, 0, nullptr, nullptr};
 
   auto MII = MBB->end();
   MachineInstrSpan MIS(MII, MBB);
@@ -260,7 +260,7 @@ TEST(MachineInstrExtraInfo, AddExtraInfo) {
   LLVMContext Ctx;
   Module Mod("Module", Ctx);
   auto MF = createMachineFunction(Ctx, Mod);
-  MCInstrDesc MCID = {0, 0, 0, 0, 0, 0, 0, nullptr, nullptr, nullptr};
+  MCInstrDesc MCID = {0, 0, 0, 0, 0, 0, 0, 0, 0, nullptr, nullptr};
 
   auto MI = MF->CreateMachineInstr(MCID, DebugLoc());
   auto MAI = MCAsmInfo();
@@ -320,7 +320,7 @@ TEST(MachineInstrExtraInfo, ChangeExtraInfo) {
   LLVMContext Ctx;
   Module Mod("Module", Ctx);
   auto MF = createMachineFunction(Ctx, Mod);
-  MCInstrDesc MCID = {0, 0, 0, 0, 0, 0, 0, nullptr, nullptr, nullptr};
+  MCInstrDesc MCID = {0, 0, 0, 0, 0, 0, 0, 0, 0, nullptr, nullptr};
 
   auto MI = MF->CreateMachineInstr(MCID, DebugLoc());
   auto MAI = MCAsmInfo();
@@ -361,7 +361,7 @@ TEST(MachineInstrExtraInfo, RemoveExtraInfo) {
   LLVMContext Ctx;
   Module Mod("Module", Ctx);
   auto MF = createMachineFunction(Ctx, Mod);
-  MCInstrDesc MCID = {0, 0, 0, 0, 0, 0, 0, nullptr, nullptr, nullptr};
+  MCInstrDesc MCID = {0, 0, 0, 0, 0, 0, 0, 0, 0, nullptr, nullptr};
 
   auto MI = MF->CreateMachineInstr(MCID, DebugLoc());
   auto MAI = MCAsmInfo();
@@ -428,9 +428,11 @@ TEST(MachineInstrDebugValue, AddDebugValueOperand) {
         TargetOpcode::DBG_INSTR_REF, TargetOpcode::DBG_PHI,
         TargetOpcode::DBG_LABEL}) {
     const MCInstrDesc MCID = {
-        Opcode, 0,       0,
-        0,      0,       (1ULL << MCID::Pseudo) | (1ULL << MCID::Variadic),
-        0,      nullptr, nullptr,
+        Opcode, 0,
+        0,      0,
+        0,      0,
+        0,      (1ULL << MCID::Pseudo) | (1ULL << MCID::Variadic),
+        0,      nullptr,
         nullptr};
 
     auto *MI = MF->CreateMachineInstr(MCID, DebugLoc());
@@ -461,7 +463,7 @@ TEST(MachineInstrBuilder, BuildMI) {
   Module Mod("Module", Ctx);
   auto MF = createMachineFunction(Ctx, Mod);
   auto MBB = MF->CreateMachineBasicBlock();
-  MCInstrDesc MCID = {0, 0, 0, 0, 0, 0, 0, nullptr, nullptr, nullptr};
+  MCInstrDesc MCID = {0, 0, 0, 0, 0, 0, 0, 0, 0, nullptr, nullptr};
   EXPECT_THAT(BuildMI(*MF, MIMD, MCID), HasMIMetadata(MIMD));
   EXPECT_THAT(BuildMI(*MF, MIMD, MCID), HasMIMetadata(MIMD));
   EXPECT_THAT(BuildMI(*MBB, MBB->end(), MIMD, MCID), HasMIMetadata(MIMD));

diff  --git a/llvm/unittests/CodeGen/RegAllocScoreTest.cpp b/llvm/unittests/CodeGen/RegAllocScoreTest.cpp
index ae5db2d8a24e1..73cbfa17be012 100644
--- a/llvm/unittests/CodeGen/RegAllocScoreTest.cpp
+++ b/llvm/unittests/CodeGen/RegAllocScoreTest.cpp
@@ -64,7 +64,7 @@ enum MockInstrId {
 
 const std::array<MCInstrDesc, MockInstrId::TotalMockInstrs> MockInstrDescs{{
 #define MOCK_SPEC(IGNORE, OPCODE, FLAGS)                                       \
-  {OPCODE, 0, 0, 0, 0, FLAGS, 0, nullptr, nullptr, nullptr},
+  {OPCODE, 0, 0, 0, 0, 0, 0, FLAGS, 0, nullptr, nullptr},
     MOCK_INSTR(MOCK_SPEC)
 #undef MOCK_SPEC
 }};

diff  --git a/llvm/utils/TableGen/InstrInfoEmitter.cpp b/llvm/utils/TableGen/InstrInfoEmitter.cpp
index a378bd46ccf14..aafbe932a212f 100644
--- a/llvm/utils/TableGen/InstrInfoEmitter.cpp
+++ b/llvm/utils/TableGen/InstrInfoEmitter.cpp
@@ -117,9 +117,9 @@ class InstrInfoEmitter {
 static void PrintDefList(const std::vector<Record*> &Uses,
                          unsigned Num, raw_ostream &OS) {
   OS << "static const MCPhysReg ImplicitList" << Num << "[] = { ";
-  for (Record *U : Uses)
-    OS << getQualifiedName(U) << ", ";
-  OS << "0 };\n";
+  for (auto [Idx, U] : enumerate(Uses))
+    OS << (Idx ? ", " : "") << getQualifiedName(U);
+  OS << " };\n";
 }
 
 //===----------------------------------------------------------------------===//
@@ -907,18 +907,13 @@ void InstrInfoEmitter::run(raw_ostream &OS) {
   // Emit all of the instruction's implicit uses and defs.
   Records.startTimer("Emit uses/defs");
   for (const CodeGenInstruction *II : Target.getInstructionsByEnumValue()) {
-    if (!II->ImplicitUses.empty()) {
-      unsigned &IL = EmittedLists[II->ImplicitUses];
+    std::vector<Record *> ImplicitOps = II->ImplicitUses;
+    llvm::append_range(ImplicitOps, II->ImplicitDefs);
+    if (!ImplicitOps.empty()) {
+      unsigned &IL = EmittedLists[ImplicitOps];
       if (!IL) {
         IL = ++ListNumber;
-        PrintDefList(II->ImplicitUses, IL, OS);
-      }
-    }
-    if (!II->ImplicitDefs.empty()) {
-      unsigned &IL = EmittedLists[II->ImplicitDefs];
-      if (!IL) {
-        IL = ++ListNumber;
-        PrintDefList(II->ImplicitDefs, IL, OS);
+        PrintDefList(ImplicitOps, IL, OS);
       }
     }
   }
@@ -1122,7 +1117,9 @@ void InstrInfoEmitter::emitRecord(const CodeGenInstruction &Inst, unsigned Num,
   OS << Num << ",\t" << MinOperands << ",\t"
      << Inst.Operands.NumDefs << ",\t"
      << Inst.TheDef->getValueAsInt("Size") << ",\t"
-     << SchedModels.getSchedClassIdx(Inst) << ",\t0";
+     << SchedModels.getSchedClassIdx(Inst) << ",\t"
+     << Inst.ImplicitUses.size() << ",\t"
+     << Inst.ImplicitDefs.size() << ",\t0";
 
   CodeGenTarget &Target = CDP.getTargetInfo();
 
@@ -1187,16 +1184,13 @@ void InstrInfoEmitter::emitRecord(const CodeGenInstruction &Inst, unsigned Num,
   OS.write_hex(Value);
   OS << "ULL, ";
 
-  // Emit the implicit uses and defs lists...
-  if (Inst.ImplicitUses.empty())
-    OS << "nullptr, ";
-  else
-    OS << "ImplicitList" << EmittedLists[Inst.ImplicitUses] << ", ";
-
-  if (Inst.ImplicitDefs.empty())
+  // Emit the implicit use/def list...
+  std::vector<Record *> ImplicitOps = Inst.ImplicitUses;
+  llvm::append_range(ImplicitOps, Inst.ImplicitDefs);
+  if (ImplicitOps.empty())
     OS << "nullptr, ";
   else
-    OS << "ImplicitList" << EmittedLists[Inst.ImplicitDefs] << ", ";
+    OS << "ImplicitList" << EmittedLists[ImplicitOps] << ", ";
 
   // Emit the operand info.
   std::vector<std::string> OperandInfo = GetOperandInfo(Inst);


        


More information about the llvm-commits mailing list