Skip to content

[HW] Floating-point compare instructions#75

Merged
Navaneeth-KunhiPurayil merged 8 commits intopulp-platform:mainfrom
FondazioneChipsIT:rgiunti/pulp/fp-cond-instr
Apr 14, 2026
Merged

[HW] Floating-point compare instructions#75
Navaneeth-KunhiPurayil merged 8 commits intopulp-platform:mainfrom
FondazioneChipsIT:rgiunti/pulp/fp-cond-instr

Conversation

@rgiunti
Copy link
Copy Markdown
Contributor

@rgiunti rgiunti commented Apr 2, 2026

PR purpose

This PR implements the HW support for the vmfne, vmfeq, vmfle, vmflt, vmfge, vmfgt instructions.

Main HW changes

  • Add decode entries for vmfne.vv/vmfne.vf using RUP as rounding mode encoding to signal boolean result inversion to the VFU
  • Add VFCMP result compaction logic in align_result: extract bit 0 from each SEW-wide FPU result slot and pack them contiguously into the vrf data output
  • Add selective byte-enable for VFCMP in operand_req_proc: only enable vl/8 bytes rounded to higher integer to preserve tail-undisturbed bits in the destination register, with saturation check

DiyouS
DiyouS previously approved these changes Apr 3, 2026
Copy link
Copy Markdown
Collaborator

@DiyouS DiyouS left a comment

Choose a reason for hiding this comment

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

Looks good to me and seems only with tiny hardware overhead. Let's wait CI passed and merge it
Out of curiosity, have you tried backend for the changes?

Comment thread hw/ip/spatz/src/spatz_vfu.sv
Comment thread hw/ip/spatz/src/spatz_vfu.sv Outdated
@rgiunti
Copy link
Copy Markdown
Contributor Author

rgiunti commented Apr 3, 2026

Looks good to me and seems only with tiny hardware overhead. Let's wait CI passed and merge it Out of curiosity, have you tried backend for the changes?

Yes I performed a synthesis in gf22. From the area report it seems that the VFU area is almost the same. Regarding the CI failure I can't check the ETH pipeline but I'm pretty sure that it fails for a test of vmfne. It is for a known bug in the pulp0.1.3 version that is used by Spatz. I opened a PR in the cvfpu pulp repo for fixing it.

@Navaneeth-KunhiPurayil
Copy link
Copy Markdown
Contributor

Looks good to me and seems only with tiny hardware overhead. Let's wait CI passed and merge it Out of curiosity, have you tried backend for the changes?

Yes I performed a synthesis in gf22. From the area report it seems that the VFU area is almost the same. Regarding the CI failure I can't check the ETH pipeline but I'm pretty sure that it fails for a test of vmfne. It is for a known bug in the pulp0.1.3 version that is used by Spatz. I opened a PR in the cvfpu pulp repo for fixing it.

Yes, this is the only test that is failing, we could comment the specific test case that is failing for now and then enable it again later after bumping fpnew.

@rgiunti rgiunti force-pushed the rgiunti/pulp/fp-cond-instr branch from f00dcc9 to bc32504 Compare April 7, 2026 12:51
@rgiunti rgiunti force-pushed the rgiunti/pulp/fp-cond-instr branch from bc32504 to fe0a6e5 Compare April 9, 2026 17:45
Copy link
Copy Markdown
Contributor

@Navaneeth-KunhiPurayil Navaneeth-KunhiPurayil left a comment

Choose a reason for hiding this comment

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

Hi @rgiunti @DiyouS ,
I believe we can merge this PR, since it looks good and can pass CI.

However, there is an edge case, for largest vector length i.e. LMUL=8 and SEW=8-bit which requires 2 x 256-bit word writes to VRF. But since currently write enable depends on the result_tag.last only one write is written.

I suggest we merge this PR, since anyways there needs to be a FPU bump to fix a failing test case these can be done together later.

What do you think?

Comment thread hw/ip/spatz/src/spatz_vfu.sv Outdated
Copy link
Copy Markdown
Collaborator

@DiyouS DiyouS left a comment

Choose a reason for hiding this comment

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

Can we remove these trailing space found by the robot before merging?

@DiyouS
Copy link
Copy Markdown
Collaborator

DiyouS commented Apr 14, 2026

GitLab CI failed due to some internal network issues. I re-ran it on GitLab and it passed without problem. I think we can merge it @Navaneeth-KunhiPurayil

@Navaneeth-KunhiPurayil
Copy link
Copy Markdown
Contributor

Navaneeth-KunhiPurayil commented Apr 14, 2026

The CI passes, @rgiunti let's merge

@Navaneeth-KunhiPurayil Navaneeth-KunhiPurayil merged commit 6850385 into pulp-platform:main Apr 14, 2026
6 of 7 checks passed
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.

3 participants