First draft vector parameters #1245
Conversation
There was a problem hiding this comment.
Good work, and much appreciated!
Unsurprisingly, perhaps, I have a number of comments to hopefully improve the PR.
By the way, I noted some text that may require additional parameters when reviewing the VFREDUSUM parameter:
A different floating-point range and precision may be chosen for the result of each operator. A node where one input is derived only from elements masked-off or beyond the active vector length may either treat that input as the additive identity of the appropriate EEW or simply copy the other input to its output. The rounded result from the root node in the tree is converted (rounded again, using the dynamic rounding mode) to the standard floating-point format indicated by SEW. An implementation is allowed to add an additional additive identity to the final result.
I didn't do an exhaustive search through the chapter.
| For a given supported fractional LMUL setting, implementations must support | ||
| SEW settings between SEW~MIN~ and LMUL * ELEN, inclusive. |
There was a problem hiding this comment.
This would seem to be a normative rule, rather than a parameter. (But don't trust my interpretation! 😉 )
There was a problem hiding this comment.
The issue below discusses this issue. The normative rule above simply states what is required to be implemented. For example, it doesnt state that SEW 64 LMUL 1/2 cannot be supported. Therefore in my mind I see it as up to the designer to allow for this implementation or not. Supporting vs not supporting these cases would produce a different result when executing the same program: that is why it should be a parameter in my mind.
There was a problem hiding this comment.
If SEW=64, then ELEN=64, so LMUL=1/2 must be supported. Per the spec:
For standard vector extensions with ELEN=64, fractional LMULs of 1/2, 1/4, and 1/8 must be supported.
I don't see a needed parameterization here, but maybe I'm missing something.
There was a problem hiding this comment.
I believe this line is refering to the fact that ELEN=64 and a standard extension implies an SEW_MIN of 8, meaning lmul 1/2, 1/4, 1/8 have to be supported LMUL. The line I quoted in the spec states that, (if ELEN =64 and SEW_MIN = 8) that for
SEW=64: fLMUL do not have to be supported
SEW=32: fMUL 1/2 has to be supported
SEW=16: fLMUL 1/2-1/4 have to be supported
SEW=8: fLMUL 1/2-1/8 have to be supported
My impression is this is leaving opportunity for fLMUL of 1/2 to be supported for SEW=64 and such if an ELEN of 128 is added. The spec does not state that one cannot implement fLMUL of 1/2 1/4 1/8 for all SEW, but does not state they have to. This causes different behavior on devices that chose to implement these lines vs not
| type: string | ||
| enum: ["no_change", "custom"] |
There was a problem hiding this comment.
Could use more eyes on this one:
- Is the parameter just about
vfredusumand "NaN" or should "no active elements" be part of the parameter name as well? In other words, is it specific to the case of "no active elements"? - Does it need to be a parameter at all, given the description is in an "informative note" and not in the regular text?
typo Co-authored-by: Paul Clarke <[email protected]> Signed-off-by: jacassidy <[email protected]>
Syntax for parameter names Co-authored-by: Paul Clarke <[email protected]> Signed-off-by: jacassidy <[email protected]>
Typo Co-authored-by: Paul Clarke <[email protected]> Signed-off-by: jacassidy <[email protected]>
punctuation Co-authored-by: Paul Clarke <[email protected]> Signed-off-by: jacassidy <[email protected]>
duplicate Co-authored-by: Paul Clarke <[email protected]> Signed-off-by: jacassidy <[email protected]>
Co-authored-by: Paul Clarke <[email protected]> Signed-off-by: jacassidy <[email protected]>
Co-authored-by: Paul Clarke <[email protected]> Signed-off-by: jacassidy <[email protected]>
Co-authored-by: Paul Clarke <[email protected]> Signed-off-by: jacassidy <[email protected]>
Co-authored-by: Paul Clarke <[email protected]> Signed-off-by: jacassidy <[email protected]>
Co-authored-by: Paul Clarke <[email protected]> Signed-off-by: jacassidy <[email protected]>
spec/std/isa/ext/V.yaml
Outdated
| MSTATUS_VS_EXISTS: | ||
| description: | | ||
| Analogous to the way in which the floating-point unit is handled, the `mstatus.VS` field may exist even if `misa.V` is clear. | ||
| schema: | ||
| type: boolean |
There was a problem hiding this comment.
This is already covered by the MSTATUS_VS_LEGAL_VALUES parameter.
There was a problem hiding this comment.
does this also apply to VSSTATUS_VS_EXISTS, meaning should I instead make a parameter for legal values instead of one for if it exists or not.
better description Co-authored-by: Paul Clarke <[email protected]> Signed-off-by: jacassidy <[email protected]>
|
@ThinkOpenly Is there any other changes I should make? |
Do you have thoughts on my first general comment, well above? |
|
@ThinkOpenly sorry I never addressed your initial comment, I added parameters for it now under "VFREDUSUM." Only a couple comments left! |
3 participants
Trying to pickup on any mentions of "implementation specific" choices that could be made by a designer of the RISC-V Vector Extension. Descriptions are direct quotes from the spec for easy search. Some parameters may be unneeded as they wouldnt affect instruction-level output behavior but I figured including more rather than less could only be benificial. Names are tentative.