Skip to content

Conversation

stevobailey
Copy link

Allows e.g. an FP16-only FPU to support INT8 and INT32:

localparam fpu_features_t CUSTOM = '{
    Width:         32,
    EnableVectors: 1'b0,
    EnableNanBox:  1'b1,
    FpFmtMask:     5'b00100,
    IntFmtMask:    4'b1110
  };

Allows e.g. an FP16-only FPU to support INT8 and INT32:
``` 
localparam fpu_features_t CUSTOM = '{
    Width:         32,
    EnableVectors: 1'b0,
    EnableNanBox:  1'b1,
    FpFmtMask:     5'b00100,
    IntFmtMask:    4'b1110
  };
```
@zarubaf zarubaf requested a review from stmach June 13, 2024 09:12
@domenicw
Copy link

We see the same issue in our project. The proposed fix would resolve the problem. If someone could look at this PR, this would be much appreciated.

Copy link
Collaborator

@stmach stmach left a comment

Choose a reason for hiding this comment

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

This change probably breaks the (super messy) way multiple vectorial lanes are generated, as now all lanes will always have all INT formats enabled (and will also have their widths).
The multiformat slice generation more or less works as follows for multiple vector lanes (if memory serves right):

  |        64      | 1x 64bit slice for all casts
+          |   32  | 1x 32bit slice for all casts <=32 (enables 2x 32bit casts in SIMD by using the 64+32 slices)
+          | 16| 16| 2x 16bit slice for all casts <=16 (enables 4x 16bit casts in SIMD by using the 64+32+2x16 slices)
+          |8|8|8|8| 4x 8bit slice for all casts <= 8 (enables 8x 8bit casts in SIMD by using all 8 slices)

The width of the available slices was until now defined by the float formats, which works fine for cases where there are more float formats than int formats (e.g. RV32D) but doesn't work when there are int widths that don't match a float format (e.g. RV64F or the one you list).

With this change, all the vectorial lanes would be as wide as the widest int (and technically would be able to cast to/from that format). In the best case, this just wastes resources by generating too many wide CONV units, in the worst case the current mess of a control logic around these slices stops working properly.

Please, ensure the current change is still functional in vectorial configurations and consider adapting or fixing the format selection for the vector lanes in fpnew_pkg::get_conv_lane_int_formats and friends instead of just force-enabling all int formats for all lanes.

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