[llvm] 703c083 - [MachineInst] Bump NumOperands back up to 24bits
Jon Roelofs via llvm-commits
llvm-commits at lists.llvm.org
Wed Jun 28 10:32:49 PDT 2023
Author: Jon Roelofs
Date: 2023-06-28T10:32:40-07:00
New Revision: 703c08362adcc7990fa5ddc444c41c5efdccbc2b
URL: https://github.com/llvm/llvm-project/commit/703c08362adcc7990fa5ddc444c41c5efdccbc2b
DIFF: https://github.com/llvm/llvm-project/commit/703c08362adcc7990fa5ddc444c41c5efdccbc2b.diff
LOG: [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
Added:
Modified:
llvm/include/llvm/CodeGen/MachineInstr.h
llvm/lib/CodeGen/MachineInstr.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/CodeGen/MachineInstr.h b/llvm/include/llvm/CodeGen/MachineInstr.h
index fa287becb60fe..2928ccfbcef72 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 <algorithm>
#include <cassert>
@@ -121,22 +122,27 @@ class MachineInstr
// 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<MachineOperand>::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 @@ class MachineInstr
/// Return whether an AsmPrinter flag is set.
bool getAsmPrinterFlag(CommentFlag Flag) const {
+ assert(isUInt<LLVM_MI_ASMPRINTERFLAGS_BITS>(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<LLVM_MI_ASMPRINTERFLAGS_BITS>(unsigned(Flag)) &&
+ "Flag is out of range for the AsmPrinterFlags field");
AsmPrinterFlags |= Flag;
}
/// Clear specific AsmPrinter flags.
void clearAsmPrinterFlag(CommentFlag Flag) {
+ assert(isUInt<LLVM_MI_ASMPRINTERFLAGS_BITS>(unsigned(Flag)) &&
+ "Flag is out of range for the AsmPrinterFlags field");
AsmPrinterFlags &= ~Flag;
}
@@ -362,15 +374,21 @@ class MachineInstr
/// Return whether an MI flag is set.
bool getFlag(MIFlag Flag) const {
+ assert(isUInt<LLVM_MI_FLAGS_BITS>(unsigned(Flag)) &&
+ "Flag is out of range for the Flags field");
return Flags & Flag;
}
/// Set a MI flag.
void setFlag(MIFlag Flag) {
+ assert(isUInt<LLVM_MI_FLAGS_BITS>(unsigned(Flag)) &&
+ "Flag is out of range for the Flags field");
Flags |= (uint32_t)Flag;
}
void setFlags(unsigned flags) {
+ assert(isUInt<LLVM_MI_FLAGS_BITS>(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 @@ class MachineInstr
/// clearFlag - Clear a MI flag.
void clearFlag(MIFlag Flag) {
+ assert(isUInt<LLVM_MI_FLAGS_BITS>(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 400e76f0e434d..a9309487a7a7f 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<LLVM_MI_NUMOPERANDS_BITS>(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.
More information about the llvm-commits
mailing list