Skip to content
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

Black Boxes for toEnum / fromEnum #2066

Merged
merged 2 commits into from Feb 2, 2022
Merged

Black Boxes for toEnum / fromEnum #2066

merged 2 commits into from Feb 2, 2022

Conversation

alex-mckenna
Copy link
Contributor

@alex-mckenna alex-mckenna commented Jan 31, 2022

This PR introduces black boxes for toEnum / fromEnum on the numeric types in Clash.Sized.Internal, suppressing the seemingly spurious integer black box warnings previously seen in #1918. As an added bonus, a commit is included which propagates the -Werror flag from GHC to Clash, allowing primitive warnings to be thrown as ClashException when desired.

Fixes #2046

Still TODO:

  • Write a changelog entry (see changelog/README.md)
  • Check copyright notices are up to date in edited files
  • Add a test based on toEnum, the current test uses fromEnum only

When running GHC with -Werror, that option is now propagated to
the ClashOpts. This means that warnings on primitive instantiation
will be promoted to errors. This is necessary to test #2046.
@alex-mckenna alex-mckenna changed the title Issue 2046 Black Boxes for toEnum / fromEnum Jan 31, 2022
@alex-mckenna alex-mckenna force-pushed the issue-2046 branch 5 times, most recently from 0a9a4da to 12217de Compare February 1, 2022 16:33
@alex-mckenna alex-mckenna marked this pull request as ready for review February 1, 2022 16:33
Copy link
Member

@martijnbastiaan martijnbastiaan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but I'd to see some HDL simulation tests using the blackboxes.

@@ -0,0 +1 @@
CHANGED: Clash now respects the -Werror option from GHC
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

@alex-mckenna alex-mckenna force-pushed the issue-2046 branch 2 times, most recently from 06d032a to f0ef6cc Compare February 2, 2022 09:20
@alex-mckenna
Copy link
Contributor Author

@martijnbastiaan I added a testBench to round-trip values through the black boxes. I think this should be enough

Comment on lines 9 to 11
, (BitVector 1, BitVector 2)
)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you test Bit too, one of the functions changed for that

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It broke CI, good luck.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

            iverilog (make testBench):      FAIL (0.04s)
              Program /usr/bin/iverilog failed with error-code 2.
              
              Full invocation:
              
                /usr/bin/iverilog -I T2046BType.o -I T2046B.dyn_hi -I T2046B.hi -I T2046B.testBench -I T2046BType.dyn_o -I T2046BType.dyn_hi -I T2046B.topEntity -I T2046B.dyn_o -I T2046BType.hi -I T2046B.o -g2 -s testBench -o testBench.exe /tmp/clash-test-b57a279493374e3d/T2046B.testBench/testBench.v /tmp/clash-test-b57a279493374e3d/T2046B.topEntity/topEntity.v
                
              
              Stderr was:
              
                /tmp/clash-test-b57a279493374e3d/T2046B.topEntity/topEntity.v:98: syntax error
                /tmp/clash-test-b57a279493374e3d/T2046B.topEntity/topEntity.v:98: error: syntax error in continuous assignment
              
              Stdout was:

To prevent Clash from warning about integer primitives in HDL
that originate from uses of fromEnum / toEnum in user code, the
functions are now primitives for the sized types in Clash.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clash over-eagerly reports "dubious primitive instantiations"
3 participants