Skip to content

Commit 703c083

Browse files
committed
[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
1 parent d7d4aa5 commit 703c083

File tree

2 files changed

+38
-16
lines changed

2 files changed

+38
-16
lines changed

llvm/include/llvm/CodeGen/MachineInstr.h

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include "llvm/MC/MCInstrDesc.h"
3030
#include "llvm/MC/MCSymbol.h"
3131
#include "llvm/Support/ArrayRecycler.h"
32+
#include "llvm/Support/MathExtras.h"
3233
#include "llvm/Support/TrailingObjects.h"
3334
#include <algorithm>
3435
#include <cassert>
@@ -121,22 +122,27 @@ class MachineInstr
121122

122123
// Operands are allocated by an ArrayRecycler.
123124
MachineOperand *Operands = nullptr; // Pointer to the first operand.
124-
uint32_t Flags = 0; // Various bits of additional
125-
// information about machine
126-
// instruction.
127-
uint16_t NumOperands = 0; // Number of operands on instruction.
128-
uint8_t AsmPrinterFlags = 0; // Various bits of information used by
129-
// the AsmPrinter to emit helpful
130-
// comments. This is *not* semantic
131-
// information. Do not use this for
132-
// anything other than to convey comment
133-
// information to AsmPrinter.
134-
135-
// OperandCapacity has uint8_t size, so it should be next to AsmPrinterFlags
125+
126+
#define LLVM_MI_NUMOPERANDS_BITS 24
127+
#define LLVM_MI_FLAGS_BITS 24
128+
#define LLVM_MI_ASMPRINTERFLAGS_BITS 8
129+
130+
/// Number of operands on instruction.
131+
uint32_t NumOperands : LLVM_MI_NUMOPERANDS_BITS;
132+
133+
// OperandCapacity has uint8_t size, so it should be next to NumOperands
136134
// to properly pack.
137135
using OperandCapacity = ArrayRecycler<MachineOperand>::Capacity;
138136
OperandCapacity CapOperands; // Capacity of the Operands array.
139137

138+
/// Various bits of additional information about the machine instruction.
139+
uint32_t Flags : LLVM_MI_FLAGS_BITS;
140+
141+
/// Various bits of information used by the AsmPrinter to emit helpful
142+
/// comments. This is *not* semantic information. Do not use this for
143+
/// anything other than to convey comment information to AsmPrinter.
144+
uint8_t AsmPrinterFlags : LLVM_MI_ASMPRINTERFLAGS_BITS;
145+
140146
/// Internal implementation detail class that provides out-of-line storage for
141147
/// extra info used by the machine instruction when this info cannot be stored
142148
/// in-line within the instruction itself.
@@ -342,16 +348,22 @@ class MachineInstr
342348

343349
/// Return whether an AsmPrinter flag is set.
344350
bool getAsmPrinterFlag(CommentFlag Flag) const {
351+
assert(isUInt<LLVM_MI_ASMPRINTERFLAGS_BITS>(unsigned(Flag)) &&
352+
"Flag is out of range for the AsmPrinterFlags field");
345353
return AsmPrinterFlags & Flag;
346354
}
347355

348356
/// Set a flag for the AsmPrinter.
349357
void setAsmPrinterFlag(uint8_t Flag) {
358+
assert(isUInt<LLVM_MI_ASMPRINTERFLAGS_BITS>(unsigned(Flag)) &&
359+
"Flag is out of range for the AsmPrinterFlags field");
350360
AsmPrinterFlags |= Flag;
351361
}
352362

353363
/// Clear specific AsmPrinter flags.
354364
void clearAsmPrinterFlag(CommentFlag Flag) {
365+
assert(isUInt<LLVM_MI_ASMPRINTERFLAGS_BITS>(unsigned(Flag)) &&
366+
"Flag is out of range for the AsmPrinterFlags field");
355367
AsmPrinterFlags &= ~Flag;
356368
}
357369

@@ -362,22 +374,30 @@ class MachineInstr
362374

363375
/// Return whether an MI flag is set.
364376
bool getFlag(MIFlag Flag) const {
377+
assert(isUInt<LLVM_MI_FLAGS_BITS>(unsigned(Flag)) &&
378+
"Flag is out of range for the Flags field");
365379
return Flags & Flag;
366380
}
367381

368382
/// Set a MI flag.
369383
void setFlag(MIFlag Flag) {
384+
assert(isUInt<LLVM_MI_FLAGS_BITS>(unsigned(Flag)) &&
385+
"Flag is out of range for the Flags field");
370386
Flags |= (uint32_t)Flag;
371387
}
372388

373389
void setFlags(unsigned flags) {
390+
assert(isUInt<LLVM_MI_FLAGS_BITS>(flags) &&
391+
"flags to be set are out of range for the Flags field");
374392
// Filter out the automatically maintained flags.
375393
unsigned Mask = BundledPred | BundledSucc;
376394
Flags = (Flags & Mask) | (flags & ~Mask);
377395
}
378396

379397
/// clearFlag - Clear a MI flag.
380398
void clearFlag(MIFlag Flag) {
399+
assert(isUInt<LLVM_MI_FLAGS_BITS>(unsigned(Flag)) &&
400+
"Flag to clear is out of range for the Flags field");
381401
Flags &= ~((uint32_t)Flag);
382402
}
383403

llvm/lib/CodeGen/MachineInstr.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,8 @@ void MachineInstr::addImplicitDefUseOperands(MachineFunction &MF) {
9696
/// the MCInstrDesc.
9797
MachineInstr::MachineInstr(MachineFunction &MF, const MCInstrDesc &TID,
9898
DebugLoc DL, bool NoImp)
99-
: MCID(&TID), DbgLoc(std::move(DL)), DebugInstrNum(0) {
99+
: MCID(&TID), NumOperands(0), Flags(0), AsmPrinterFlags(0),
100+
DbgLoc(std::move(DL)), DebugInstrNum(0) {
100101
assert(DbgLoc.hasTrivialDestructor() && "Expected trivial destructor");
101102

102103
// Reserve space for the expected number of operands.
@@ -114,8 +115,8 @@ MachineInstr::MachineInstr(MachineFunction &MF, const MCInstrDesc &TID,
114115
/// Does not copy the number from debug instruction numbering, to preserve
115116
/// uniqueness.
116117
MachineInstr::MachineInstr(MachineFunction &MF, const MachineInstr &MI)
117-
: MCID(&MI.getDesc()), Info(MI.Info), DbgLoc(MI.getDebugLoc()),
118-
DebugInstrNum(0) {
118+
: MCID(&MI.getDesc()), NumOperands(0), Flags(0), AsmPrinterFlags(0),
119+
Info(MI.Info), DbgLoc(MI.getDebugLoc()), DebugInstrNum(0) {
119120
assert(DbgLoc.hasTrivialDestructor() && "Expected trivial destructor");
120121

121122
CapOperands = OperandCapacity::get(MI.getNumOperands());
@@ -192,7 +193,8 @@ static void moveOperands(MachineOperand *Dst, MachineOperand *Src,
192193
/// an explicit operand it is added at the end of the explicit operand list
193194
/// (before the first implicit operand).
194195
void MachineInstr::addOperand(MachineFunction &MF, const MachineOperand &Op) {
195-
assert(NumOperands < USHRT_MAX && "Cannot add more operands.");
196+
assert(isUInt<LLVM_MI_NUMOPERANDS_BITS>(NumOperands + 1) &&
197+
"Cannot add more operands.");
196198
assert(MCID && "Cannot add operands before providing an instr descriptor");
197199

198200
// Check if we're adding one of our existing operands.

0 commit comments

Comments
 (0)