[llvm] [CodeGenSchedule] Don't allow invalid ReadAdvances to be formed (PR #82685)
Visoiu Mistrih Francis via llvm-commits
llvm-commits at lists.llvm.org
Thu Feb 22 16:34:53 PST 2024
https://github.com/francisvm updated https://github.com/llvm/llvm-project/pull/82685
>From b48fb81038fc5f8058ac29ef88f37716a9dcc831 Mon Sep 17 00:00:00 2001
From: Francis Visoiu Mistrih <francisvm at apple.com>
Date: Thu, 22 Feb 2024 12:30:59 -0800
Subject: [PATCH] [CodeGenSchedule] Don't allow invalid ReadAdvances to be
formed
Forming a `ReadAdvance` with an entry in the `ValidWrites` list that is not
used by any instruction results in the entire `ReadAdvance` to be ignored
by the scheduler due to an invalid entry.
The `SchedRW` collection code only picks up `SchedWrites` that are
reachable from `Instructions`, `InstRW`, `ItinRW` and `SchedAlias`,
leaving the unreachable ones with an invalid entry (0) in
`SubtargetEmitter::GenSchedClassTables` when going through the list of
`ReadAdvances`
---
.../Target/AArch64/AArch64SchedExynosM4.td | 4 +--
.../Target/AArch64/AArch64SchedExynosM5.td | 4 +--
llvm/test/TableGen/ReadAdvanceInvalidWrite.td | 29 +++++++++++++++++++
.../AArch64/Exynos/float-divide-multiply.s | 12 ++++----
llvm/utils/TableGen/CodeGenSchedule.cpp | 9 ++++++
llvm/utils/TableGen/SubtargetEmitter.cpp | 5 +++-
6 files changed, 50 insertions(+), 13 deletions(-)
create mode 100644 llvm/test/TableGen/ReadAdvanceInvalidWrite.td
diff --git a/llvm/lib/Target/AArch64/AArch64SchedExynosM4.td b/llvm/lib/Target/AArch64/AArch64SchedExynosM4.td
index 5163de280f2e4f..b75264602dbc11 100644
--- a/llvm/lib/Target/AArch64/AArch64SchedExynosM4.td
+++ b/llvm/lib/Target/AArch64/AArch64SchedExynosM4.td
@@ -309,7 +309,6 @@ def M4WriteFMAC3H : SchedWriteRes<[M4UnitFMACH]> { let Latency = 3; }
def M4WriteFMAC3 : SchedWriteRes<[M4UnitFMAC]> { let Latency = 3; }
def M4WriteFMAC4 : SchedWriteRes<[M4UnitFMAC]> { let Latency = 4; }
def M4WriteFMAC4H : SchedWriteRes<[M4UnitFMACH]> { let Latency = 4; }
-def M4WriteFMAC5 : SchedWriteRes<[M4UnitFMAC]> { let Latency = 5; }
def M4WriteFSQR7H : SchedWriteRes<[M4UnitFSQRH]> { let Latency = 7;
let ReleaseAtCycles = [6]; }
@@ -495,8 +494,7 @@ def M4WriteMOVI : SchedWriteVariant<[SchedVar<IsZeroFPIdiomPred, [M4WriteZ0]>
// Fast forwarding.
def M4ReadAESM1 : SchedReadAdvance<+1, [M4WriteNCRY1]>;
def M4ReadFMACM1 : SchedReadAdvance<+1, [M4WriteFMAC4,
- M4WriteFMAC4H,
- M4WriteFMAC5]>;
+ M4WriteFMAC4H]>;
def M4ReadNMULM1 : SchedReadAdvance<+1, [M4WriteNMUL3]>;
def M4ReadNMULP2 : SchedReadAdvance<-2, [M4WriteNMUL3]>;
diff --git a/llvm/lib/Target/AArch64/AArch64SchedExynosM5.td b/llvm/lib/Target/AArch64/AArch64SchedExynosM5.td
index 2ccbe1614dcd79..6b5a6da76b3a81 100644
--- a/llvm/lib/Target/AArch64/AArch64SchedExynosM5.td
+++ b/llvm/lib/Target/AArch64/AArch64SchedExynosM5.td
@@ -338,7 +338,6 @@ def M5WriteFDIV12 : SchedWriteRes<[M5UnitFDIV]> { let Latency = 12;
def M5WriteFMAC3 : SchedWriteRes<[M5UnitFMAC]> { let Latency = 3; }
def M5WriteFMAC4 : SchedWriteRes<[M5UnitFMAC]> { let Latency = 4; }
-def M5WriteFMAC5 : SchedWriteRes<[M5UnitFMAC]> { let Latency = 5; }
def M5WriteFSQR5 : SchedWriteRes<[M5UnitFSQR]> { let Latency = 5;
let ReleaseAtCycles = [2]; }
@@ -530,8 +529,7 @@ def M5WriteMOVI : SchedWriteVariant<[SchedVar<IsZeroFPIdiomPred, [M5WriteZ0]>
// Fast forwarding.
def M5ReadFM1 : SchedReadAdvance<+1, [M5WriteF2]>;
def M5ReadAESM2 : SchedReadAdvance<+2, [M5WriteNCRY2]>;
-def M5ReadFMACM1 : SchedReadAdvance<+1, [M5WriteFMAC4,
- M5WriteFMAC5]>;
+def M5ReadFMACM1 : SchedReadAdvance<+1, [M5WriteFMAC4]>;
def M5ReadNMULM1 : SchedReadAdvance<+1, [M5WriteNMUL3]>;
//===----------------------------------------------------------------------===//
diff --git a/llvm/test/TableGen/ReadAdvanceInvalidWrite.td b/llvm/test/TableGen/ReadAdvanceInvalidWrite.td
new file mode 100644
index 00000000000000..d185cbd56f8e96
--- /dev/null
+++ b/llvm/test/TableGen/ReadAdvanceInvalidWrite.td
@@ -0,0 +1,29 @@
+// RUN: not llvm-tblgen -gen-subtarget -I %p/../../include %s 2>&1 | FileCheck %s
+
+// Make sure we don't form ReadAdvances with ValidWrites entries that are not
+// associated with any instructions.
+
+include "llvm/Target/Target.td"
+
+def TargetX : Target;
+
+def WriteX : SchedWrite;
+def WriteY : SchedWrite;
+def ReadX : SchedRead;
+
+def InstX : Instruction {
+ let OutOperandList = (outs);
+ let InOperandList = (ins);
+ let SchedRW = [WriteX, ReadX];
+}
+
+def SchedModelX: SchedMachineModel {
+ let CompleteModel = 0;
+}
+
+let SchedModel = SchedModelX in {
+ def : ReadAdvance<ReadX, 1, [WriteX, WriteY]>;
+ // CHECK: error: ReadAdvance referencing a ValidWrite that is not used by any instruction (WriteY)
+}
+
+def ProcessorX: ProcessorModel<"ProcessorX", SchedModelX, []>;
diff --git a/llvm/test/tools/llvm-mca/AArch64/Exynos/float-divide-multiply.s b/llvm/test/tools/llvm-mca/AArch64/Exynos/float-divide-multiply.s
index a24d8a279606cf..ecfd019452afcd 100644
--- a/llvm/test/tools/llvm-mca/AArch64/Exynos/float-divide-multiply.s
+++ b/llvm/test/tools/llvm-mca/AArch64/Exynos/float-divide-multiply.s
@@ -26,11 +26,11 @@ fsqrt d11, d12
# EM3-NEXT: Total uOps: 800
# EM4-NEXT: Instructions: 1200
-# EM4-NEXT: Total Cycles: 575
+# EM4-NEXT: Total Cycles: 572
# EM4-NEXT: Total uOps: 1200
# EM5-NEXT: Instructions: 1200
-# EM5-NEXT: Total Cycles: 433
+# EM5-NEXT: Total Cycles: 434
# EM5-NEXT: Total uOps: 1200
# ALL: Dispatch Width: 6
@@ -39,12 +39,12 @@ fsqrt d11, d12
# EM3-NEXT: IPC: 0.18
# EM3-NEXT: Block RThroughput: 45.0
-# EM4-NEXT: uOps Per Cycle: 2.09
-# EM4-NEXT: IPC: 2.09
+# EM4-NEXT: uOps Per Cycle: 2.10
+# EM4-NEXT: IPC: 2.10
# EM4-NEXT: Block RThroughput: 4.0
-# EM5-NEXT: uOps Per Cycle: 2.77
-# EM5-NEXT: IPC: 2.77
+# EM5-NEXT: uOps Per Cycle: 2.76
+# EM5-NEXT: IPC: 2.76
# EM5-NEXT: Block RThroughput: 4.0
# ALL: Instruction Info:
diff --git a/llvm/utils/TableGen/CodeGenSchedule.cpp b/llvm/utils/TableGen/CodeGenSchedule.cpp
index b4c624703626c3..d819016f8b5610 100644
--- a/llvm/utils/TableGen/CodeGenSchedule.cpp
+++ b/llvm/utils/TableGen/CodeGenSchedule.cpp
@@ -2190,6 +2190,15 @@ void CodeGenSchedModels::addWriteRes(Record *ProcWriteResDef, unsigned PIdx) {
// Add resources for a ReadAdvance to this processor if they don't exist.
void CodeGenSchedModels::addReadAdvance(Record *ProcReadAdvanceDef,
unsigned PIdx) {
+ for (const Record *ValidWrite :
+ ProcReadAdvanceDef->getValueAsListOfDefs("ValidWrites"))
+ if (getSchedRWIdx(ValidWrite, /*IsRead=*/false) == 0)
+ PrintFatalError(
+ ProcReadAdvanceDef->getLoc(),
+ "ReadAdvance referencing a ValidWrite that is not used by "
+ "any instruction (" +
+ ValidWrite->getName() + ")");
+
RecVec &RADefs = ProcModels[PIdx].ReadAdvanceDefs;
if (is_contained(RADefs, ProcReadAdvanceDef))
return;
diff --git a/llvm/utils/TableGen/SubtargetEmitter.cpp b/llvm/utils/TableGen/SubtargetEmitter.cpp
index 2707f54eed6e9b..d350d7de139f6e 100644
--- a/llvm/utils/TableGen/SubtargetEmitter.cpp
+++ b/llvm/utils/TableGen/SubtargetEmitter.cpp
@@ -1262,7 +1262,10 @@ void SubtargetEmitter::GenSchedClassTables(const CodeGenProcModel &ProcModel,
WriteIDs.push_back(0);
else {
for (Record *VW : ValidWrites) {
- WriteIDs.push_back(SchedModels.getSchedRWIdx(VW, /*IsRead=*/false));
+ unsigned WriteID = SchedModels.getSchedRWIdx(VW, /*IsRead=*/false);
+ assert(WriteID != 0 &&
+ "Expected a valid SchedRW in the list of ValidWrites");
+ WriteIDs.push_back(WriteID);
}
}
llvm::sort(WriteIDs);
More information about the llvm-commits
mailing list