From 703c08362adcc7990fa5ddc444c41c5efdccbc2b Mon Sep 17 00:00:00 2001 From: Jon Roelofs Date: Mon, 26 Jun 2023 10:31:57 -0700 Subject: [PATCH] [MachineInst] Bump NumOperands back up to 24bits In https://reviews.llvm.org/D149445, it was lowered from 32 to 16bits, which broke an internal project of ours. 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. This change unifies `NumOperands`, `Flags`, and `AsmPrinterFlags` into a packed 7-byte struct, which `CapOperands` can follow as the 8th byte, rounding it up to a nice alignment before the `Info` field. rdar://111217742&109362033 Differential revision: https://reviews.llvm.org/D153791 --- llvm/include/llvm/CodeGen/MachineInstr.h | 44 +++++++++++++++++++++++--------- llvm/lib/CodeGen/MachineInstr.cpp | 10 +++++--- 2 files changed, 38 insertions(+), 16 deletions(-) diff --git a/llvm/include/llvm/CodeGen/MachineInstr.h b/llvm/include/llvm/CodeGen/MachineInstr.h index fa287be..2928ccf 100644 --- a/llvm/include/llvm/CodeGen/MachineInstr.h +++ b/llvm/include/llvm/CodeGen/MachineInstr.h @@ -29,6 +29,7 @@ #include "llvm/MC/MCInstrDesc.h" #include "llvm/MC/MCSymbol.h" #include "llvm/Support/ArrayRecycler.h" +#include "llvm/Support/MathExtras.h" #include "llvm/Support/TrailingObjects.h" #include #include @@ -121,22 +122,27 @@ private: // Operands are allocated by an ArrayRecycler. MachineOperand *Operands = nullptr; // Pointer to the first operand. - uint32_t Flags = 0; // Various bits of additional - // information about machine - // instruction. - uint16_t NumOperands = 0; // Number of operands on instruction. - uint8_t AsmPrinterFlags = 0; // Various bits of information used by - // the AsmPrinter to emit helpful - // comments. This is *not* semantic - // information. Do not use this for - // anything other than to convey comment - // information to AsmPrinter. - - // OperandCapacity has uint8_t size, so it should be next to AsmPrinterFlags + +#define LLVM_MI_NUMOPERANDS_BITS 24 +#define LLVM_MI_FLAGS_BITS 24 +#define LLVM_MI_ASMPRINTERFLAGS_BITS 8 + + /// Number of operands on instruction. + uint32_t NumOperands : LLVM_MI_NUMOPERANDS_BITS; + + // OperandCapacity has uint8_t size, so it should be next to NumOperands // to properly pack. using OperandCapacity = ArrayRecycler::Capacity; OperandCapacity CapOperands; // Capacity of the Operands array. + /// Various bits of additional information about the machine instruction. + uint32_t Flags : LLVM_MI_FLAGS_BITS; + + /// Various bits of information used by the AsmPrinter to emit helpful + /// comments. This is *not* semantic information. Do not use this for + /// anything other than to convey comment information to AsmPrinter. + uint8_t AsmPrinterFlags : LLVM_MI_ASMPRINTERFLAGS_BITS; + /// Internal implementation detail class that provides out-of-line storage for /// extra info used by the machine instruction when this info cannot be stored /// in-line within the instruction itself. @@ -342,16 +348,22 @@ public: /// Return whether an AsmPrinter flag is set. bool getAsmPrinterFlag(CommentFlag Flag) const { + assert(isUInt(unsigned(Flag)) && + "Flag is out of range for the AsmPrinterFlags field"); return AsmPrinterFlags & Flag; } /// Set a flag for the AsmPrinter. void setAsmPrinterFlag(uint8_t Flag) { + assert(isUInt(unsigned(Flag)) && + "Flag is out of range for the AsmPrinterFlags field"); AsmPrinterFlags |= Flag; } /// Clear specific AsmPrinter flags. void clearAsmPrinterFlag(CommentFlag Flag) { + assert(isUInt(unsigned(Flag)) && + "Flag is out of range for the AsmPrinterFlags field"); AsmPrinterFlags &= ~Flag; } @@ -362,15 +374,21 @@ public: /// Return whether an MI flag is set. bool getFlag(MIFlag Flag) const { + assert(isUInt(unsigned(Flag)) && + "Flag is out of range for the Flags field"); return Flags & Flag; } /// Set a MI flag. void setFlag(MIFlag Flag) { + assert(isUInt(unsigned(Flag)) && + "Flag is out of range for the Flags field"); Flags |= (uint32_t)Flag; } void setFlags(unsigned flags) { + assert(isUInt(flags) && + "flags to be set are out of range for the Flags field"); // Filter out the automatically maintained flags. unsigned Mask = BundledPred | BundledSucc; Flags = (Flags & Mask) | (flags & ~Mask); @@ -378,6 +396,8 @@ public: /// clearFlag - Clear a MI flag. void clearFlag(MIFlag Flag) { + assert(isUInt(unsigned(Flag)) && + "Flag to clear is out of range for the Flags field"); Flags &= ~((uint32_t)Flag); } diff --git a/llvm/lib/CodeGen/MachineInstr.cpp b/llvm/lib/CodeGen/MachineInstr.cpp index 400e76f..a930948 100644 --- a/llvm/lib/CodeGen/MachineInstr.cpp +++ b/llvm/lib/CodeGen/MachineInstr.cpp @@ -96,7 +96,8 @@ void MachineInstr::addImplicitDefUseOperands(MachineFunction &MF) { /// the MCInstrDesc. MachineInstr::MachineInstr(MachineFunction &MF, const MCInstrDesc &TID, DebugLoc DL, bool NoImp) - : MCID(&TID), DbgLoc(std::move(DL)), DebugInstrNum(0) { + : MCID(&TID), NumOperands(0), Flags(0), AsmPrinterFlags(0), + DbgLoc(std::move(DL)), DebugInstrNum(0) { assert(DbgLoc.hasTrivialDestructor() && "Expected trivial destructor"); // Reserve space for the expected number of operands. @@ -114,8 +115,8 @@ MachineInstr::MachineInstr(MachineFunction &MF, const MCInstrDesc &TID, /// Does not copy the number from debug instruction numbering, to preserve /// uniqueness. MachineInstr::MachineInstr(MachineFunction &MF, const MachineInstr &MI) - : MCID(&MI.getDesc()), Info(MI.Info), DbgLoc(MI.getDebugLoc()), - DebugInstrNum(0) { + : MCID(&MI.getDesc()), NumOperands(0), Flags(0), AsmPrinterFlags(0), + Info(MI.Info), DbgLoc(MI.getDebugLoc()), DebugInstrNum(0) { assert(DbgLoc.hasTrivialDestructor() && "Expected trivial destructor"); CapOperands = OperandCapacity::get(MI.getNumOperands()); @@ -192,7 +193,8 @@ static void moveOperands(MachineOperand *Dst, MachineOperand *Src, /// 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(isUInt(NumOperands + 1) && + "Cannot add more operands."); assert(MCID && "Cannot add operands before providing an instr descriptor"); // Check if we're adding one of our existing operands. -- 2.7.4