Enhance zve

[chihminchao]

There are some part of V that don't be covered by zveXXX configuration. The patch set wants to handle

1. add a zve state to present the vector capability is enabled by V or Zve
2. handle vector widening and narrow conversion with difference floating width
3. handle vector index load with zve64 and rv32
4. handle widening integer arithmetic which may not be supported on zve64

[nadime15]

Two more things:

Line 101 in (isa_parser.cc) needs to be changed too:
case 'v': vlen = 128; elen = 64; zvf = true; zvd = true; zve = true;

Note: If Zve32x is supported then Zvkb or Zvbb provide support for EEW of 8, 16, and 32. If Zve64x is
supported then Zvkb or Zvbb also add support for EEW 64

All of the other Vector Crypto Extensions depend on

I have not checked whether Spike currently respects this, but the vector crypto extensions can be enabled via Zve32x and Zve64x.

[nadime15]

By the way, we had a similar PR recently for the Sail RISC-V Model (see riscv/sail-riscv#1170) that addressed all these cases, including Vector Crypto. It might be helpful, at least for cross-checking.

[chihminchao]

Two more things:

Line 101 in (isa_parser.cc) needs to be changed too: case 'v': vlen = 128; elen = 64; zvf = true; zvd = true; zve = true;

Note: If Zve32x is supported then Zvkb or Zvbb provide support for EEW of 8, 16, and 32. If Zve64x is
supported then Zvkb or Zvbb also add support for EEW 64
All of the other Vector Crypto Extensions depend on

I have not checked whether Spike currently respects this, but the vector crypto extensions can be enabled via Zve32x and Zve64x.

1. I don't think it is necessary. zve state is used to tell vector unit is enabled by 'v' or zveXXX.
   `v' includes zve64d but a small part of functionality are only available for 'v'
2. The description in zveXXX doesn't mention anything about vector crypto extension. I think zveXXX doesn't affect vector crypto behavior.

[nadime15]

I don't think it is necessary. zve state is used to tell vector unit is enabled by 'v' or zveXXX.
`v' includes zve64d but a small part of functionality are only available for 'v'

The v extension is a superset of Zve64d. If Zve64d sets zve = true, then v should too. Currently, enabling only v leaves zve as false but zvf and zvd true, which seems inconsistent.

The description in zveXXX doesn't mention anything about vector crypto extension. I think zveXXX doesn't affect vector crypto behavior.

Yes, you are right. It is not mentioned in the Zve section, but it appears in the vector crypto section.

All of the other Vector Crypto Extensions depend on Zve32x

[aswaterman]

The v extension is a superset of Zve64d. If Zve64d sets zve = true, then v should too. Currently, enabling only v leaves zve as false but zvf and zvd true, which seems inconsistent.

Since this is an internal implementation detail, we don't really need to adhere to the principle that V is a superset of Zve, as long as users of Spike can't tell the difference.

With that said, I agree this approach makes the code a bit confusing. @chihminchao did you consider doing this the other way around: like, in vmulh, do require(p->extension_enabled('V') || P.VU.vsew < e64); instead of checking get_zve() and ELEN? Then we could make V a proper superset of Zve, like @nadime15 suggested.

[chihminchao]

The v extension is a superset of Zve64d. If Zve64d sets zve = true, then v should too. Currently, enabling only v leaves zve as false but zvf and zvd true, which seems inconsistent.

Since this is an internal implementation detail, we don't really need to adhere to the principle that V is a superset of Zve, as long as users of Spike can't tell the difference.

With that said, I agree this approach makes the code a bit confusing. @chihminchao did you consider doing this the other way around: like, in vmulh, do require(p->extension_enabled('V') || P.VU.vsew < e64); instead of checking get_zve() and ELEN? Then we could make V a proper superset of Zve, like @nadime15 suggested.

It is reasonable and let me check again

[aswaterman]

Cool, ping me when it's time to re-review.

[chihminchao]

The zve state has been removed by suggestion.

[aswaterman]

Yeah, I think this version is cleaner. Thanks @chihminchao (and @nadime15 for making the suggestion).