[TableGen] Add CodeGenIntrinsicsMap for on-demand intrinsic creation
#107100
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add class `CodeGenIntrinsicMap` for on-demand creation of `CodeGenIntrinsic`. Add class `CodeGenIntrinsicContext` to capture global information required to build `CodeGenIntrinsic` objects. Adopt GlobalISel PatternParser and SearchableTableEmitter to use the new class.
|
Note, the motivation for this is to add better error checking when the number of return values from an intrinsic exceeds what is supported. Once this is in, I'll capture that limit in the CodeGenIntrinsicContext and have the CodeGenIntrinsic constructor error out if the limit exceeds. Today this case spits out with a non-obvious error followed by a assert failure in cast<>. Likely a crash in release builds. |
|
I realized that this is also fixing and/or avoiding potential future bugs where the code in PatternParser or SearchableTableEmitter today is not getting the right properties for the CodeGenIntrinsic objects created. |
|
@llvm/pr-subscribers-llvm-globalisel Author: Rahul Joshi (jurahul) ChangesAdd class Add class Adopt GlobalISel PatternParser and SearchableTableEmitter to use the new class. Full diff: https://github.com/llvm/llvm-project/pull/107100.diff 5 Files Affected:
diff --git a/llvm/utils/TableGen/Basic/CodeGenIntrinsics.cpp b/llvm/utils/TableGen/Basic/CodeGenIntrinsics.cpp
index 4bca904e9f38bd..c0edbf0f01523a 100644
--- a/llvm/utils/TableGen/Basic/CodeGenIntrinsics.cpp
+++ b/llvm/utils/TableGen/Basic/CodeGenIntrinsics.cpp
@@ -25,20 +25,20 @@ using namespace llvm;
// CodeGenIntrinsic Implementation
//===----------------------------------------------------------------------===//
-CodeGenIntrinsicTable::CodeGenIntrinsicTable(const RecordKeeper &RC) {
- std::vector<Record *> IntrProperties =
- RC.getAllDerivedDefinitions("IntrinsicProperty");
-
- std::vector<const Record *> DefaultProperties;
- for (const Record *Rec : IntrProperties)
+CodeGenIntrinsicContext::CodeGenIntrinsicContext(const RecordKeeper &RC) {
+ for (const Record *Rec : RC.getAllDerivedDefinitions("IntrinsicProperty"))
if (Rec->getValueAsBit("IsDefault"))
DefaultProperties.push_back(Rec);
+}
+
+CodeGenIntrinsicTable::CodeGenIntrinsicTable(const RecordKeeper &RC) {
+ CodeGenIntrinsicContext Ctx(RC);
std::vector<Record *> Defs = RC.getAllDerivedDefinitions("Intrinsic");
Intrinsics.reserve(Defs.size());
for (const Record *Def : Defs)
- Intrinsics.push_back(CodeGenIntrinsic(Def, DefaultProperties));
+ Intrinsics.push_back(CodeGenIntrinsic(Def, Ctx));
llvm::sort(Intrinsics,
[](const CodeGenIntrinsic &LHS, const CodeGenIntrinsic &RHS) {
@@ -54,8 +54,18 @@ CodeGenIntrinsicTable::CodeGenIntrinsicTable(const RecordKeeper &RC) {
Targets.back().Count = Intrinsics.size() - Targets.back().Offset;
}
+CodeGenIntrinsic &CodeGenIntrinsicMap::operator[](const Record *Record) {
+ if (!Record->isSubClassOf("Intrinsic"))
+ PrintFatalError("Intrinsic defs should be subclass of 'Intrinsic' class");
+
+ auto [Iter, Inserted] = Map.try_emplace(Record);
+ if (Inserted)
+ Iter->second = std::make_unique<CodeGenIntrinsic>(Record, Ctx);
+ return *Iter->second;
+}
+
CodeGenIntrinsic::CodeGenIntrinsic(const Record *R,
- ArrayRef<const Record *> DefaultProperties)
+ const CodeGenIntrinsicContext &Ctx)
: TheDef(R) {
StringRef DefName = TheDef->getName();
ArrayRef<SMLoc> DefLoc = R->getLoc();
@@ -119,7 +129,7 @@ CodeGenIntrinsic::CodeGenIntrinsic(const Record *R,
}
// Set default properties to true.
- setDefaultProperties(DefaultProperties);
+ setDefaultProperties(Ctx.DefaultProperties);
// Also record the SDPatternOperator Properties.
Properties = parseSDPatternOperatorProperties(R);
diff --git a/llvm/utils/TableGen/Basic/CodeGenIntrinsics.h b/llvm/utils/TableGen/Basic/CodeGenIntrinsics.h
index 19e29af2fa85f3..51c23591553802 100644
--- a/llvm/utils/TableGen/Basic/CodeGenIntrinsics.h
+++ b/llvm/utils/TableGen/Basic/CodeGenIntrinsics.h
@@ -15,6 +15,7 @@
#include "SDNodeProperties.h"
#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/Support/ModRef.h"
#include <string>
@@ -25,6 +26,12 @@ namespace llvm {
class Record;
class RecordKeeper;
+// Global information needed to build intrinsics.
+struct CodeGenIntrinsicContext {
+ explicit CodeGenIntrinsicContext(const RecordKeeper &RC);
+ std::vector<const Record *> DefaultProperties;
+};
+
struct CodeGenIntrinsic {
const Record *TheDef; // The actual record defining this intrinsic.
std::string Name; // The name of the LLVM function "llvm.bswap.i32"
@@ -155,8 +162,7 @@ struct CodeGenIntrinsic {
bool isParamImmArg(unsigned ParamIdx) const;
- CodeGenIntrinsic(const Record *R,
- ArrayRef<const Record *> DefaultProperties = {});
+ CodeGenIntrinsic(const Record *R, const CodeGenIntrinsicContext &Ctx);
};
class CodeGenIntrinsicTable {
@@ -171,7 +177,6 @@ class CodeGenIntrinsicTable {
std::vector<TargetSet> Targets;
explicit CodeGenIntrinsicTable(const RecordKeeper &RC);
- CodeGenIntrinsicTable() = default;
bool empty() const { return Intrinsics.empty(); }
size_t size() const { return Intrinsics.size(); }
@@ -182,6 +187,17 @@ class CodeGenIntrinsicTable {
return Intrinsics[Pos];
}
};
+
+// This class builds `CodeGenIntrinsic` on demand for a given Def.
+class CodeGenIntrinsicMap {
+ DenseMap<const Record *, std::unique_ptr<CodeGenIntrinsic>> Map;
+ const CodeGenIntrinsicContext Ctx;
+
+public:
+ explicit CodeGenIntrinsicMap(const RecordKeeper &RC) : Ctx(RC) {}
+ CodeGenIntrinsic &operator[](const Record *Def);
+};
+
} // namespace llvm
#endif // LLVM_UTILS_TABLEGEN_BASIC_CODEGENINTRINSICS_H
diff --git a/llvm/utils/TableGen/Common/CodeGenDAGPatterns.cpp b/llvm/utils/TableGen/Common/CodeGenDAGPatterns.cpp
index a8cecca0d4a54f..df3f72ff2ec7f5 100644
--- a/llvm/utils/TableGen/Common/CodeGenDAGPatterns.cpp
+++ b/llvm/utils/TableGen/Common/CodeGenDAGPatterns.cpp
@@ -3160,10 +3160,8 @@ void TreePattern::dump() const { print(errs()); }
CodeGenDAGPatterns::CodeGenDAGPatterns(RecordKeeper &R,
PatternRewriterFn PatternRewriter)
- : Records(R), Target(R), LegalVTS(Target.getLegalValueTypes()),
- PatternRewriter(PatternRewriter) {
-
- Intrinsics = CodeGenIntrinsicTable(Records);
+ : Records(R), Target(R), Intrinsics(R),
+ LegalVTS(Target.getLegalValueTypes()), PatternRewriter(PatternRewriter) {
ParseNodeInfo();
ParseNodeTransforms();
ParseComplexPatterns();
diff --git a/llvm/utils/TableGen/Common/GlobalISel/PatternParser.cpp b/llvm/utils/TableGen/Common/GlobalISel/PatternParser.cpp
index 82fa3c8f3121f0..73b6097554edad 100644
--- a/llvm/utils/TableGen/Common/GlobalISel/PatternParser.cpp
+++ b/llvm/utils/TableGen/Common/GlobalISel/PatternParser.cpp
@@ -107,13 +107,10 @@ getInstrForIntrinsic(const CodeGenTarget &CGT, const CodeGenIntrinsic *I) {
static const CodeGenIntrinsic *getCodeGenIntrinsic(Record *R) {
// Intrinsics need to have a static lifetime because the match table keeps
// references to CodeGenIntrinsic objects.
- static DenseMap<const Record *, std::unique_ptr<CodeGenIntrinsic>>
- AllIntrinsics;
-
- auto &Ptr = AllIntrinsics[R];
- if (!Ptr)
- Ptr = std::make_unique<CodeGenIntrinsic>(R);
- return Ptr.get();
+ static CodeGenIntrinsicMap *AllIntrinsics;
+ if (!AllIntrinsics)
+ AllIntrinsics = new CodeGenIntrinsicMap(R->getRecords());
+ return &(*AllIntrinsics)[R];
}
std::unique_ptr<Pattern>
diff --git a/llvm/utils/TableGen/SearchableTableEmitter.cpp b/llvm/utils/TableGen/SearchableTableEmitter.cpp
index 59ae3bf3daedcb..8d394f8051ced5 100644
--- a/llvm/utils/TableGen/SearchableTableEmitter.cpp
+++ b/llvm/utils/TableGen/SearchableTableEmitter.cpp
@@ -94,7 +94,7 @@ struct GenericTable {
class SearchableTableEmitter {
RecordKeeper &Records;
std::unique_ptr<CodeGenTarget> Target;
- DenseMap<Init *, std::unique_ptr<CodeGenIntrinsic>> Intrinsics;
+ std::unique_ptr<CodeGenIntrinsicMap> Intrinsics;
std::vector<std::unique_ptr<GenericEnum>> Enums;
DenseMap<Record *, GenericEnum *> EnumMap;
std::set<std::string> PreprocessorGuards;
@@ -146,10 +146,13 @@ class SearchableTableEmitter {
}
CodeGenIntrinsic &getIntrinsic(Init *I) {
- std::unique_ptr<CodeGenIntrinsic> &Intr = Intrinsics[I];
- if (!Intr)
- Intr = std::make_unique<CodeGenIntrinsic>(cast<DefInit>(I)->getDef());
- return *Intr;
+ const Record *Def = cast<DefInit>(I)->getDef();
+ // Build the Intrinsics map on demand. If we instantiate one in the
+ // constructor, we may get errors if the TableGen file being processed does
+ // not include Intrinsics.td and does not do anything with intrinsics.
+ if (!Intrinsics)
+ Intrinsics = std::make_unique<CodeGenIntrinsicMap>(Records);
+ return (*Intrinsics)[Def];
}
bool compareBy(Record *LHS, Record *RHS, const SearchIndex &Index);
|
arsenm
reviewed
Sep 4, 2024
arsenm
approved these changes
Sep 4, 2024
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/30/builds/5473 Here is the relevant piece of the build log for the reference |
These errors seem unrelated to probably its a misattribution. But will keep a watch if I see other messages. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
CodeGenIntrinsicMapfor on-demand creation ofCodeGenIntrinsic.CodeGenIntrinsicContextto capture global informationrequired to build
CodeGenIntrinsicobjects.