[TableGen] Add maps from Write/ReadType to the parent WriteRes/ReadAdvance. NFC #123876
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
We had two WriteRes for WriteJalr with difference latencies. I don't know which is correct. I chose Latency=2 to match WriteJal.
…Advance. NFC Use this to improve performance of SubtargetEmitter::findWriteResources and SubtargetEmitter::findReadAdvance. Now we can do a map lookup instead of a linear search through all WriteRes/ReadAdvance records. This reduces the build time of RISCVGenSubtargetInfo.inc on my machine from 43 seconds to 10 seconds.
topperc
requested review from
mshockwave,
michaelmaitland,
s-barannikov and
jurahul
|
@llvm/pr-subscribers-backend-risc-v Author: Craig Topper (topperc) ChangesUse this to improve performance of SubtargetEmitter::findWriteResources This reduces the build time of RISCVGenSubtargetInfo.inc on my Stacked on #123865 Full diff: https://github.com/llvm/llvm-project/pull/123876.diff 4 Files Affected:
diff --git a/llvm/lib/Target/RISCV/RISCVSchedMIPSP8700.td b/llvm/lib/Target/RISCV/RISCVSchedMIPSP8700.td
index 550f83a59b8b0e..ab6402dc96af10 100644
--- a/llvm/lib/Target/RISCV/RISCVSchedMIPSP8700.td
+++ b/llvm/lib/Target/RISCV/RISCVSchedMIPSP8700.td
@@ -125,7 +125,6 @@ def : WriteRes<WriteCSR, [p8700ALQ]>;
// Handle CTI Pipeline.
def : WriteRes<WriteJmp, [p8700IssueCTI]>;
-def : WriteRes<WriteJalr, [p8700IssueCTI]>;
let Latency = 2 in {
def : WriteRes<WriteJal, [p8700IssueCTI]>;
def : WriteRes<WriteJalr, [p8700IssueCTI]>;
diff --git a/llvm/utils/TableGen/Common/CodeGenSchedule.cpp b/llvm/utils/TableGen/Common/CodeGenSchedule.cpp
index 8eaba05e65ce92..2a42262f865cb9 100644
--- a/llvm/utils/TableGen/Common/CodeGenSchedule.cpp
+++ b/llvm/utils/TableGen/Common/CodeGenSchedule.cpp
@@ -2112,6 +2112,14 @@ void CodeGenSchedModels::addWriteRes(const Record *ProcWriteResDef,
return;
WRDefs.push_back(ProcWriteResDef);
+ if (ProcWriteResDef->isSubClassOf("WriteRes")) {
+ auto &WRMap = ProcModels[PIdx].WriteResMap;
+ const Record *WRDef = ProcWriteResDef->getValueAsDef("WriteType");
+ if (!WRMap.try_emplace(WRDef, ProcWriteResDef).second)
+ PrintFatalError(ProcWriteResDef->getLoc(),
+ "WriteType already used in another WriteRes");
+ }
+
// Visit ProcResourceKinds referenced by the newly discovered WriteRes.
for (const Record *ProcResDef :
ProcWriteResDef->getValueAsListOfDefs("ProcResources")) {
@@ -2135,6 +2143,14 @@ void CodeGenSchedModels::addReadAdvance(const Record *ProcReadAdvanceDef,
if (is_contained(RADefs, ProcReadAdvanceDef))
return;
RADefs.push_back(ProcReadAdvanceDef);
+
+ if (ProcReadAdvanceDef->isSubClassOf("ReadAdvance")) {
+ auto &RAMap = ProcModels[PIdx].ReadAdvanceMap;
+ const Record *RADef = ProcReadAdvanceDef->getValueAsDef("ReadType");
+ if (!RAMap.try_emplace(RADef, ProcReadAdvanceDef).second)
+ PrintFatalError(ProcReadAdvanceDef->getLoc(),
+ "ReadType already used in another ReadAdvance");
+ }
}
unsigned CodeGenProcModel::getProcResourceIdx(const Record *PRDef) const {
diff --git a/llvm/utils/TableGen/Common/CodeGenSchedule.h b/llvm/utils/TableGen/Common/CodeGenSchedule.h
index fed8b3e1ccb8a6..467b77e8acba31 100644
--- a/llvm/utils/TableGen/Common/CodeGenSchedule.h
+++ b/llvm/utils/TableGen/Common/CodeGenSchedule.h
@@ -244,6 +244,12 @@ struct CodeGenProcModel {
ConstRecVec WriteResDefs;
ConstRecVec ReadAdvanceDefs;
+ // Map from the WriteType field to the parent WriteRes record.
+ DenseMap<const Record *, const Record *> WriteResMap;
+
+ // Map from the ReadType field to the parent ReadAdvance record.
+ DenseMap<const Record *, const Record *> ReadAdvanceMap;
+
// Per-operand machine model resources associated with this processor.
ConstRecVec ProcResourceDefs;
diff --git a/llvm/utils/TableGen/SubtargetEmitter.cpp b/llvm/utils/TableGen/SubtargetEmitter.cpp
index 1120f06875c778..3db3ae65cc5557 100644
--- a/llvm/utils/TableGen/SubtargetEmitter.cpp
+++ b/llvm/utils/TableGen/SubtargetEmitter.cpp
@@ -943,24 +943,23 @@ SubtargetEmitter::findWriteResources(const CodeGenSchedRW &SchedWrite,
// Check this processor's list of write resources.
const Record *ResDef = nullptr;
- for (const Record *WR : ProcModel.WriteResDefs) {
- if (!WR->isSubClassOf("WriteRes"))
- continue;
- const Record *WRDef = WR->getValueAsDef("WriteType");
- if (AliasDef == WRDef || SchedWrite.TheDef == WRDef) {
- if (ResDef) {
- PrintFatalError(WR->getLoc(), "Resources are defined for both "
- "SchedWrite and its alias on processor " +
- ProcModel.ModelName);
- }
- ResDef = WR;
- // If there is no AliasDef and we find a match, we can early exit since
- // there is no need to verify whether there are resources defined for both
- // SchedWrite and its alias.
- if (!AliasDef)
- break;
+
+ auto I = ProcModel.WriteResMap.find(SchedWrite.TheDef);
+ if (I != ProcModel.WriteResMap.end())
+ ResDef = I->second;
+
+ if (AliasDef) {
+ I = ProcModel.WriteResMap.find(AliasDef);
+ if (I != ProcModel.WriteResMap.end()) {
+ if (ResDef)
+ PrintFatalError(I->second->getLoc(),
+ "Resources are defined for both SchedWrite and its "
+ "alias on processor " +
+ ProcModel.ModelName);
+ ResDef = I->second;
}
}
+
// TODO: If ProcModel has a base model (previous generation processor),
// then call FindWriteResources recursively with that model here.
if (!ResDef) {
@@ -1003,24 +1002,24 @@ SubtargetEmitter::findReadAdvance(const CodeGenSchedRW &SchedRead,
// Check this processor's ReadAdvanceList.
const Record *ResDef = nullptr;
- for (const Record *RA : ProcModel.ReadAdvanceDefs) {
- if (!RA->isSubClassOf("ReadAdvance"))
- continue;
- const Record *RADef = RA->getValueAsDef("ReadType");
- if (AliasDef == RADef || SchedRead.TheDef == RADef) {
- if (ResDef) {
- PrintFatalError(RA->getLoc(), "Resources are defined for both "
- "SchedRead and its alias on processor " +
- ProcModel.ModelName);
- }
- ResDef = RA;
- // If there is no AliasDef and we find a match, we can early exit since
- // there is no need to verify whether there are resources defined for both
- // SchedRead and its alias.
- if (!AliasDef)
- break;
+
+ auto I = ProcModel.ReadAdvanceMap.find(SchedRead.TheDef);
+ if (I != ProcModel.ReadAdvanceMap.end())
+ ResDef = I->second;
+
+ if (AliasDef) {
+ I = ProcModel.ReadAdvanceMap.find(AliasDef);
+ if (I != ProcModel.ReadAdvanceMap.end()) {
+ if (ResDef)
+ PrintFatalError(
+ I->second->getLoc(),
+ "Resources are defined for both SchedRead and its alias on "
+ "processor " +
+ ProcModel.ModelName);
+ ResDef = I->second;
}
}
+
// TODO: If ProcModel has a base model (previous generation processor),
// then call FindReadAdvance recursively with that model here.
if (!ResDef && SchedRead.TheDef->getName() != "ReadDefault") {
|
|
@llvm/pr-subscribers-tablegen Author: Craig Topper (topperc) ChangesUse this to improve performance of SubtargetEmitter::findWriteResources This reduces the build time of RISCVGenSubtargetInfo.inc on my Stacked on #123865 Full diff: https://github.com/llvm/llvm-project/pull/123876.diff 4 Files Affected:
diff --git a/llvm/lib/Target/RISCV/RISCVSchedMIPSP8700.td b/llvm/lib/Target/RISCV/RISCVSchedMIPSP8700.td
index 550f83a59b8b0e..ab6402dc96af10 100644
--- a/llvm/lib/Target/RISCV/RISCVSchedMIPSP8700.td
+++ b/llvm/lib/Target/RISCV/RISCVSchedMIPSP8700.td
@@ -125,7 +125,6 @@ def : WriteRes<WriteCSR, [p8700ALQ]>;
// Handle CTI Pipeline.
def : WriteRes<WriteJmp, [p8700IssueCTI]>;
-def : WriteRes<WriteJalr, [p8700IssueCTI]>;
let Latency = 2 in {
def : WriteRes<WriteJal, [p8700IssueCTI]>;
def : WriteRes<WriteJalr, [p8700IssueCTI]>;
diff --git a/llvm/utils/TableGen/Common/CodeGenSchedule.cpp b/llvm/utils/TableGen/Common/CodeGenSchedule.cpp
index 8eaba05e65ce92..2a42262f865cb9 100644
--- a/llvm/utils/TableGen/Common/CodeGenSchedule.cpp
+++ b/llvm/utils/TableGen/Common/CodeGenSchedule.cpp
@@ -2112,6 +2112,14 @@ void CodeGenSchedModels::addWriteRes(const Record *ProcWriteResDef,
return;
WRDefs.push_back(ProcWriteResDef);
+ if (ProcWriteResDef->isSubClassOf("WriteRes")) {
+ auto &WRMap = ProcModels[PIdx].WriteResMap;
+ const Record *WRDef = ProcWriteResDef->getValueAsDef("WriteType");
+ if (!WRMap.try_emplace(WRDef, ProcWriteResDef).second)
+ PrintFatalError(ProcWriteResDef->getLoc(),
+ "WriteType already used in another WriteRes");
+ }
+
// Visit ProcResourceKinds referenced by the newly discovered WriteRes.
for (const Record *ProcResDef :
ProcWriteResDef->getValueAsListOfDefs("ProcResources")) {
@@ -2135,6 +2143,14 @@ void CodeGenSchedModels::addReadAdvance(const Record *ProcReadAdvanceDef,
if (is_contained(RADefs, ProcReadAdvanceDef))
return;
RADefs.push_back(ProcReadAdvanceDef);
+
+ if (ProcReadAdvanceDef->isSubClassOf("ReadAdvance")) {
+ auto &RAMap = ProcModels[PIdx].ReadAdvanceMap;
+ const Record *RADef = ProcReadAdvanceDef->getValueAsDef("ReadType");
+ if (!RAMap.try_emplace(RADef, ProcReadAdvanceDef).second)
+ PrintFatalError(ProcReadAdvanceDef->getLoc(),
+ "ReadType already used in another ReadAdvance");
+ }
}
unsigned CodeGenProcModel::getProcResourceIdx(const Record *PRDef) const {
diff --git a/llvm/utils/TableGen/Common/CodeGenSchedule.h b/llvm/utils/TableGen/Common/CodeGenSchedule.h
index fed8b3e1ccb8a6..467b77e8acba31 100644
--- a/llvm/utils/TableGen/Common/CodeGenSchedule.h
+++ b/llvm/utils/TableGen/Common/CodeGenSchedule.h
@@ -244,6 +244,12 @@ struct CodeGenProcModel {
ConstRecVec WriteResDefs;
ConstRecVec ReadAdvanceDefs;
+ // Map from the WriteType field to the parent WriteRes record.
+ DenseMap<const Record *, const Record *> WriteResMap;
+
+ // Map from the ReadType field to the parent ReadAdvance record.
+ DenseMap<const Record *, const Record *> ReadAdvanceMap;
+
// Per-operand machine model resources associated with this processor.
ConstRecVec ProcResourceDefs;
diff --git a/llvm/utils/TableGen/SubtargetEmitter.cpp b/llvm/utils/TableGen/SubtargetEmitter.cpp
index 1120f06875c778..3db3ae65cc5557 100644
--- a/llvm/utils/TableGen/SubtargetEmitter.cpp
+++ b/llvm/utils/TableGen/SubtargetEmitter.cpp
@@ -943,24 +943,23 @@ SubtargetEmitter::findWriteResources(const CodeGenSchedRW &SchedWrite,
// Check this processor's list of write resources.
const Record *ResDef = nullptr;
- for (const Record *WR : ProcModel.WriteResDefs) {
- if (!WR->isSubClassOf("WriteRes"))
- continue;
- const Record *WRDef = WR->getValueAsDef("WriteType");
- if (AliasDef == WRDef || SchedWrite.TheDef == WRDef) {
- if (ResDef) {
- PrintFatalError(WR->getLoc(), "Resources are defined for both "
- "SchedWrite and its alias on processor " +
- ProcModel.ModelName);
- }
- ResDef = WR;
- // If there is no AliasDef and we find a match, we can early exit since
- // there is no need to verify whether there are resources defined for both
- // SchedWrite and its alias.
- if (!AliasDef)
- break;
+
+ auto I = ProcModel.WriteResMap.find(SchedWrite.TheDef);
+ if (I != ProcModel.WriteResMap.end())
+ ResDef = I->second;
+
+ if (AliasDef) {
+ I = ProcModel.WriteResMap.find(AliasDef);
+ if (I != ProcModel.WriteResMap.end()) {
+ if (ResDef)
+ PrintFatalError(I->second->getLoc(),
+ "Resources are defined for both SchedWrite and its "
+ "alias on processor " +
+ ProcModel.ModelName);
+ ResDef = I->second;
}
}
+
// TODO: If ProcModel has a base model (previous generation processor),
// then call FindWriteResources recursively with that model here.
if (!ResDef) {
@@ -1003,24 +1002,24 @@ SubtargetEmitter::findReadAdvance(const CodeGenSchedRW &SchedRead,
// Check this processor's ReadAdvanceList.
const Record *ResDef = nullptr;
- for (const Record *RA : ProcModel.ReadAdvanceDefs) {
- if (!RA->isSubClassOf("ReadAdvance"))
- continue;
- const Record *RADef = RA->getValueAsDef("ReadType");
- if (AliasDef == RADef || SchedRead.TheDef == RADef) {
- if (ResDef) {
- PrintFatalError(RA->getLoc(), "Resources are defined for both "
- "SchedRead and its alias on processor " +
- ProcModel.ModelName);
- }
- ResDef = RA;
- // If there is no AliasDef and we find a match, we can early exit since
- // there is no need to verify whether there are resources defined for both
- // SchedRead and its alias.
- if (!AliasDef)
- break;
+
+ auto I = ProcModel.ReadAdvanceMap.find(SchedRead.TheDef);
+ if (I != ProcModel.ReadAdvanceMap.end())
+ ResDef = I->second;
+
+ if (AliasDef) {
+ I = ProcModel.ReadAdvanceMap.find(AliasDef);
+ if (I != ProcModel.ReadAdvanceMap.end()) {
+ if (ResDef)
+ PrintFatalError(
+ I->second->getLoc(),
+ "Resources are defined for both SchedRead and its alias on "
+ "processor " +
+ ProcModel.ModelName);
+ ResDef = I->second;
}
}
+
// TODO: If ProcModel has a base model (previous generation processor),
// then call FindReadAdvance recursively with that model here.
if (!ResDef && SchedRead.TheDef->getName() != "ReadDefault") {
|
topperc
changed the title
mshockwave
approved these changes
Jan 22, 2025
Nice! |
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.
Use this to improve performance of SubtargetEmitter::findWriteResources
and SubtargetEmitter::findReadAdvance. Now we can do a map lookup
instead of a linear search through all WriteRes/ReadAdvance records.
This reduces the build time of RISCVGenSubtargetInfo.inc on my
machine from 43 seconds to 10 seconds.
Stacked on #123865