Open AyaElAkhras opened 3 months ago
Thanks for the report. I was able to replicate the problem on the HEAD of main
using the hw.mlir
file your provided.
After investigating the issue, it looks like the Chisel LSQ component cannot represent LSQs that have 0 load ports ("numLoadPorts": 0
in the JSON configuration file). In particular, the component attempts to compute the base-2 logarithm of the number of load ports; when the latter is 0, it understandably fails and generates this error.
I tried to fix this quickly but unfortunately failed to do so (I fixed the error at the top-level but things didn't work in nested components either) due to my limited knowledge of Chisel and of the LSQ architecture. It would be great if someone more knowledgeable than me on these topics could try to tackle this. I suspect this is not too difficult of a problem to solve when one knows what they are doing.
Thanks for checking. Now I see that the LSQ code is the same as that used in the old Dynamatic but this error was never there in the old Dynamatic. Could the problem be in the generation of the JSON configuration file? I'm suggesting so because all of the input C++ codes that trigger this problem contain load operations so they should result in the addition of load ports to the LSQ.
Could the problem be in the generation of the JSON configuration file? I'm suggesting so because all of the input C++ codes that trigger this problem contain load operations so they should result in the addition of load ports to the LSQ.
If it is it is not showcased by the example you provide. The handshake.mlir
IR (that your pass spits out supposedly) does not contain any load port. So the LSQ generator is correctly instructed to have 0 load ports according to your circuit.
I tried running one example on the old and new Dynamatic and found out that the generated json files are different. Specifically, the "numLoadPorts" is 2 in the file generated by the old flow and is 0 in the file generated by the new flow. I'm attaching the two json files for reference. new.json old.json
I'm also attaching the C++ example file and the generated MLIR files, in case they are needed. example.zip mlir_files.zip
I just checked with the C example you provide and the HEAD of main
. While there are loads in the source code these are optimized away by MLIR---one can see by looking at the code that they are indeed not necessary to guarantee correct execution---therefore the LSQ is correctly configured to be generated with 0 load ports.
The problem does not happen with legacy Dynamatic because LLVM does not optimize away these loads (I have seen other similar examples where MLIR can better optimize the source code than LLVM). I guess this situation never presents itself in any of our other integration tests, so nobody ever noticed that the Chisel does not work when 0 load ports are requested (which I think is still a sane use case of the LSQ).
I see, then it is truly a limitation in the Chisel code.
Benchmarks failed to compile due to this issue (in #120 ):
covariance
,
covariance_float
,
Sometimes, we get the following error (attached screenshot) that seems to be triggered from the lsq-generator. The error appears when we use the write-hdl --experimental. I'm attaching the mlir files (as zip files because mlir files seem to not be supported) of one example that triggers this error. hw.zip handshake_export.zip handshake.zip