From b64db7cfe74e4b488437a57e4e540c251f71122b Mon Sep 17 00:00:00 2001 From: Schuyler Eldridge Date: Thu, 28 Mar 2024 20:12:30 -0400 Subject: [PATCH 1/2] [FIRRTL] Treat blackboxes in layers as "testbench" When determining which directory to place a blackbox into, make two changes to get this working correctly with layers: 1. Never change a directory if one is already specified. 2. Treat any bound instance as not under the DUT. Taken together, this preserves both the original behavior of grand central blackbox extraction and causes blackboxes instantiated under layers to be placed in the testbench directory. Fixes #6880. Signed-off-by: Schuyler Eldridge --- .../FIRRTL/Transforms/BlackBoxReader.cpp | 12 +-- test/firtool/blackbox-directories.fir | 76 +++++++++++++++++-- 2 files changed, 76 insertions(+), 12 deletions(-) diff --git a/lib/Dialect/FIRRTL/Transforms/BlackBoxReader.cpp b/lib/Dialect/FIRRTL/Transforms/BlackBoxReader.cpp index 276135471616..75a72de4e4e5 100644 --- a/lib/Dialect/FIRRTL/Transforms/BlackBoxReader.cpp +++ b/lib/Dialect/FIRRTL/Transforms/BlackBoxReader.cpp @@ -398,8 +398,6 @@ StringAttr BlackBoxReaderPass::loadFile(Operation *op, StringRef inputPath, OutputFileInfo BlackBoxReaderPass::getOutputFile(Operation *origOp, StringAttr fileNameAttr, bool isCover) { - // If the original operation has a specified output file that is not a - // directory, then just use that. auto outputFile = origOp->getAttrOfType("output_file"); if (outputFile && !outputFile.isDirectory()) { return {outputFile.getFilename(), Priority::TargetDir, @@ -413,16 +411,18 @@ OutputFileInfo BlackBoxReaderPass::getOutputFile(Operation *origOp, auto ext = llvm::sys::path::extension(fileName); bool exclude = (ext == ".h" || ext == ".vh" || ext == ".svh"); auto outDir = std::make_pair(targetDir, Priority::TargetDir); + // If the original operation has a specified output file that is not a + // directory, then just use that. + if (outputFile) + outDir = {outputFile.getFilename(), Priority::Explicit}; // In order to output into the testbench directory, we need to have a // testbench dir annotation, not have a blackbox target directory annotation // (or one set to the current directory), have a DUT annotation, and the // module needs to be in or under the DUT. - if (!testBenchDir.empty() && targetDir.equals(".") && dut && !isDut(origOp)) + else if (!testBenchDir.empty() && targetDir.equals(".") && dut && !isDut(origOp)) outDir = {testBenchDir, Priority::TestBench}; else if (isCover) outDir = {coverDir, Priority::Verification}; - else if (outputFile) - outDir = {outputFile.getFilename(), Priority::Explicit}; // If targetDir is not set explicitly and this is a testbench module, then // update the targetDir to be the "../testbench". @@ -448,6 +448,8 @@ bool BlackBoxReaderPass::isDut(Operation *module) { bool anyParentIsDut = false; if (node) for (auto *u : node->uses()) { + if (cast(u->getInstance().getOperation()).getLowerToBind()) + return false; // Recursively check the parents. auto dut = isDut(u->getInstance()->getParentOfType()); // Cache the result. diff --git a/test/firtool/blackbox-directories.fir b/test/firtool/blackbox-directories.fir index 5f6ffc6e6fdf..bd865fe17621 100644 --- a/test/firtool/blackbox-directories.fir +++ b/test/firtool/blackbox-directories.fir @@ -7,11 +7,14 @@ ; ; The test enumerates all possible combinations for a simple design that ; consists of a TestHarness top module, a DUT instantiated by the TestHarness, -; and a Grand Central companion module inside the DUT. This is summarized by -; the table below. A one in the "T" column indicates it is instantiated by the -; TestHarness, a one in the "D" column indicates it is instantiated by the DUT, -; and a one in the "G" column indicates it is instantiated by the Grand Central -; companion. +; and a Grand Central companion module inside the DUT. Additionally, both the +; DUT and the TestHarness have layerblocks of the Verification color. This is +; summarized by the table below. A one ("1") in the "T" column indicates it is +; instantiated by the TestHarness, a one in the "D" column indicates it is +; instantiated by the DUT, and a one in the "G" column indicates it is +; instantiated by the Grand Central companion. A zero ("0") indicates it is not +; instantiated. An "L" indicates that it is instantiated in a layer in that +; module. ; ; T D G Output ; -------------------- @@ -22,6 +25,9 @@ ; Quz 1 0 1 gct/Quz.sv ; Corge 1 1 0 Corge.sv ; Grault 1 1 1 Grault +; Bazola 0 L 0 testbench/Bazola.sv +; Ztesch L 0 0 testbench/Ztesch.sv +; Thud L L 0 testbench/Thud.sv ; ; CHECK-LABEL: module DUT ; @@ -32,8 +38,11 @@ ; CHECK: FILE "gct{{[/\]}}Quz.sv" ; CHECK: FILE ".{{[/\]}}Corge.sv" ; CHECK: FILE ".{{[/\]}}Grault.sv" +; CHECK: FILE "testbench{{[/\]}}Bazola.sv" +; CHECK: FILE "testbench{{[/\]}}Ztesch.sv" +; CHECK: FILE "testbench{{[/\]}}Thud.sv" -FIRRTL version 3.0.0 +FIRRTL version 4.0.0 circuit TestHarness: %[[ { "class": "sifive.enterprise.grandcentral.ViewAnnotation", @@ -148,8 +157,34 @@ circuit TestHarness: %[[ "target": "~TestHarness|BlackBox_Grault_GCT", "name": "Grault.sv", "text": "module Grault #(parameter X=hello)(output a);\nendmodule" + }, + { + "class": "firrtl.transforms.BlackBoxInlineAnno", + "target": "~TestHarness|BlackBox_Bazola_DUT", + "name": "Bazola.sv", + "text": "module Bazola #(parameter X=hello)(output a);\nendmodule" + }, + { + "class": "firrtl.transforms.BlackBoxInlineAnno", + "target": "~TestHarness|BlackBox_Ztesch_TestHarness", + "name": "Ztesch.sv", + "text": "module Ztesch #(parameter X=hello)(output a);\nendmodule" + }, + { + "class": "firrtl.transforms.BlackBoxInlineAnno", + "target": "~TestHarness|BlackBox_Thud_TestHarness", + "name": "Thud.sv", + "text": "module Thud #(parameter X=hello)(output a);\nendmodule" + }, + { + "class": "firrtl.transforms.BlackBoxInlineAnno", + "target": "~TestHarness|BlackBox_Thud_DUT", + "name": "Thud.sv", + "text": "module Thud #(parameter X=hello)(output a);\nendmodule" } ]] + layer Verification, bind: + extmodule BlackBox_Foo_GCT: output a: UInt<1> defname = Foo @@ -205,7 +240,26 @@ circuit TestHarness: %[[ defname = Grault parameter X = "Grault_GCT" - module TestHarness: + extmodule BlackBox_Bazola_DUT: + output a: UInt<1> + defname = Bazola + parameter X = "Bazola_TestHarness" + + extmodule BlackBox_Ztesch_TestHarness: + output a: UInt<1> + defname = Ztesch + parameter X = "Ztesch_TestHarness" + + extmodule BlackBox_Thud_TestHarness: + output a: UInt<1> + defname = Thud + parameter X = "Thud_TestHarness" + extmodule BlackBox_Thud_DUT: + output a: UInt<1> + defname = Thud + parameter X = "Thud_DUT" + + public module TestHarness: inst dut of DUT @@ -214,6 +268,10 @@ circuit TestHarness: %[[ inst blackBox_Corge_TestHarness of BlackBox_Corge_TestHarness inst blackBox_Grault_TestHarness of BlackBox_Grault_TestHarness + layerblock Verification: + inst blackBox_Ztesch_TestHarness of BlackBox_Ztesch_TestHarness + inst blackBox_Thud_TestHarness of BlackBox_Thud_TestHarness + module DUT: wire a: UInt<1> @@ -226,6 +284,10 @@ circuit TestHarness: %[[ inst blackBox_Corge_DUT of BlackBox_Corge_DUT inst blackBox_Grault_DUT of BlackBox_Grault_DUT + layerblock Verification: + inst blackBox_Bazola_DUT of BlackBox_Bazola_DUT + inst blackBox_Thud_DUT of BlackBox_Thud_DUT + module GrandCentral: inst blackBox_Foo_GCT of BlackBox_Foo_GCT From b11d941e027a7b7edb9ec8ac2928053eacde8d03 Mon Sep 17 00:00:00 2001 From: Schuyler Eldridge Date: Thu, 28 Mar 2024 23:22:32 -0400 Subject: [PATCH 2/2] fixup! [FIRRTL] Treat blackboxes in layers as "testbench" --- lib/Dialect/FIRRTL/Transforms/BlackBoxReader.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/Dialect/FIRRTL/Transforms/BlackBoxReader.cpp b/lib/Dialect/FIRRTL/Transforms/BlackBoxReader.cpp index 75a72de4e4e5..b97892735cf9 100644 --- a/lib/Dialect/FIRRTL/Transforms/BlackBoxReader.cpp +++ b/lib/Dialect/FIRRTL/Transforms/BlackBoxReader.cpp @@ -419,7 +419,8 @@ OutputFileInfo BlackBoxReaderPass::getOutputFile(Operation *origOp, // testbench dir annotation, not have a blackbox target directory annotation // (or one set to the current directory), have a DUT annotation, and the // module needs to be in or under the DUT. - else if (!testBenchDir.empty() && targetDir.equals(".") && dut && !isDut(origOp)) + else if (!testBenchDir.empty() && targetDir.equals(".") && dut && + !isDut(origOp)) outDir = {testBenchDir, Priority::TestBench}; else if (isCover) outDir = {coverDir, Priority::Verification};