-
Notifications
You must be signed in to change notification settings - Fork 470
[FIRRTLToHW] Remove the SYNTHESIS guard in printf/flush lowering path #10526
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
1299751
e7c2c5b
4367640
e760eda
f587125
97326ff
92cd9c2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -62,10 +62,17 @@ void sv::emitFileDescriptorRuntime(Operation *fileScopeOp, | |
| auto getterSymName = ::getFileDescriptorGetterSymName(builder.getContext()); | ||
| auto fragmentSymName = | ||
| ::getFileDescriptorFragmentSymName(builder.getContext()); | ||
| auto macroSymName = builder.getStringAttr(kFileDescriptorMacroName); | ||
| auto *fileScopeBlock = &fileScopeOp->getRegion(0).front(); | ||
| bool fileDescriptorMacroNeedsCreation = false; | ||
| auto macroSymName = | ||
| sv::lookupOrGenerateMacroSymName(fileScopeBlock, kFileDescriptorMacroName, | ||
| fileDescriptorMacroNeedsCreation); | ||
| bool synthesisMacroNeedsCreation = false; | ||
| auto synthesisSymName = sv::lookupOrGenerateMacroSymName( | ||
| fileScopeBlock, "SYNTHESIS", synthesisMacroNeedsCreation); | ||
| SymbolTable symbolTable(fileScopeOp); | ||
|
|
||
| auto emitGuard = [&](StringRef guard, llvm::function_ref<void(void)> body) { | ||
| auto emitGuard = [&](StringAttr guard, llvm::function_ref<void(void)> body) { | ||
| sv::IfDefOp::create( | ||
| builder, guard, [] {}, body); | ||
| }; | ||
|
|
@@ -95,15 +102,26 @@ void sv::emitFileDescriptorRuntime(Operation *fileScopeOp, | |
| symbolTable.insert(func); | ||
| } | ||
|
|
||
| if (!symbolTable.lookup(macroSymName)) | ||
| symbolTable.insert(sv::MacroDeclOp::create(builder, macroSymName)); | ||
| if (synthesisMacroNeedsCreation) | ||
| symbolTable.insert(sv::MacroDeclOp::create( | ||
| builder, builder.getLoc(), synthesisSymName, /*args=*/ArrayAttr{}, | ||
| /*verilogName=*/synthesisSymName.getValue() == "SYNTHESIS" | ||
| ? StringAttr{} | ||
| : builder.getStringAttr("SYNTHESIS"))); | ||
|
|
||
| if (fileDescriptorMacroNeedsCreation) | ||
| symbolTable.insert(sv::MacroDeclOp::create( | ||
| builder, builder.getLoc(), macroSymName, /*args=*/ArrayAttr{}, | ||
| /*verilogName=*/macroSymName.getValue() == kFileDescriptorMacroName | ||
| ? StringAttr{} | ||
| : builder.getStringAttr(kFileDescriptorMacroName))); | ||
|
|
||
| if (symbolTable.lookup(fragmentSymName)) | ||
| return; | ||
|
|
||
| auto fragment = emit::FragmentOp::create(builder, fragmentSymName, [&] { | ||
| emitGuard("SYNTHESIS", [&]() { | ||
| emitGuard(kFileDescriptorMacroName, [&]() { | ||
| emitGuard(synthesisSymName, [&]() { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can't we remove this?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would be more cautious about removing this. We could simply remove the synthesis guard for print operations, mainly because Chisel places these operations in a separate layer, which are then output to separate files. These separate files themselves have macro guard, and also it's easy to disable it by not including these separate files in the synthesis. As far as I know, there doesn't seem to be a similar mechanism for this part; it outputs separately to each file used, unlike print operations which are separated from the design code. We might be able to improve it further, but I think this might require another evaluation. |
||
| emitGuard(macroSymName, [&]() { | ||
| sv::VerbatimOp::create(builder, R"(// CIRCT Logging Library | ||
| package __circt_lib_logging; | ||
| class FileDescriptor; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,7 @@ | |
| #include "circt/Dialect/HW/ModuleImplementation.h" | ||
| #include "circt/Dialect/SV/SVAttributes.h" | ||
| #include "circt/Support/CustomDirectiveImpl.h" | ||
| #include "circt/Support/Namespace.h" | ||
| #include "circt/Support/ProceduralRegionTrait.h" | ||
| #include "mlir/IR/Builders.h" | ||
| #include "mlir/IR/BuiltinTypes.h" | ||
|
|
@@ -56,6 +57,30 @@ bool sv::isExpression(Operation *op) { | |
| MacroRefExprOp, MacroRefExprSEOp>(op); | ||
| } | ||
|
|
||
| StringAttr sv::lookupOrGenerateMacroSymName(Block *symbolTableBlock, | ||
| StringRef verilogName, | ||
| bool &created) { | ||
| for (auto decl : symbolTableBlock->getOps<sv::MacroDeclOp>()) { | ||
| if (decl.getMacroIdentifier() == verilogName) { | ||
| created = false; | ||
| return decl.getSymNameAttr(); | ||
| } | ||
| } | ||
|
|
||
| Namespace ns; | ||
| auto symbolNameId = | ||
| StringAttr::get(symbolTableBlock->getParentOp()->getContext(), | ||
| SymbolTable::getSymbolAttrName()); | ||
| for (auto &op : *symbolTableBlock) { | ||
| if (auto symbol = op.getAttrOfType<StringAttr>(symbolNameId)) | ||
| ns.add(symbol.getValue()); | ||
| } | ||
|
|
||
| created = true; | ||
| return StringAttr::get(symbolTableBlock->getParentOp()->getContext(), | ||
| ns.newName(verilogName)); | ||
| } | ||
|
Comment on lines
+70
to
+82
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I understand it existed before, but could you consider using SymbolTable::insert instead (as SymbolTable is MLIR proper)?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I may have misunderstood. I think |
||
|
|
||
| /// Returns the operation registered with the given symbol name with the regions | ||
| /// of 'symbolTableOp'. recurse through nested regions which don't contain the | ||
| /// symboltable trait. Returns nullptr if no valid symbol was found. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,5 @@ | ||
| add_subdirectory(Utils) | ||
|
|
||
| if(CIRCT_SLANG_FRONTEND_ENABLED) | ||
| add_subdirectory(ImportVerilog) | ||
| endif() |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| add_circt_unittest(CIRCTConversionUtilsTests | ||
| SVLoweringUtilsTest.cpp | ||
| ) | ||
|
|
||
| target_link_libraries(CIRCTConversionUtilsTests | ||
| PRIVATE | ||
| CIRCTEmit | ||
| CIRCTHW | ||
| CIRCTSV | ||
| CIRCTSVLoweringUtils | ||
| ) |
Uh oh!
There was an error while loading. Please reload this page.