my slang version is slang version 3.0.149+32dc81bc. Another things is that the slang v3.0+ uses C++20 so that I set(CMAKE_CXX_STANDARD 20) in ImportVerilog/CMakeLists.txt in order to build.
Could you please explain the location -DDEBUG in Slang and -D_DEBUG in Circt?
I’m having trouble understanding why this different causing a memory leak.
Hello,
I found that the issue lies in the initialization of the driver. In ImportVerilog.cpp:139 the driver is initialized as follow.
slang::driver::Driver driver;
The driver contains a diagEngine, which is also initialized with the sourceManager during driver initialization.
Driver::Driver() : diagEngine(sourceManager) {
diagClient = std::make_shared<TextDiagnosticClient>();
diagEngine.addClient(diagClient);
}
diagEngine constructor
explicit DiagnosticEngine(const SourceManager& sourceManager);
in slang/diagnostic/DiagnosticEngine.h:267, diagEngine contains a vector: clients, and the addClients function pushes values into this vector.
// A list of all registered clients that receive issued diagnostics.
std::vector<std::shared_ptr<DiagnosticClient>> clients;
When debugging with GDB, I entered the constructor of the Driver. Before executing the initialization list, the diagEngine.clients variable was not initialized, and its internal value was chaotic as follow.
After the initialization, if we print diagEngine.clients, we can see
indicating that it has been initialized. Continuing to execute diagEngine.addClient(diagClient); successfully pushes a value into the vector. However, when we return to importVerilog and print driver.diagEngine.clients, we can see that the internal state of the vector has reverted back to an uninitialized state, causing the subsequent addClient to fail.
I ran again to check the addresses of the driver and its related members before initialization.
driver : 0x7fffffffc290
driver.diagEngine : 0x7fffffffc4a8
driver.diagEngine.clients : 0x7fffffffc5d8
After initialization in Driver.cpp.
driver : 0x7fffffffc290
diagEngine : 0x7fffffffc498
diagEngine.clients : 0x7fffffffc5a8
this : 0x7fffffffc290
After returning to importVerilog.cpp, I checked the addresses of the relevant members.
driver : 0x7fffffffc290
driver.diagEngine : 0x7fffffffc4a8
driver.diagEngine.clients : 0x7fffffffc5d8
It indicates that when initializing diagEngine in the Driver, it was constructed at the wrong memory, and this address was lost when returning to importVerilog.
I ran gdb in slang, there is no problem as above, the clients keeps normal.
my environment:
GNU gdb (Ubuntu 12.1-0ubuntu1~22.04) 12.1
OS: Ubuntu22.04
slang: 3.0.149 (which use c++20)
I have to assume you’re using different build configurations for slang vs llvm. The DEBUG flag is just one example that can affect ABI, though it’s the one people usually run into first. Generally that’s why you want to either build them in the same cmake invocation (by pulling in the sources via add_subdirectory) or by using the cmake package via find_package.
Just found this thread, a couple notes of interest:
-
slang at HEAD can now be built with exceptions and RTTI disabled, which might save you some build headaches here.
-
I’ve been thinking more about pulling MLIR into slang for a middle layer IR, to assist in things like dataflow analysis. It seems logical to me that CIRCT would be better off consuming such output IR from slang instead of trying to pull the whole slang frontend into tree and building off the AST, but maybe you guys have other thoughts (and it’s worth pointing out I haven’t done any work towards this yet either).
1 Like
Great to hear that exceptions and RTTI can be disabled now!
I also love the idea of having slang frontend dialect in MLIR that CIRCT could work with. That would be fantastic!
I have solved these question and pushed my change to GitHub - albertethon/circt at slang-fronted
I’m using external package slang3.0 in circt, it’s ture that different build configurations are used for slang vs circt. I check these configurations by VERBOSE=1 and found some macro flags are missed in circt. Now there’s still some question while I using FetchContent way in circt.
@fabianschuiki Hello, sir. When I use scf::IfOp, an error is thrown:
Building op scf.if but it isn't known in this MLIRContext: the dialect may not be loaded or this operation hasn't been added by the dialect.
Then I link and register it.
But it still exists.
Hi @hailongSun
This is very strange. I would expect the registry.insert<scf::SCFDialect>() to fix the issue. Can you check whether the registerDialects function actually gets called (by inserting a print there, something like llvm::errs() << "hello\n";)?
If this actually calls a transformation pass in the background, you might have to add SCF as a dependentDialect in the *.td file where the pass is defined.
2 Likes
Hey @fabianschuiki
As you said, “hello\n” was printed in the terminal. But there is no defined ImportVerilogPass in the Passes.td file, I have to implement it and the constructor field is a trouble for me. I need to spend time defining it(void runOnOperaion() override{}).
By the way, I committed some code in the PR(#24). Could you review it at your convenience?
It’s okay that there is no ImportVerilogPass, since this is just an MLIR translation, which has its own registration mechanism. No need to add one 
2 Likes
Sorry to be a late reply. I went back to my hometown last week for a funeral.
If so, scf::IfOp can not be used, we should implement moore::IfOp. I implemented it, but it is a little rough.
Sorry to hear that, my condolences 
!
The scf::IfOp should definitely work. No need to implement moore::IfOp. This will turn out to be some hiccup somewhere, or some missed dialect/op registration. Do you have a branch somewhere on GitHub where you have your current work?
2 Likes
Thanks for your condolences!
Health is first, I wish you and your family good health.
OK, I will attempt to fix it. And replace moore::IfOp with it.
Yeah, I have two branches (slang-frontend and framework), the first is used to test, and the second is used to pull requests.
By the way, to what extent can the ImportVerilogPass need to be supported to merge on the main branch?
I solved it. The scf::IfOp can work, but I should modify my previous code that handles the if-else statement with the moore::IfOp. Now I need to solve the problem brought by the scf::YieldOp.
1 Like
That looks great! Congratulations 
1 Like
At your slang-frontend branch, I created a pull request, which includes some commits for slang statements and expressions conversion. Actually, I should create multiple PRs to classify these commits. But I think taking the code into one PR may be more readable. If needed, I can divide that into multiple PRs after you review it and give me some guiding suggestions.