Fix: Handle underflow properly at subnormal/normal boundary

[IhsaneTahir]

This PR offers a patch for T-Head openc910 DivSqrt unit instantiated in THMULTI.

It fixes the handling of the underflow exception in the case where the result of the division, as though the exponent range were unbounded, lies strictly between ±b^emin. Even if the rounded result is exactly ±b^emin.

It resolves issue #155

[MikeOpenHWGroup]

Hi @IhsaneTahir. Typically we do not include patch files to PR as they are redundant once the merge is done. Is there a reason the patch file is needed? If not, please update this PR to remove it. Thanks.

[IhsaneTahir]

Hi @MikeOpenHWGroup, thanks for pointing this out.

From my understanding, the vendor/openc910 directory is maintained through ./util/vendor.py, so local changes usually need to be captured as patches to avoid being lost when someone runs --update. In that sense, I thought the patch file was needed to keep this fix reproducible across future vendor updates. I also noticed that other bug fixes to this IP in the repo have been handled this way.

That said, if the preferred workflow is to maintain a fork of openc910 and vendor from there instead, I’m happy to adjust and I will remove the file.

[MikeOpenHWGroup]

Hmmm. That workflow is not documented herein (at least, I didn't find it), and it is not the way most other OpenHW are maintained. Having said that, if that what most downstream users expect, we shouldn't change the flow without notifying everyone. Let me get back to you...

[IhsaneTahir]

Hi @MikeOpenHWGroup, I would like to re-open this conversion to get this PR merged. I did a deep dive to understand the vendor.py script used to integrate the openc910 DivSqrt unit. Based on the util/README.md, the tool used comes from the OpenTitan project. Step 2b details the steps to follow to make modifications to an imported IP and the use of the patch files. It seems that @davideschiavone used it in the past for the cv32e40p, maybe he can provide more insight.

[MikeOpenHWGroup]

I would like to re-open this conversion to get this PR merged.

Thanks @IhsaneTahir, yes it is time that we get moving on this. Thanks for "waking it up".

I did a deep dive to understand the vendor.py script used to integrate the openc910 DivSqrt unit.

Fair enough, if you are comfortable with it, I'll approve it, but I would like to fully document this workflow in this repo.

Based on the util/README.md, the tool used comes from the OpenTitan project. Step 2b details the steps to follow

Hmmm. I don't see anything like "Step 2b" in those docs. I'll open an Issue.

[MikeOpenHWGroup]

Approved. I will defer to @ricted98 for the final approval and merge.

[IhsaneTahir]

Here is the section I was refering to in the OpenTitan documentation.

I fully agree that this process should be documented in the CVFPU repository.

[ricted98]

Approved. I will defer to @ricted98 for the final approval and merge.

Thanks for the vote of confidence, but @rgiunti is the FPU-expert Riccardo you're looking for! 😁