[llvm] [CodeGenSchedule] Don't allow invalid ReadAdvances to be formed (PR #82685)

[Visoiu Mistrih Francis via llvm-commits]

https://github.com/francisvm created https://github.com/llvm/llvm-project/pull/82685

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`

>From 5f7e794e3fdc4046290195b26a0b56c866c0a1dc 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`
---
 llvm/test/TableGen/ReadAdvanceInvalidWrite.td | 29 +++++++++++++++++++
 llvm/utils/TableGen/CodeGenSchedule.cpp       |  9 ++++++
 llvm/utils/TableGen/SubtargetEmitter.cpp      |  5 +++-
 3 files changed, 42 insertions(+), 1 deletion(-)
 create mode 100644 llvm/test/TableGen/ReadAdvanceInvalidWrite.td

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/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);