From 3e4e816c2ed9bf0217e69ca5f6eb4fa491dd13e2 Mon Sep 17 00:00:00 2001 From: Visoiu Mistrih Francis <890283+francisvm@users.noreply.github.com> Date: Mon, 26 Feb 2024 18:25:21 -0800 Subject: [PATCH] [CodeGenSchedule] Don't allow invalid ReadAdvances to be formed (#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` commit-id:70afb878 --- .../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 // 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 // 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; + // 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);