Calyx wrapper for Berkeley HardFloat Verilog library

[jiahanxie353]

trafficstars

Implement a wrapper in Calyx for Berkeley HardFloat fNToRecFN (IEEE standard format to HardFloat recoded format) Verilog module (adopted based on PyMTL's corresponding file).

And include a corresponding test case to convert from standard format to recoded format when the input floating-point number is a normal number.

I want to first create a draft PR to make sure that I'm on the right track. And I will include more test cases, such as subnormals, NaNs; and work on other HardFloat modules.

[rachitnigam]

Woo! Awesome! This is exactly what I had in mind as well! Couple of things:

• To get the CI working, you need to open a branch in the repo. I've invited you as a collaborator the repo so you should be able to do this
• We should talk about where this code is going to live exactly. I was imagining keeping this in a separate repo but that would make testing more difficult. We can keep this in the repo under primitives/float?
• We should also extend the Calyx data format to allow users to write down exact floating point values instead of having to work with unsigned representations. This magic happens in json_to_dat.py which uses the numeric_types.py code.

[jiahanxie353]

• To get the CI working, you need to open a branch in the repo. I've invited you as a collaborator the repo so you should be able to do this

Just opened up calyx-float! Though it shows "This branch has not been deployed"?

• We should talk about where this code is going to live exactly. I was imagining keeping this in a separate repo but that would make testing more difficult. We can keep this in the repo under primitives/float?

Agree. Yes primitives/float sounds reasonable.

• We should also extend the Calyx data format to allow users to write down exact floating point values instead of having to work with unsigned representations. This magic happens in json_to_dat.py which uses the numeric_types.py code.

Sounds good! So we start with first extending the existing parser?

[jiahanxie353]

Hi @rachitnigam ! Just want to share an update that the current wrapper can perform floating add/sub and multiplication!

So I think I can move on to extending the user input data type to conduct more thorough tests. Any thoughts or tips on initiating and streamlining this process? Thanks!

[rachitnigam]

User inputs seem like the right next step. Do you want to create a plan to scope out the work in this PR? For example one primitive plus fud extension should be enough

[jiahanxie353]

Do you want to create a plan to scope out the work in this PR? For example one primitive plus fud extension should be enough

Sure!

What does "fud extension" mean here, fud numeric type extension for floating-point values?

And do you agree that Calyx users will only use normal floating-point values, and at most, they will use IEEE format representation, and that they barely want to interact with HardFloat specific formats like "recoded" formats and "raw deconstructions"?

If that's the case, then I think ideally, I'd start with primitive addFN, which does two standard IEEE format floating-point numbers addition (and it doesn't exist yet).

Now I only have primitive fNToRecFN, recFNToFN, and primitive addRecFN to wrap their original HardFloat Verilog modules. So the current addition is executed as a workaround:

1. taking two standard IEEE format floating-point numbers, convert them to HardFloat recoded format;
2. use recoded format addition addRecFN to add two numbers;
3. convert the result back to standard form using recFNToFN.

If we indeed want an addFN Calyx primitive, I can start with first making a separate Verilog file/module called addFN and chain up the above three steps. And finally declare a Calyx primtive to wrap this module by using extern. Then I can extend it in Calyx.

[jiahanxie353]

Do you want to create a plan to scope out the work in this PR?

Hi @rachitnigam , I think I have some idea about how to proceed! Can you check my plan to see if it makes sense?

1. Create a new class called FloatPoint in numeric_types.py and add the corresponding json_to_data transformation.
2. Modify the addFN test case to use floatpoint as the numeric_type and check the correctness.

[rachitnigam]

Hi @jiahanxie353, I'm traveling a lot this month so I'll recommend that you go ahead and try things out instead of waiting on my responses. Folks in the Calyx zulip and slack should also be able to help.

[jiahanxie353]

Sure thing, sounds good!

[jiahanxie353]

Hi @rachitnigam , just finished a milestone that we can support passing floating point values in json and perform two floating-point numbers addition when using fud!

[rachitnigam]

Looks cool! Couple of notes:

1. Seems like all the test inputs are hard-coded. We probably instead want to specifying these as floating point numbers in the fud .data files.
2. The adder is implemented combinationally which will probably not meet timing on real designs. We should make it take some number of cycles. @andrewb1999 any thoughts?

[andrewb1999]

Yeah we definitely want floating point units to be pipelined when used in real designs. I think berkeley hardfloat is all combinational? If that's the case we can maybe add registers at the beginning and/or end and hope that retiming will do a decent job. In Vitis HLS all the floating point units I've seen are between 3 and 5 stage pipelines. I think this is definitely another case where allowing the latency to be parameterized would be very helpful.

[rachitnigam]

@jiahanxie353 I think the move is to pipeline the output value of the adder by 4 cycles for now. Once we have a set of primitives and some designs, we can push them through the FPGA flow and see if designs are correctly being mapped onto DSPs on the FPGA (which are hardened blocks for efficiently performing operations likes add and mult).

Once this change is done, I think you're good to merge! Next step would be to change fud to ingest and output numbers in floating-point representation when the data-format is set to "float" instead of "bitnum".

[sampsyo]

Excellent. I'm late to the party, but I agree with where everything has ended up: namely, @jiahanxie353, your plan enumerated above sounds right to me, and @rachitnigam's suggested next steps (test inputs from data files, and a fixed 4-cycle latency) are also the right things to do here.

To expand in a possibly-unnecessary way about the 4-cycle latency: basically, the idea in Berkeley HardFloat is that all the operations are provided as combinational logic, but realistic designs do not want combinational FP ops. So its intended use case is that people just stick "useless" registers in front of or behind the HardFloat operation, and the EDA toolchain does its best to perform retiming (magically transforming the combinational-plus-registers description into a proper, balanced pipeline). So one option would be to expose these as comb primitives and call it good for now, but we would eventually want to add sequential versions that would be more practical. Another option is to go ahead and pick a latency (4 is fine!); then we'll be slightly closer to having a practical library from the start, in addition to a functionally correct one.

[sampsyo]

One extra category of logistical discussion:

• Do we need to think about licensing here? Looks like HardFloat uses an MIT (ish?) license, which means we need to preserve the copyright notice and attribution somewhere.
• Will it be a pain to check in a snapshot of the HardFloat Verilog code? This could make it semi-annoying to adapt to upstream changes. An alternative would be to provide a script to download the most recent version instead.

[jiahanxie353]

Thanks for all the advice folks! Got a 4-cycle adder by using dummy registers.

As per

One extra category of logistical discussion:

• Do we need to think about licensing here? Looks like HardFloat uses an MIT (ish?) license, which means we need to preserve the copyright notice and attribution somewhere.

I believe you are right. That's why I kept

This Verilog include file is part of the Berkeley HardFloat IEEE Floating-Point Arithmetic Package, Release 1, by John R. Hauser. Copyright 2019 The Regents of the University of California. All rights reserved.

in the source files like here for attribution. Is there anything else I need to do for copyright?

• Will it be a pain to check in a snapshot of the HardFloat Verilog code? This could make it semi-annoying to adapt to upstream changes. An alternative would be to provide a script to download the most recent version instead.

It'll probably be annoying to adapt to upstream changes in the future if we just use a static snapshot. The issue is that HardFloat is implemented in Chisel in their repo. Implementing a translator/transpiler from Chisel to Verilog could (?) solve the concern but I doubt it's worth the effort. And since there's no direct Verilog implementation, I believe people just use the Verilog files obtained from a zip file in this page.

[rachitnigam]

This all sounds good!

One extremely tangential note: "transpiler" is not a meaningful term: https://rachit.pl/post/transpiler/

Chisel already has a compiler to Verilog.

[sampsyo]

Got it; thanks, @jiahanxie353! Here's my suggestion about how to deal with including external source code like this. I think we have two options:

• Check the HardFloat Verilog released code into our repo (this is called "vendoring"). With this route, it would be best to preserve the structure of the released code as identically as possible: that is, maybe we literally have a subdirectory inside primitives/float called HardFloat-1 (the name of the zip archive), and within that we preserve COPYING.txt, README.txt, and the source subdirectory. We could decide to exclude some files, but we would not modify any files—they would be preserved byte-for-byte as in the zip folder, not renamed/relocated or anything. This has the advantage of (a) making the licensing issues completely clear, as we are preserving literally the license from the original distribution, and (b) making it extremely clear what one would have to do if HardFloat were ever to be updated (just expand the zip into the right location; no further changes necessary).
• Fetch the zip on demand. That is, we check in a little get_hardfloat.sh script that essentially does curl -LO http://www.jhauser.us/arithmetic/HardFloat-1.zip followed by an unzip. People would have to run this before they can use the float library. While this is obviously annoying to require, the advantages are that (a) it is even clearer what is our code and what is external code, (b) grepping in our repo will not be contaminated by matches in unrelated Verilog, and (c) it will be literally impossible for anyone to modify the HardFloat sources in the future, making them diverge from upstream.

Anyway, I think either could work! Just wanted to lay out the options.

[jiahanxie353]

Got it, I'll take the second fetch-and-unzip approach!

While working on it, got a decision to make regarding (1) changing the HardFloat source code after fetching it; (2) modifying verilator and icarus in fud.

1. Option 1 changing the HardFloat source code is inspired by pymtl3-hardfloat, where it adds an includeFile.v and add an extra include line in the common included file in the source files, recFNToFN, so that every file in the source code can now cross reference. This is straightforward but since this changes the source file (though only after unzipping), will this mess up with licensing issues?

2. The second approach involves changing fud stages. Take compile through icarus as the example, we need to change cmd and compile_with_iverilog. This might be more programmable but I'm not sure if it's worth it change it just for the sake of float? And I know we are undergoing changes to fud2.

Do you have any preference over 2 options?

[sampsyo]

I see! Good question… do you think you could elaborate a tiny bit more on the problem that each of these changes would solve? Here is my guess, but I am low-confidence:

The way (most? all?) Calyx primitives currently work is that the Verilog code for each primitive gets inlined into the generated Verilog code. The result is an entirely self-contained Verilog program, consisting of both primitive code and Calyx-compiled code, that we can compile all by itself. HardFloat presents another level of logistical complexity because, of course, we need to include a bunch of external files. The current solution is that our addFN.sv primitive implementation includes the relevant HardFloat source files: https://github.com/calyxir/calyx/blob/ebd20693fe1e46284e122a4c5d8b0de29305d298/primitives/float/addFN.sv#L4-L6

This in turn requires the actual HardFloat source files to use the same path prefix to import one another: https://github.com/calyxir/calyx/blob/ebd20693fe1e46284e122a4c5d8b0de29305d298/primitives/float/source/fNToRecFN.v#L4

Obviously, the "pristine" HardFloat source files don't know about the path primitives/float, so that line isn't present as written there. In the actual fNToRecFN.sv source file as released, it's just:

`include "HardFloat_localFuncs.vi"

…and that doesn't necessary work by default to include the file relative to the containing source file, for some reason. I don't understand Verilog!!! It seems like it should just work! It is annoying that it doesn't!!!!!!!

• Actually modify the HardFloat source files to hack them to use our blessed import paths.
• Require all consumers of the generated Verilog (not just Icarus and Verilator—anyone who uses the generated Verilog to do anything, such as other EDA tools!) to support file-relative includes. For example, Verilator seems to have a --relative-includes option that enables this kind of include. And Icarus seems to have -grelative-include that does the same thing. So I believe @jiahanxie353's proposal is to add this flag to those simulators? But presumably every other user of the generated Verilog would need the same kind of option?

Is that a correct summary of the situation? If so, just adding the flag seems way easier to me… clearly, there is more we should do around the problem of Calyx now not emitting self-contained Verilog (it now has includes that depend on other files). But just adding the flag seems like a simple way to deal with it… if we think that other Verilog-consuming tools (Vivado? OpenROAD?) can also support this kind of include. Do you think that's true?

[jiahanxie353]

Yes, that's pretty much the situation and grelative-include can solve most of them. And please let me add a few points that makes this tricky.

Firstly, the "pristine" HardFloat has this wacky kind of import: https://github.com/pymtl/pymtl3-hardfloat/blob/aea69e416e079178df4bb5697795ef653f55df58/HardFloat/source/mulRecFN.v#L39-L40

Even if they intended to mean relative import, they should at least write:

`include "RISCV/HardFloat_specialize.vi"

But anyhow, this is not difficult to solve. And my proposed solution is to append -I calyx/primitives/float/HardFloat-1/source/RISCV to icarus.py file when compiling to Verilog, which can be done for example, add an include_path argument to https://github.com/calyxir/calyx/blob/4bc300787371b155b1aad45e7b465485ead0de7c/fud/icarus/icarus.py#L109

A trickier thing seems unsolvable by using grelative-include and -I flag. Take the example you mentioned: https://github.com/calyxir/calyx/blob/ebd20693fe1e46284e122a4c5d8b0de29305d298/primitives/float/source/fNToRecFN.v#L4

This line is manually inserted by me, and there's no such import in the "pristine" HardFloat.

And in fact, there's no single file that has "include "HardFloat_primitives.vi" despite the fact HardFloat_primitives.vi contains essential helper modules like countLeadingZeros, which is being used throughout, such as in fNToRecFN.v and addRecFN.v (so this means these files are just using modules that are from nowhere!)

As a workaround, pymtl manually created this includeFile.v, which includes HardFloat_primitives.v. And then they insert it, by changing the source code, to recFNToFN.v because they found out that this is a commonly shared file across the source code.

[rachitnigam]

I haven't kept track of this thread that well but one thing that would be good to have is being able to have a default flow that generates exactly one Verilog file like we do today. We can emit some information information the user that they've requested that we link with HardFloat and therefore are implicitly agreeing to its LICENSE. We also should be carefully when we provide a new release on crates.io if we place Hardfloat in calyx-stdlib.

[jiahanxie353]

Hi folks, just finished my current solution:

1. I got rid of all the HardFloat source code to keep Calyx repo clean and wrote a script to let users fetch the HardFloat zip file, as in this commit.
2. Added an includeFile.sv to include all files that are direct children of the parent source code directory, i.e., HardFloat/source, and I adjusted my addFN.sv and mulFN.sv include path accordingly, as in this commit.
3. To address including files that are not direct children of the HardFloat parent directory, I resorted to -grelative-include and -I flag (reference), and added an -i flag for fud so that users can type the library files that they want to include.

I'm pretty confident that this solution does not involve changing HardFloat source code at all so the license concern won't be a concern. But I'm not sure if adding an additional -i flag in fud adheres to our fud design principle. Please let me know what you think, thanks!

[jiahanxie353]

We also should be carefully when we provide a new release on crates.io if we place Hardfloat in calyx-stdlib.

I'm not entirely clear about how creating a release work and what's the consequence if HardFloat is in/or not in calyx-stdlib. Can you elaborate on that? @rachitnigam Thanks!

[rachitnigam]

Don't worry about that part for now. Let's get this stuff working with real benchmarks and then we can worry about creating a release.

[sampsyo]

Thanks for all the details, @jiahanxie353! About the general problem of including the right files in the HardFloat release:

• Regarding the import of HardFloat_specialize.v[i]: Looks like the HardFloat manual covers this, in Section 8. Namely, what it describes that the IEEE standard leaves some things implementation-defined, and various ISAs fill in those blanks in different ways. That's why there is no "default" option, and there's a way to select the behavior you want through including the files from one of the subdirectories: RISC-V, ARM, or x86. You've chosen RISC-V, which I suppose is fine for our purposes.
• About the "missing imports": That's pretty weird. It's not clear what the "intended" use case is for these files, and the manual doesn't seem to cover it. Maybe they are supposed to be concatenated into one big file? Anyway, the PyMTL approach you mention is also one solution…

For @rachitnigam:

one thing that would be good to have is being able to have a default flow that generates exactly one Verilog file like we do today.

This seems pretty hard to achieve in a literal sense with HardFloat, because the necessary functionality is spread across many separate Verilog files. So we would need some way to concatenate all the files into one big blob if we want this to produce a single, standalone Verilog file. I guess it seems to me like we might want to leave that challenge for another day?

[rachitnigam]

@jiahanxie353 anything blocking the merge?

[jiahanxie353]

@jiahanxie353 anything blocking the merge?

No in terms of pushed changes, they work well with runt testing.

Then I've been working on pushing them through FPGA and actually simulate them, but got blocked by Verilog include thing when generating the IP in Vivado. So I'm reinvestigating the solution to deal with including external libraries like HardFloat. And I'm trying to use morty as it seems suitable to generate a single "fatty" Verilog file. I've been exploring different ways locally and will push my changes once found a concrete solution!

[rachitnigam]

Got it! I would recommend merging small PRs quickly instead of waiting for things to be complete. One of the very common things we do in the repo is merge an initial PR and then open a "tracker" issue to track the bigger picture. For example, see https://github.com/calyxir/calyx/issues/1913

A benefit of this approach is that tracker issue can be used as a the place to provide updates and discuss any roadblocks in the bigger picture.

[jiahanxie353]

Got it! I would recommend merging small PRs quickly instead of waiting for things to be complete. One of the very common things we do in the repo is merge an initial PR and then open a "tracker" issue to track the bigger picture. For example, see #1913

A benefit of this approach is that tracker issue can be used as a the place to provide updates and discuss any roadblocks in the bigger picture.

Sure! Now it's ready for review!

[rachitnigam]

@jiahanxie353 please answer and fix the questions that @sampsyo's asked in the previous review first