Skip to content

[TableGen] Change SetTheory set/vec to use const Record * #107692

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 9, 2024
Merged

[TableGen] Change SetTheory set/vec to use const Record * #107692

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion clang/utils/TableGen/NeonEmitter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1636,7 +1636,7 @@ std::pair<Type, std::string> Intrinsic::DagEmitter::emitDagShuffle(DagInit *DI){
int64_t VectorSize = cast<IntInit>(Expr->getArg(0))->getValue();
VectorSize /= ElementSize;

std::vector<Record *> Revved;
std::vector<const Record *> Revved;
for (unsigned VI = 0; VI < Elts2.size(); VI += VectorSize) {
for (int LI = VectorSize - 1; LI >= 0; --LI) {
Revved.push_back(Elts2[VI + LI]);
Expand Down
4 changes: 2 additions & 2 deletions llvm/include/llvm/TableGen/Record.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1669,7 +1669,7 @@ class Record {
RecordKeeper &TrackedRecords;

// The DefInit corresponding to this record.
DefInit *CorrespondingDefInit = nullptr;
mutable DefInit *CorrespondingDefInit = nullptr;

// Unique record ID.
unsigned ID;
Expand Down Expand Up @@ -1736,7 +1736,7 @@ class Record {
RecordRecTy *getType() const;

/// get the corresponding DefInit.
DefInit *getDefInit();
DefInit *getDefInit() const;

bool isClass() const { return Kind == RK_Class; }

Expand Down
4 changes: 2 additions & 2 deletions llvm/include/llvm/TableGen/SetTheory.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ class Record;

class SetTheory {
public:
using RecVec = std::vector<Record *>;
using RecSet = SmallSetVector<Record *, 16>;
using RecVec = std::vector<const Record *>;
using RecSet = SmallSetVector<const Record *, 16>;

/// Operator - A callback representing a DAG operator.
class Operator {
Expand Down
6 changes: 3 additions & 3 deletions llvm/lib/TableGen/Record.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2802,10 +2802,10 @@ RecordRecTy *Record::getType() const {
return RecordRecTy::get(TrackedRecords, DirectSCs);
}

DefInit *Record::getDefInit() {
DefInit *Record::getDefInit() const {
if (!CorrespondingDefInit) {
CorrespondingDefInit =
new (TrackedRecords.getImpl().Allocator) DefInit(this);
CorrespondingDefInit = new (TrackedRecords.getImpl().Allocator)
DefInit(const_cast<Record *>(this));
Copy link
Contributor Author

@jurahul jurahul Sep 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only issue/smell is here, where CodeGenDAGPatterns.cpp and CodeGenRegisters.cpp call DefInit::get() on the result of a SetTheory::expand() returned record (which is now const *). I looked a little bit to see what they are doing, but decided to "cut off" the change here to keep it smaller. This is part of effort to have better const correctness w.r.t records and recordkeeper, so with this, the "smell" is atleast localized to this function and we can have const records otherwise.

Copy link
Contributor Author

@jurahul jurahul Sep 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did look into making the Def pointer in DefInit const. It's possible, but will spawn a bunch of other changes, so best done as a separate PR. I also checked if we can always instantiate the CorrespondingDefInit when a new Record is added to the RecordKeeper when parsing, it seems only a small fraction of Records have CorrespondingDefInit set to non-null (measured in RecordKeeper destructor) so unconditionally populating CorrespondingDefInit will likely cause memory footprint increase. So this seems like a reasonable thing, and will be followed by removing the const_cast<> (but CorrespondingDefInit will stay mutable, its like a cache).

}
return CorrespondingDefInit;
}
Expand Down
38 changes: 20 additions & 18 deletions llvm/utils/TableGen/AsmMatcherEmitter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ class AsmMatcherInfo;
// RegisterSets can be seen in the outputted AsmMatcher tables occasionally, and
// can even affect compiler output (at least seen in diagnostics produced when
// all matches fail). So we use a type that sorts them consistently.
typedef std::set<Record *, LessRecordByID> RegisterSet;
typedef std::set<const Record *, LessRecordByID> RegisterSet;

class AsmMatcherEmitter {
RecordKeeper &Records;
Expand Down Expand Up @@ -242,7 +242,7 @@ struct ClassInfo {
if (!isRegisterClass() || !RHS.isRegisterClass())
return false;

std::vector<Record *> Tmp;
std::vector<const Record *> Tmp;
std::set_intersection(Registers.begin(), Registers.end(),
RHS.Registers.begin(), RHS.Registers.end(),
std::back_inserter(Tmp), LessRecordByID());
Expand Down Expand Up @@ -403,7 +403,7 @@ struct MatchableInfo {
bool IsIsolatedToken;

/// Register record if this token is singleton register.
Record *SingletonReg;
const Record *SingletonReg;

explicit AsmOperand(bool IsIsolatedToken, StringRef T)
: Token(T), Class(nullptr), SubOpIdx(-1),
Expand Down Expand Up @@ -582,7 +582,7 @@ struct MatchableInfo {
void formTwoOperandAlias(StringRef Constraint);

void initialize(const AsmMatcherInfo &Info,
SmallPtrSetImpl<Record *> &SingletonRegisters,
SmallPtrSetImpl<const Record *> &SingletonRegisters,
AsmVariantInfo const &Variant, bool HasMnemonicFirst);

/// validate - Return true if this matchable is a valid thing to match against
Expand Down Expand Up @@ -757,7 +757,8 @@ class AsmMatcherInfo {
std::vector<OperandMatchEntry> OperandMatchInfo;

/// Map of Register records to their class information.
typedef std::map<Record *, ClassInfo *, LessRecordByID> RegisterClassesTy;
typedef std::map<const Record *, ClassInfo *, LessRecordByID>
RegisterClassesTy;
RegisterClassesTy RegisterClasses;

/// Map of Predicate records to their subtarget information.
Expand All @@ -784,7 +785,8 @@ class AsmMatcherInfo {

/// buildRegisterClasses - Build the ClassInfo* instances for register
/// classes.
void buildRegisterClasses(SmallPtrSetImpl<Record *> &SingletonRegisters);
void
buildRegisterClasses(SmallPtrSetImpl<const Record *> &SingletonRegisters);

/// buildOperandClasses - Build the ClassInfo* instances for user defined
/// operand classes.
Expand Down Expand Up @@ -935,10 +937,10 @@ static void extractSingletonRegisterForAsmOperand(MatchableInfo::AsmOperand &Op,
// be some random non-register token, just ignore it.
}

void MatchableInfo::initialize(const AsmMatcherInfo &Info,
SmallPtrSetImpl<Record *> &SingletonRegisters,
AsmVariantInfo const &Variant,
bool HasMnemonicFirst) {
void MatchableInfo::initialize(
const AsmMatcherInfo &Info,
SmallPtrSetImpl<const Record *> &SingletonRegisters,
AsmVariantInfo const &Variant, bool HasMnemonicFirst) {
AsmVariantID = Variant.AsmVariantNo;
AsmString = CodeGenInstruction::FlattenAsmStringVariants(
AsmString, Variant.AsmVariantNo);
Expand Down Expand Up @@ -972,8 +974,8 @@ void MatchableInfo::initialize(const AsmMatcherInfo &Info,
// Collect singleton registers, if used.
for (MatchableInfo::AsmOperand &Op : AsmOperands) {
extractSingletonRegisterForAsmOperand(Op, Info, Variant.RegisterPrefix);
if (Record *Reg = Op.SingletonReg)
SingletonRegisters.insert(Reg);
if (Op.SingletonReg)
SingletonRegisters.insert(Op.SingletonReg);
}

const RecordVal *DepMask = TheDef->getValue("DeprecatedFeatureMask");
Expand Down Expand Up @@ -1253,7 +1255,7 @@ struct LessRegisterSet {
};

void AsmMatcherInfo::buildRegisterClasses(
SmallPtrSetImpl<Record *> &SingletonRegisters) {
SmallPtrSetImpl<const Record *> &SingletonRegisters) {
const auto &Registers = Target.getRegBank().getRegisters();
auto &RegClassList = Target.getRegBank().getRegClasses();

Expand All @@ -1268,14 +1270,14 @@ void AsmMatcherInfo::buildRegisterClasses(
RegisterSet(RC.getOrder().begin(), RC.getOrder().end()));

// Add any required singleton sets.
for (Record *Rec : SingletonRegisters) {
for (const Record *Rec : SingletonRegisters) {
RegisterSets.insert(RegisterSet(&Rec, &Rec + 1));
}

// Introduce derived sets where necessary (when a register does not determine
// a unique register set class), and build the mapping of registers to the set
// they should classify to.
std::map<Record *, RegisterSet> RegisterMap;
std::map<const Record *, RegisterSet> RegisterMap;
for (const CodeGenRegister &CGR : Registers) {
// Compute the intersection of all sets containing this register.
RegisterSet ContainingSet;
Expand Down Expand Up @@ -1369,7 +1371,7 @@ void AsmMatcherInfo::buildRegisterClasses(
RegisterClasses[It.first] = RegisterSetClasses[It.second];

// Name the register classes which correspond to singleton registers.
for (Record *Rec : SingletonRegisters) {
for (const Record *Rec : SingletonRegisters) {
ClassInfo *CI = RegisterClasses[Rec];
assert(CI && "Missing singleton register class info!");

Expand Down Expand Up @@ -1526,7 +1528,7 @@ void AsmMatcherInfo::buildInfo() {

// Parse the instructions; we need to do this first so that we can gather the
// singleton register classes.
SmallPtrSet<Record *, 16> SingletonRegisters;
SmallPtrSet<const Record *, 16> SingletonRegisters;
unsigned VariantCount = Target.getAsmParserVariantCount();
for (unsigned VC = 0; VC != VariantCount; ++VC) {
Record *AsmVariant = Target.getAsmParserVariant(VC);
Expand Down Expand Up @@ -1617,7 +1619,7 @@ void AsmMatcherInfo::buildInfo() {
StringRef Token = Op.Token;

// Check for singleton registers.
if (Record *RegRecord = Op.SingletonReg) {
if (const Record *RegRecord = Op.SingletonReg) {
Op.Class = RegisterClasses[RegRecord];
assert(Op.Class && Op.Class->Registers.size() == 1 &&
"Unexpected class for singleton register");
Expand Down
29 changes: 13 additions & 16 deletions llvm/utils/TableGen/Common/CodeGenRegisters.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ void CodeGenSubRegIndex::computeConcatTransitiveClosure() {
// CodeGenRegister
//===----------------------------------------------------------------------===//

CodeGenRegister::CodeGenRegister(Record *R, unsigned Enum)
CodeGenRegister::CodeGenRegister(const Record *R, unsigned Enum)
: TheDef(R), EnumValue(Enum),
CostPerUse(R->getValueAsListOfInts("CostPerUse")),
CoveredBySubRegs(R->getValueAsBit("CoveredBySubRegs")),
Expand Down Expand Up @@ -657,10 +657,10 @@ struct TupleExpander : SetTheory::Expander {
RecordKeeper &RK = Def->getRecords();
for (unsigned n = 0; n != Length; ++n) {
std::string Name;
Record *Proto = Lists[0][n];
const Record *Proto = Lists[0][n];
std::vector<Init *> Tuple;
for (unsigned i = 0; i != Dim; ++i) {
Record *Reg = Lists[i][n];
const Record *Reg = Lists[i][n];
if (i)
Name += '_';
Name += Reg->getName();
Expand Down Expand Up @@ -1198,18 +1198,17 @@ CodeGenRegBank::CodeGenRegBank(RecordKeeper &Records,
Idx.updateComponents(*this);

// Read in the register and register tuple definitions.
std::vector<Record *> Regs = Records.getAllDerivedDefinitions("Register");
const RecordKeeper &RC = Records;
std::vector<const Record *> Regs = RC.getAllDerivedDefinitions("Register");
if (!Regs.empty() && Regs[0]->isSubClassOf("X86Reg")) {
// For X86, we need to sort Registers and RegisterTuples together to list
// new registers and register tuples at a later position. So that we can
// reduce unnecessary iterations on unsupported registers in LiveVariables.
// TODO: Remove this logic when migrate from LiveVariables to LiveIntervals
// completely.
std::vector<Record *> Tups =
Records.getAllDerivedDefinitions("RegisterTuples");
for (Record *R : Tups) {
for (const Record *R : Records.getAllDerivedDefinitions("RegisterTuples")) {
// Expand tuples and merge the vectors
std::vector<Record *> TupRegs = *Sets.expand(R);
std::vector<const Record *> TupRegs = *Sets.expand(R);
Regs.insert(Regs.end(), TupRegs.begin(), TupRegs.end());
}

Expand All @@ -1224,13 +1223,10 @@ CodeGenRegBank::CodeGenRegBank(RecordKeeper &Records,
getReg(Regs[i]);

// Expand tuples and number the new registers.
std::vector<Record *> Tups =
Records.getAllDerivedDefinitions("RegisterTuples");

for (Record *R : Tups) {
std::vector<Record *> TupRegs = *Sets.expand(R);
for (Record *R : Records.getAllDerivedDefinitions("RegisterTuples")) {
std::vector<const Record *> TupRegs = *Sets.expand(R);
llvm::sort(TupRegs, LessRecordRegister());
for (Record *RC : TupRegs)
for (const Record *RC : TupRegs)
getReg(RC);
}
}
Expand Down Expand Up @@ -1342,7 +1338,7 @@ CodeGenRegBank::findSubRegIdx(const Record *Def) const {
return Def2SubRegIdx.lookup(Def);
}

CodeGenRegister *CodeGenRegBank::getReg(Record *Def) {
CodeGenRegister *CodeGenRegBank::getReg(const Record *Def) {
CodeGenRegister *&Reg = Def2Reg[Def];
if (Reg)
return Reg;
Expand Down Expand Up @@ -2508,7 +2504,8 @@ CodeGenRegBank::getMinimalPhysRegClass(Record *RegRecord,
return BestRC;
}

BitVector CodeGenRegBank::computeCoveredRegisters(ArrayRef<Record *> Regs) {
BitVector
CodeGenRegBank::computeCoveredRegisters(ArrayRef<const Record *> Regs) {
SetVector<const CodeGenRegister *> Set;

// First add Regs with all sub-registers.
Expand Down
16 changes: 9 additions & 7 deletions llvm/utils/TableGen/Common/CodeGenRegisters.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ class CodeGenSubRegIndex {
/// CodeGenRegister - Represents a register definition.
class CodeGenRegister {
public:
Record *TheDef;
const Record *TheDef;
unsigned EnumValue;
std::vector<int64_t> CostPerUse;
bool CoveredBySubRegs = true;
Expand All @@ -184,7 +184,7 @@ class CodeGenRegister {
typedef std::map<CodeGenSubRegIndex *, CodeGenRegister *, deref<std::less<>>>
SubRegMap;

CodeGenRegister(Record *R, unsigned Enum);
CodeGenRegister(const Record *R, unsigned Enum);

StringRef getName() const;

Expand Down Expand Up @@ -314,7 +314,7 @@ inline bool operator==(const CodeGenRegister &A, const CodeGenRegister &B) {
class CodeGenRegisterClass {
CodeGenRegister::Vec Members;
// Allocation orders. Order[0] always contains all registers in Members.
std::vector<SmallVector<Record *, 16>> Orders;
std::vector<SmallVector<const Record *, 16>> Orders;
// Bit mask of sub-classes including this, indexed by their EnumValue.
BitVector SubClasses;
// List of super-classes, topologocally ordered to have the larger classes
Expand Down Expand Up @@ -452,7 +452,9 @@ class CodeGenRegisterClass {
// Returns an ordered list of class members.
// The order of registers is the same as in the .td file.
// No = 0 is the default allocation order, No = 1 is the first alternative.
ArrayRef<Record *> getOrder(unsigned No = 0) const { return Orders[No]; }
ArrayRef<const Record *> getOrder(unsigned No = 0) const {
return Orders[No];
}

// Return the total number of allocation orders available.
unsigned getNumOrders() const { return Orders.size(); }
Expand Down Expand Up @@ -594,7 +596,7 @@ class CodeGenRegBank {
// Registers.
std::deque<CodeGenRegister> Registers;
StringMap<CodeGenRegister *> RegistersByName;
DenseMap<Record *, CodeGenRegister *> Def2Reg;
DenseMap<const Record *, CodeGenRegister *> Def2Reg;
unsigned NumNativeRegUnits;

std::map<TopoSigId, unsigned> TopoSigs;
Expand Down Expand Up @@ -713,7 +715,7 @@ class CodeGenRegBank {
}

// Find a register from its Record def.
CodeGenRegister *getReg(Record *);
CodeGenRegister *getReg(const Record *);

// Get a Register's index into the Registers array.
unsigned getRegIndex(const CodeGenRegister *Reg) const {
Expand Down Expand Up @@ -846,7 +848,7 @@ class CodeGenRegBank {
//
// This is used to compute the mask of call-preserved registers from a list
// of callee-saves.
BitVector computeCoveredRegisters(ArrayRef<Record *> Regs);
BitVector computeCoveredRegisters(ArrayRef<const Record *> Regs);

// Bit mask of lanes that cover their registers. A sub-register index whose
// LaneMask is contained in CoveringLanes will be completely covered by
Expand Down
Loading