[llvm] [NFC][TableGen] Code cleanup in Record.h/cpp (PR #138876)
via llvm-commits
llvm-commits at lists.llvm.org
Thu May 8 08:27:00 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-tablegen
Author: Rahul Joshi (jurahul)
<details>
<summary>Changes</summary>
- Use range for loops in several places.
- Change some variable names to conform to LLVM coding standard.
- Use ListSeparator instead of manual code to generate comma interleaved strings.
- Remove unnecessary copies in SETDAGOP evaluation in BinOpInit::Fold.
- Eliminate duplicated code in the 2 overloads of RecordVal::setValue.
- Use explicit type in some range for loops.
---
Patch is 24.60 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/138876.diff
2 Files Affected:
- (modified) llvm/include/llvm/TableGen/Record.h (+17-25)
- (modified) llvm/lib/TableGen/Record.cpp (+117-179)
``````````diff
diff --git a/llvm/include/llvm/TableGen/Record.h b/llvm/include/llvm/TableGen/Record.h
index 982cc255553a2..d7d744aeec956 100644
--- a/llvm/include/llvm/TableGen/Record.h
+++ b/llvm/include/llvm/TableGen/Record.h
@@ -19,6 +19,7 @@
#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/FoldingSet.h"
#include "llvm/ADT/PointerIntPair.h"
+#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
@@ -244,7 +245,7 @@ class RecordRecTy final : public RecTy,
RecordRecTy &operator=(const RecordRecTy &) = delete;
// Do not use sized deallocation due to trailing objects.
- void operator delete(void *p) { ::operator delete(p); }
+ void operator delete(void *Ptr) { ::operator delete(Ptr); }
static bool classof(const RecTy *RT) {
return RT->getRecTyKind() == RecordRecTyKind;
@@ -598,7 +599,7 @@ class BitsInit final : public TypedInit,
BitsInit &operator=(const BitsInit &) = delete;
// Do not use sized deallocation due to trailing objects.
- void operator delete(void *p) { ::operator delete(p); }
+ void operator delete(void *Ptr) { ::operator delete(Ptr); }
static bool classof(const Init *I) {
return I->getKind() == IK_BitsInit;
@@ -615,18 +616,8 @@ class BitsInit final : public TypedInit,
convertInitializerBitRange(ArrayRef<unsigned> Bits) const override;
std::optional<int64_t> convertInitializerToInt() const;
- bool isComplete() const override {
- for (unsigned i = 0; i != getNumBits(); ++i)
- if (!getBit(i)->isComplete()) return false;
- return true;
- }
-
- bool allInComplete() const {
- for (unsigned i = 0; i != getNumBits(); ++i)
- if (getBit(i)->isComplete()) return false;
- return true;
- }
-
+ bool isComplete() const override;
+ bool allInComplete() const;
bool isConcrete() const override;
std::string getAsString() const override;
@@ -773,7 +764,7 @@ class ListInit final : public TypedInit,
ListInit &operator=(const ListInit &) = delete;
// Do not use sized deallocation due to trailing objects.
- void operator delete(void *p) { ::operator delete(p); }
+ void operator delete(void *Ptr) { ::operator delete(Ptr); }
static bool classof(const Init *I) {
return I->getKind() == IK_ListInit;
@@ -786,13 +777,13 @@ class ListInit final : public TypedInit,
return ArrayRef(getTrailingObjects<const Init *>(), NumValues);
}
- const Init *getElement(unsigned Index) const { return getValues()[Index]; }
+ const Init *getElement(unsigned Idx) const { return getValues()[Idx]; }
const RecTy *getElementType() const {
return cast<ListRecTy>(getType())->getElementType();
}
- const Record *getElementAsRecord(unsigned i) const;
+ const Record *getElementAsRecord(unsigned Idx) const;
const Init *convertInitializerTo(const RecTy *Ty) const override;
@@ -1060,6 +1051,8 @@ class CondOpInit final : public TypedInit,
return ArrayRef(getTrailingObjects<const Init *>() + NumConds, NumConds);
}
+ auto getCondAndVals() const { return zip_equal(getConds(), getVals()); }
+
const Init *Fold(const Record *CurRec) const;
const Init *resolveReferences(Resolver &R) const override;
@@ -1349,7 +1342,7 @@ class VarDefInit final
VarDefInit &operator=(const VarDefInit &) = delete;
// Do not use sized deallocation due to trailing objects.
- void operator delete(void *p) { ::operator delete(p); }
+ void operator delete(void *Ptr) { ::operator delete(Ptr); }
static bool classof(const Init *I) {
return I->getKind() == IK_VarDefInit;
@@ -1495,6 +1488,8 @@ class DagInit final
return ArrayRef(getTrailingObjects<const StringInit *>(), NumArgs);
}
+ auto getArgAndNames() const { return zip_equal(getArgs(), getArgNames()); }
+
const Init *resolveReferences(Resolver &R) const override;
bool isConcrete() const override;
@@ -1798,9 +1793,9 @@ class Record {
}
void removeValue(const Init *Name) {
- for (unsigned i = 0, e = Values.size(); i != e; ++i)
- if (Values[i].getNameInit() == Name) {
- Values.erase(Values.begin()+i);
+ for (auto [Idx, Value] : enumerate(Values))
+ if (Value.getNameInit() == Name) {
+ Values.erase(Values.begin() + Idx);
return;
}
llvm_unreachable("Cannot remove an entry that does not exist!");
@@ -2123,10 +2118,7 @@ struct LessRecordRegister {
size_t size() { return Parts.size(); }
- std::pair<bool, StringRef> getPart(size_t i) {
- assert (i < Parts.size() && "Invalid idx!");
- return Parts[i];
- }
+ std::pair<bool, StringRef> getPart(size_t Idx) { return Parts[Idx]; }
};
bool operator()(const Record *Rec1, const Record *Rec2) const {
diff --git a/llvm/lib/TableGen/Record.cpp b/llvm/lib/TableGen/Record.cpp
index f3d54e6083e48..3df6a32b6b9fd 100644
--- a/llvm/lib/TableGen/Record.cpp
+++ b/llvm/lib/TableGen/Record.cpp
@@ -294,11 +294,9 @@ std::string RecordRecTy::getAsString() const {
return getClasses()[0]->getNameInitAsString();
std::string Str = "{";
- bool First = true;
+ ListSeparator LS;
for (const Record *R : getClasses()) {
- if (!First)
- Str += ", ";
- First = false;
+ Str += LS;
Str += R->getNameInitAsString();
}
Str += "}";
@@ -520,9 +518,9 @@ const Init *BitsInit::convertInitializerTo(const RecTy *Ty) const {
std::optional<int64_t> BitsInit::convertInitializerToInt() const {
int64_t Result = 0;
- for (unsigned i = 0, e = getNumBits(); i != e; ++i)
- if (auto *Bit = dyn_cast<BitInit>(getBit(i)))
- Result |= static_cast<int64_t>(Bit->getValue()) << i;
+ for (auto [Idx, InitV] : enumerate(getBits()))
+ if (auto *Bit = dyn_cast<BitInit>(InitV))
+ Result |= static_cast<int64_t>(Bit->getValue()) << Idx;
else
return std::nullopt;
return Result;
@@ -532,27 +530,30 @@ const Init *
BitsInit::convertInitializerBitRange(ArrayRef<unsigned> Bits) const {
SmallVector<const Init *, 16> NewBits(Bits.size());
- for (unsigned i = 0, e = Bits.size(); i != e; ++i) {
- if (Bits[i] >= getNumBits())
+ for (auto [Bit, NewBit] : zip_equal(Bits, NewBits)) {
+ if (Bit >= getNumBits())
return nullptr;
- NewBits[i] = getBit(Bits[i]);
+ NewBit = getBit(Bit);
}
return BitsInit::get(getRecordKeeper(), NewBits);
}
+bool BitsInit::isComplete() const {
+ return all_of(getBits(), [](const Init *Bit) { return Bit->isComplete(); });
+}
+bool BitsInit::allInComplete() const {
+ return all_of(getBits(), [](const Init *Bit) { return !Bit->isComplete(); });
+}
bool BitsInit::isConcrete() const {
- for (unsigned i = 0, e = getNumBits(); i != e; ++i) {
- if (!getBit(i)->isConcrete())
- return false;
- }
- return true;
+ return all_of(getBits(), [](const Init *Bit) { return Bit->isConcrete(); });
}
std::string BitsInit::getAsString() const {
std::string Result = "{ ";
- for (unsigned i = 0, e = getNumBits(); i != e; ++i) {
- if (i) Result += ", ";
- if (const Init *Bit = getBit(e - i - 1))
+ ListSeparator LS;
+ for (const Init *Bit : reverse(getBits())) {
+ Result += LS;
+ if (Bit)
Result += Bit->getAsString();
else
Result += "*";
@@ -569,9 +570,8 @@ const Init *BitsInit::resolveReferences(Resolver &R) const {
const Init *CachedBitVarRef = nullptr;
const Init *CachedBitVarResolved = nullptr;
- for (unsigned i = 0, e = getNumBits(); i != e; ++i) {
- const Init *CurBit = getBit(i);
- const Init *NewBit = CurBit;
+ for (auto [CurBit, NewBit] : zip_equal(getBits(), NewBits)) {
+ NewBit = CurBit;
if (const auto *CurBitVar = dyn_cast<VarBitInit>(CurBit)) {
if (CurBitVar->getBitVar() != CachedBitVarRef) {
@@ -587,7 +587,6 @@ const Init *BitsInit::resolveReferences(Resolver &R) const {
if (isa<UnsetInit>(NewBit) && R.keepUnsetBits())
NewBit = CurBit;
- NewBits[i] = NewBit;
Changed |= CurBit != NewBit;
}
@@ -644,12 +643,11 @@ const Init *IntInit::convertInitializerTo(const RecTy *Ty) const {
const Init *IntInit::convertInitializerBitRange(ArrayRef<unsigned> Bits) const {
SmallVector<const Init *, 16> NewBits(Bits.size());
- for (unsigned i = 0, e = Bits.size(); i != e; ++i) {
- if (Bits[i] >= 64)
+ for (auto [Bit, NewBit] : zip_equal(Bits, NewBits)) {
+ if (Bit >= 64)
return nullptr;
- NewBits[i] =
- BitInit::get(getRecordKeeper(), Value & (INT64_C(1) << Bits[i]));
+ NewBit = BitInit::get(getRecordKeeper(), Value & (INT64_C(1) << Bit));
}
return BitsInit::get(getRecordKeeper(), NewBits);
}
@@ -751,8 +749,9 @@ const Init *ListInit::convertInitializerTo(const RecTy *Ty) const {
Elements.push_back(CI);
if (CI != I)
Changed = true;
- } else
+ } else {
return nullptr;
+ }
if (!Changed)
return this;
@@ -762,9 +761,8 @@ const Init *ListInit::convertInitializerTo(const RecTy *Ty) const {
return nullptr;
}
-const Record *ListInit::getElementAsRecord(unsigned i) const {
- assert(i < NumValues && "List element index out of range!");
- const auto *DI = dyn_cast<DefInit>(getElement(i));
+const Record *ListInit::getElementAsRecord(unsigned Idx) const {
+ const auto *DI = dyn_cast<DefInit>(getElement(Idx));
if (!DI)
PrintFatalError("Expected record in list!");
return DI->getDef();
@@ -787,27 +785,20 @@ const Init *ListInit::resolveReferences(Resolver &R) const {
}
bool ListInit::isComplete() const {
- for (const Init *Element : *this) {
- if (!Element->isComplete())
- return false;
- }
- return true;
+ return all_of(*this,
+ [](const Init *Element) { return Element->isComplete(); });
}
bool ListInit::isConcrete() const {
- for (const Init *Element : *this) {
- if (!Element->isConcrete())
- return false;
- }
- return true;
+ return all_of(*this,
+ [](const Init *Element) { return Element->isConcrete(); });
}
std::string ListInit::getAsString() const {
std::string Result = "[";
- const char *sep = "";
+ ListSeparator LS;
for (const Init *Element : *this) {
- Result += sep;
- sep = ", ";
+ Result += LS;
Result += Element->getAsString();
}
return Result + "]";
@@ -1125,9 +1116,9 @@ static const StringInit *interleaveStringList(const ListInit *List,
SmallString<80> Result(Element->getValue());
StringInit::StringFormat Fmt = StringInit::SF_String;
- for (unsigned I = 1, E = List->size(); I < E; ++I) {
+ for (const Init *Elem : List->getValues().drop_front()) {
Result.append(Delim->getValue());
- const auto *Element = dyn_cast<StringInit>(List->getElement(I));
+ const auto *Element = dyn_cast<StringInit>(Elem);
if (!Element)
return nullptr;
Result.append(Element->getValue());
@@ -1147,10 +1138,10 @@ static const StringInit *interleaveIntList(const ListInit *List,
return nullptr;
SmallString<80> Result(Element->getAsString());
- for (unsigned I = 1, E = List->size(); I < E; ++I) {
+ for (const Init *Elem : List->getValues().drop_front()) {
Result.append(Delim->getValue());
const auto *Element = dyn_cast_or_null<IntInit>(
- List->getElement(I)->convertInitializerTo(IntRecTy::get(RK)));
+ Elem->convertInitializerTo(IntRecTy::get(RK)));
if (!Element)
return nullptr;
Result.append(Element->getAsString());
@@ -1317,13 +1308,11 @@ const Init *BinOpInit::Fold(const Record *CurRec) const {
SmallVector<const Init *, 8> Args;
SmallVector<const StringInit *, 8> ArgNames;
- for (unsigned i = 0, e = LHSs->getNumArgs(); i != e; ++i) {
- Args.push_back(LHSs->getArg(i));
- ArgNames.push_back(LHSs->getArgName(i));
- }
- for (unsigned i = 0, e = RHSs->getNumArgs(); i != e; ++i) {
- Args.push_back(RHSs->getArg(i));
- ArgNames.push_back(RHSs->getArgName(i));
+ for (const DagInit *Dag : {LHSs, RHSs}) {
+ for (auto [Arg, ArgName] : Dag->getArgAndNames()) {
+ Args.push_back(Arg);
+ ArgNames.push_back(ArgName);
+ }
}
return DagInit::get(Op, nullptr, Args, ArgNames);
}
@@ -1422,8 +1411,8 @@ const Init *BinOpInit::Fold(const Record *CurRec) const {
if (!LHSi || !RHSi)
break;
- auto Start = LHSi->getValue();
- auto End = RHSi->getValue();
+ int64_t Start = LHSi->getValue();
+ int64_t End = RHSi->getValue();
SmallVector<const Init *, 8> Args;
if (getOpcode() == RANGEC) {
// Closed interval
@@ -1519,15 +1508,8 @@ const Init *BinOpInit::Fold(const Record *CurRec) const {
case SETDAGOP: {
const auto *Dag = dyn_cast<DagInit>(LHS);
const auto *Op = dyn_cast<DefInit>(RHS);
- if (Dag && Op) {
- SmallVector<const Init *, 8> Args;
- SmallVector<const StringInit *, 8> ArgNames;
- for (unsigned i = 0, e = Dag->getNumArgs(); i != e; ++i) {
- Args.push_back(Dag->getArg(i));
- ArgNames.push_back(Dag->getArgName(i));
- }
- return DagInit::get(Op, nullptr, Args, ArgNames);
- }
+ if (Dag && Op)
+ return DagInit::get(Op, nullptr, Dag->getArgs(), Dag->getArgNames());
break;
}
case ADD:
@@ -1696,10 +1678,8 @@ static const Init *ForeachDagApply(const Init *LHS, const DagInit *MHSd,
Change = true;
SmallVector<std::pair<const Init *, const StringInit *>, 8> NewArgs;
- for (unsigned int i = 0; i < MHSd->getNumArgs(); ++i) {
- const Init *Arg = MHSd->getArg(i);
+ for (auto [Arg, ArgName] : MHSd->getArgAndNames()) {
const Init *NewArg;
- const StringInit *ArgName = MHSd->getArgName(i);
if (const auto *Argd = dyn_cast<DagInit>(Arg))
NewArg = ForeachDagApply(LHS, Argd, RHS, CurRec);
@@ -1937,9 +1917,9 @@ const Init *TernOpInit::Fold(const Record *CurRec) const {
assert(*ArgNo < Dag->getNumArgs());
SmallVector<const Init *, 8> Args(Dag->getArgs());
- SmallVector<const StringInit *, 8> Names(Dag->getArgNames());
Args[*ArgNo] = RHS;
- return DagInit::get(Dag->getOperator(), Dag->getName(), Args, Names);
+ return DagInit::get(Dag->getOperator(), Dag->getName(), Args,
+ Dag->getArgNames());
}
break;
}
@@ -1954,10 +1934,10 @@ const Init *TernOpInit::Fold(const Record *CurRec) const {
assert(*ArgNo < Dag->getNumArgs());
- SmallVector<const Init *, 8> Args(Dag->getArgs());
SmallVector<const StringInit *, 8> Names(Dag->getArgNames());
Names[*ArgNo] = dyn_cast<StringInit>(RHS);
- return DagInit::get(Dag->getOperator(), Dag->getName(), Args, Names);
+ return DagInit::get(Dag->getOperator(), Dag->getName(), Dag->getArgs(),
+ Names);
}
break;
}
@@ -2546,10 +2526,9 @@ const Init *VarDefInit::Fold() const {
std::string VarDefInit::getAsString() const {
std::string Result = Class->getNameInitAsString() + "<";
- const char *sep = "";
+ ListSeparator LS;
for (const Init *Arg : args()) {
- Result += sep;
- sep = ", ";
+ Result += LS;
Result += Arg->getAsString();
}
return Result + ">";
@@ -2648,15 +2627,14 @@ const CondOpInit *CondOpInit::get(ArrayRef<const Init *> Conds,
const Init *CondOpInit::resolveReferences(Resolver &R) const {
SmallVector<const Init *, 4> NewConds;
+ SmallVector<const Init *, 4> NewVals;
+
bool Changed = false;
- for (const Init *Case : getConds()) {
- const Init *NewCase = Case->resolveReferences(R);
- NewConds.push_back(NewCase);
- Changed |= NewCase != Case;
- }
+ for (auto [Cond, Val] : getCondAndVals()) {
+ const Init *NewCond = Cond->resolveReferences(R);
+ NewConds.push_back(NewCond);
+ Changed |= NewCond != Cond;
- SmallVector<const Init *, 4> NewVals;
- for (const Init *Val : getVals()) {
const Init *NewVal = Val->resolveReferences(R);
NewVals.push_back(NewVal);
Changed |= NewVal != Val;
@@ -2671,10 +2649,7 @@ const Init *CondOpInit::resolveReferences(Resolver &R) const {
const Init *CondOpInit::Fold(const Record *CurRec) const {
RecordKeeper &RK = getRecordKeeper();
- for (unsigned i = 0; i < NumConds; ++i) {
- const Init *Cond = getCond(i);
- const Init *Val = getVal(i);
-
+ for (auto [Cond, Val] : getCondAndVals()) {
if (const auto *CondI = dyn_cast_or_null<IntInit>(
Cond->convertInitializerTo(IntRecTy::get(RK)))) {
if (CondI->getValue())
@@ -2692,36 +2667,24 @@ const Init *CondOpInit::Fold(const Record *CurRec) const {
}
bool CondOpInit::isConcrete() const {
- for (const Init *Case : getConds())
- if (!Case->isConcrete())
- return false;
-
- for (const Init *Val : getVals())
- if (!Val->isConcrete())
- return false;
-
- return true;
+ return all_of(getCondAndVals(), [](const auto &Pair) {
+ return std::get<0>(Pair)->isConcrete() && std::get<1>(Pair)->isConcrete();
+ });
}
bool CondOpInit::isComplete() const {
- for (const Init *Case : getConds())
- if (!Case->isComplete())
- return false;
-
- for (const Init *Val : getVals())
- if (!Val->isConcrete())
- return false;
-
- return true;
+ return all_of(getCondAndVals(), [](const auto &Pair) {
+ return std::get<0>(Pair)->isComplete() && std::get<1>(Pair)->isComplete();
+ });
}
std::string CondOpInit::getAsString() const {
std::string Result = "!cond(";
- for (unsigned i = 0; i < getNumConds(); i++) {
- Result += getCond(i)->getAsString() + ": ";
- Result += getVal(i)->getAsString();
- if (i != getNumConds()-1)
- Result += ", ";
+ ListSeparator LS;
+ for (auto [Cond, Val] : getCondAndVals()) {
+ Result += LS;
+ Result += Cond->getAsString() + ": ";
+ Result += Val->getAsString();
}
return Result + ")";
}
@@ -2731,20 +2694,15 @@ const Init *CondOpInit::getBit(unsigned Bit) const {
}
static void ProfileDagInit(FoldingSetNodeID &ID, const Init *V,
- const StringInit *VN,
- ArrayRef<const Init *> ArgRange,
- ArrayRef<const StringInit *> NameRange) {
+ const StringInit *VN, ArrayRef<const Init *> Args,
+ ArrayRef<const StringInit *> ArgNames) {
ID.AddPointer(V);
ID.AddPointer(VN);
- ArrayRef<const Init *>::iterator Arg = ArgRange.begin();
- ArrayRef<const StringInit *>::iterator Name = NameRange.begin();
- while (Arg != ArgRange.end()) {
- assert(Name != NameRange.end() && "Arg name underflow!");
- ID.AddPointer(*Arg++);
- ID.AddPointer(*Name++);
+ for (auto [Arg, Name] : zip_equal(Args, ArgNames)) {
+ ID.AddPointer(Arg);
+ ID.AddPointer(Name);
}
- assert(Name == NameRange.end() && "Arg name overflow!");
}
DagInit::DagInit(const Init *V, const StringInit *VN,
@@ -2805,10 +2763,9 @@ const Record *DagInit::getOperatorAsDef(ArrayRef<SMLoc> Loc) const {
}
std::optional<unsigned> DagInit::getArgNo(StringRef Name) const {
- for (unsigned i = 0, e = getNumArgs(); i < e; ++i) {
- const StringInit *ArgName = getArgName(i);
+ for (auto [Idx, ArgName] : enumerate(getArgNames())) {
if (ArgName && ArgName->getValue() == Name)
- return i;
+ return Idx;
}
return std::nullopt;
}
@@ -2833,11 +2790,7 @@ const Init *DagInit::resolveReferences(Resolver &R) const {
bool DagInit::isConcrete() const {
if (!Val->isConcrete())
return false;
- for (const Init *Elt : getArgs()) {
- if (!Elt->isConcrete())
- return false;
- }
- return true;
+ return all_of(getArgs(), [](const Init *Elt) { return Elt->isConcrete(); });
}
std::string DagInit::getAsString() const {
@@ -2845,11 +2798,13 @@ std::string DagInit::getAsString() const {
if (ValName)
Result += ":" + ValName->getAsUnquotedString();
if (!arg_empty()) {
- Result += " " + getArg(0)->getAsString();
- if (getArgName(0)) Result += ":$" + getArgName(0)->getAsUnquotedString();
- for (unsigned i = 1, e = getNumArgs(); i != e; ++i) {
- Result += ", " + getArg(i)->getAsString();
- if (getArgName(i)) Result += ":$" + getArgName(i)->getAsUnquotedString();
+ Result += " ";
+ ListSeparator LS;
+ for (auto [Arg, Name] : getArgAndNames()) {
+ Result += LS;
+ Result += Arg->getAsString();
+ if (Name)
+ Result += ":$" + Name->getAsUnquotedString();
}
}
return Result + ")";
@@ -2893,24 +2848,27 @@ std::string RecordVal::getPrintType() const {
}
bool RecordVa...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/138876
More information about the llvm-commits
mailing list