[MachineInst] Switch NumOperands to 16bits

[xbolva00]

Summary

Decrease NumOperands from 32 to 16bits (matches MCInstrDesc) so we can use saved bits to extend Flags (https://reviews.llvm.org/D118118).

[xbolva00]

llvm/lib/CodeGen/MachineInstr.cpp

744

MCID:
unsigned short NumOperands;

so OK.

[nikic]

Change seems conceptually fine, but I don't really know anything about the machine layer and couldn't say whether this is safe or not.

[xbolva00]

Use USHRT_MAX. Added check.

[craig.topper]

llvm/include/llvm/CodeGen/MachineInstr.h

519

You can leave this returning unsigned

626

Leave this returning unsigned

llvm/lib/CodeGen/MachineFunction.cpp

937 ↗

(On Diff #517909)

Leave this as unsigned.

[xbolva00]

Addressed review feedback.

[xbolva00]

any futher comments?

[barannikov88]

Looks fine. Just in case, in D118118 you will need to change the order of fields in order to avoid padding between them.

[jroelofs]

This change broke the build for an internal project of ours (breadcrumb: rdar://109362033). The relevant code being compiled is a fairly large nested switch that results in a PHI node with 65k+ operands, which can't easily be turned into a table for perf reasons.

WDYT about making it the lower 24 bits of a uint32_t bitfield shared with AsmPrinterFlags in the upper 8?

[barannikov88]

In D149445#4441593, @jroelofs wrote:

This change broke the build for an internal project of ours (breadcrumb: rdar://109362033). The relevant code being compiled is a fairly large nested switch that results in a PHI node with 65k+ operands, which can't easily be turned into a table for perf reasons.

WDYT about making it the lower 24 bits of a uint32_t bitfield shared with AsmPrinterFlags in the upper 8?

Sounds fine to me

[jroelofs]

In D149445#4446238, @barannikov88 wrote:

In D149445#4441593, @jroelofs wrote:

This change broke the build for an internal project of ours (breadcrumb: rdar://109362033). The relevant code being compiled is a fairly large nested switch that results in a PHI node with 65k+ operands, which can't easily be turned into a table for perf reasons.

WDYT about making it the lower 24 bits of a uint32_t bitfield shared with AsmPrinterFlags in the upper 8?

Sounds fine to me

https://reviews.llvm.org/D153791

llvm/include/llvm/CodeGen/MachineInstr.h

public:
   };
 
 private:
   const MCInstrDesc *MCID;              // Instruction descriptor.
   MachineBasicBlock *Parent = nullptr;  // Pointer to the owning basic block.
 
   // Operands are allocated by an ArrayRecycler.
   MachineOperand *Operands = nullptr;   // Pointer to the first operand.
-  unsigned NumOperands = 0;             // Number of operands on instruction.
+  uint16_t NumOperands = 0;             // Number of operands on instruction.
 
   uint16_t Flags = 0;                   // Various bits of additional
                                         // information about machine
                                         // instruction.
 
   uint8_t AsmPrinterFlags = 0;          // Various bits of information used by
                                         // the AsmPrinter to emit helpful
                                         // comments.  This is *not* semantic
public:
 
   /// Returns the target instruction descriptor of this MachineInstr.
   const MCInstrDesc &getDesc() const { return *MCID; }
 
   /// Returns the opcode of this MachineInstr.
   unsigned getOpcode() const { return MCID->Opcode; }
 
   /// Retuns the total number of operands.
   unsigned getNumOperands() const { return NumOperands; }
 
   /// Returns the total number of operands which are debug locations.
   unsigned getNumDebugOperands() const {
     return std::distance(debug_operands().begin(), debug_operands().end());
   }
 
   const MachineOperand& getOperand(unsigned i) const {
     assert(i < getNumOperands() && "getOperand() out of range!");
bool isOperandSubregIdx(unsigned OpIdx) const {
     if (isRegSequence() && OpIdx > 1 && (OpIdx % 2) == 0)
       return true;
     if (isSubregToReg() && OpIdx == 3)
       return true;
     return false;
   }
 
   /// Returns the number of non-implicit operands.
   unsigned getNumExplicitOperands() const;
 
   /// Returns the number of non-implicit definitions.
   unsigned getNumExplicitDefs() const;
 
   /// iterator/begin/end - Iterate over all operands of a machine instruction.
   using mop_iterator = MachineOperand *;
   using const_mop_iterator = const MachineOperand *;
 

llvm/lib/CodeGen/MachineInstr.cpp

static void moveOperands(MachineOperand *Dst, MachineOperand *Src,
   std::memmove(Dst, Src, NumOps * sizeof(MachineOperand));
 }
 
 /// addOperand - Add the specified operand to the instruction.  If it is an
 /// implicit operand, it is added to the end of the operand list.  If it is
 /// an explicit operand it is added at the end of the explicit operand list
 /// (before the first implicit operand).
 void MachineInstr::addOperand(MachineFunction &MF, const MachineOperand &Op) {
+  assert(NumOperands < USHRT_MAX && "Cannot add more operands.");
   assert(MCID && "Cannot add operands before providing an instr descriptor");
 
   // Check if we're adding one of our existing operands.
   if (&Op >= Operands && &Op < Operands + NumOperands) {
     // This is unusual: MI->addOperand(MI->getOperand(i)).
     // If adding Op requires reallocating or moving existing operands around,
     // the Op reference could go stale. Support it by copying Op.
     MachineOperand CopyOp(Op);

 bool MachineInstr::shouldUpdateCallSiteInfo() const {
   if (isBundle())
     return isCandidateForCallSiteEntry(MachineInstr::AnyInBundle);
   return isCandidateForCallSiteEntry();
 }
 
 unsigned MachineInstr::getNumExplicitOperands() const {
   unsigned NumOperands = MCID->getNumOperands();
   if (!MCID->isVariadic())
     return NumOperands;
 
   for (unsigned I = NumOperands, E = getNumOperands(); I != E; ++I) {
     const MachineOperand &MO = getOperand(I);
     // The operands must always be in the following order:
     // - explicit reg defs,
     // - other explicit operands (reg uses, immediates, etc.),
     // - implicit reg defs