Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Remove use of always_comb that Icarus does not support #2266

Closed
wants to merge 2 commits into from

Conversation

KarlJoad
Copy link

Attempt to close #2253. Still a WiP.

@KarlJoad
Copy link
Author

@rachitnigam, I just tried this change on the unsigned-dot-product example in interp/, and It seems to have simulated fine?

@rachitnigam
Copy link
Contributor

Was it previously erroring out?

@KarlJoad
Copy link
Author

I just checked on my checkout of main (which is on commit f4e80ec). It looks like Icarus executes correctly and produces a waveform. So no, it looks like Icarus was not erroring out. I'm just worried about the warning message Icarus prints.

$ ./target/debug/fud2 ./interp/tests/complex/unsigned-dot-product.futil --from calyx -o test.vcd --through icarus -s sim.data=./interp/tests/complex/unsigned-dot-product.futil.data
[WARN  calyx_frontend::attribute] The attribute @static is deprecated and will be ignored by the compiler.
[WARN  calyx_frontend::attribute] The attribute @static is deprecated and will be ignored by the compiler.
[WARN  calyx_frontend::attribute] The attribute @static is deprecated and will be ignored by the compiler.
[WARN  calyx_frontend::attribute] The attribute @static is deprecated and will be ignored by the compiler.
verilog-noverify.sv:149: sorry: constant selects in always_* processes are not currently supported (all bits will be included).

[nix-shell:~/Repos/calyx] 09:57:03 $ ll test.vcd
-rw-r--r-- 1 karljoad users 46415 Sep 30 09:57 test.vcd

@rachitnigam
Copy link
Contributor

Okay, then let's trigger the remaining tests and see if there are any failures. You need to mark the PR ready for review when you're ready.

@rachitnigam rachitnigam marked this pull request as ready for review October 6, 2024 21:53
@rachitnigam
Copy link
Contributor

@ekiwi wanted another pair of eyes on this in case you think there could be any problems changing the always_comb to always @*. I'm going to fix the failing tests and get this merge ready. If you think it's good, let's merge.

@rachitnigam
Copy link
Contributor

Closing in favor of #2330.

ekiwi pushed a commit that referenced this pull request Nov 4, 2024
Changes from #2266 since author has not responded. Fixes #2253.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor std_fp_div_pipe primitive to avoid Verilog features not supported by Icarus
3 participants