circt
circt copied to clipboard
llvm
[FIRTOOL] Fallback to outDir for empty chiselInterfaceOutDir
When the user does not explicitly specify the output directory for the generated chisel interface via --chisel-interface-out-dir, firtool will use the output directory as the fallback.
This provides the default option for generated files to all be put in the directory specified by -o.
--chisel-interface-out-dir is good, but if we don't want separate argument for the chisel interface file, it should be safe to just let the -o override it.
Also, without --split-verilog and --chisel-interface-out-dir, the interface scala will be concatenated to the generated verilog. We should avoid this (and there're more cases, such as the blackbox filelist, and I propose to fix them as well in the next steps?)
Also, without
--split-verilog
Split verilog should be considered the only sane output for actual use. Single file output is really only useful for writing tests, it isn't likely even correct enough for injestion by any tool.
Is chisel-interface-out-dir even useful? Should it just be a fixed subdirectory relative to the output? There is a tendency to add too many output options.
Is chisel-interface-out-dir even useful? Should it just be a fixed subdirectory relative to the output? There is a tendency to add too many output options.
I agree there are too many output options. This chisel-interface-out-dir seems to be useless for me.
The only reason why we sometimes need a separate directory is that different files (for design, verification, synthesis flow, etc) should be put in separate directory. However, I still don't think firtool should handle this. These should be handled by the scripts in the project's development environment, instead of the code compilation tool.
I'm not a CIRCT maintainer, and thus I'm just expressing my opinion here. Not sure about how the CIRCT developers think about these.
The problem seems to be in firtool.cpp:88. The emptiness of the option should not choose between llvm::outs and a directory based emission.
The problem seems to be in firtool.cpp:88. The emptiness of the option should not choose between llvm::outs and a directory based emission.
I'm considering them simply as different API meanings for getChiselInterfaceOutputDirectory. I'm good if you would like to choose the other way to fixing this. Happy to see this bug fixed.
Feel free to close this PR once the bug is fixed. Thanks for your patience and efforts.