Re: RISC-V: Support XTheadVector extensions

["Christoph Müllner" <christoph.muellner@vrull.eu>]

Hi Juzhe,

Sorry for the late reply, but I was not on CC, so I missed this email.

On Fri, Nov 17, 2023 at 2:41 PM juzhe.zhong@rivai.ai
<juzhe.zhong@rivai.ai> wrote:
>
> Ok. I just read the theadvector extension.
>
> https://github.com/T-head-Semi/thead-extension-spec/blob/master/xtheadvector.adoc
>
> Theadvector is not custom extension. Just a uarch to disable some of the RVV1.0 extension
> Theadvector can be considered as subextension of 'V' extension with disabling some of the
> instructions and adding some new thead vector target load/store (This is another story).
>
> So, for disabling the instruction that theadvector doesn't support.
> You don't need to touch such many codes.
>
> Here is a much simpler approach to do (I think it's definitely working):
> 1. Don't change any codes in vector.md and keep GCC generates ASM with "th." prefix.
> 2. Add !TARGET_THEADVECTOR into vector-iterator.md to disable the mode you don't want.
> For example , theadvector doesn't support fractional vector.
>
> Then it's pretty simple:
>
> RVVMF2SI "TARGET_VECTOR && !TARGET_THEADVECTOR".
>
> 3. Remove all the tests you add in this patch.
> 4. You can add theadvector specific load/store for example, th.vlb instructions they are allowed.
> 5. Modify binutils, and make th.vmulh.vv as the pseudo instruction of vmulh.vv
> 6. So with compile option "-S", you will still see ASM as  "vmulh.vv". but with objdump, you will see th.vmulh.vv.

Yes, all these points sound reasonable, to minimize the patchset size.
I believe in point 1 you meant "without th. prefix".

I've added Jin Ma (who is the main author of the Binutils patchset) so
he is also aware
of the proposal to use pseudo instructions to avoid duplication in Binutils.

Thank you very much!
Christoph


>
> After this change, you can send V2, then I can continue to review on GCC-15.
>
> Thanks.
>
> ________________________________
> juzhe.zhong@rivai.ai
>
>
> From: juzhe.zhong@rivai.ai
> Date: 2023-11-17 19:39
> To: gcc-patches
> CC: kito.cheng; kito.cheng; cooper.joshua; Robin Dapp; jeffreyalaw
> Subject: RISC-V: Support XTheadVector extensions
> 90% theadvector extension reusing current RVV 1.0 instructions patterns:
> Just change ASM, For example:
>
> @@ -2923,7 +2923,7 @@ (define_insn "*pred_mulh<v_su><mode>_scalar"
>       (match_operand:VFULLI_D 3 "register_operand"  "vr,vr, vr, vr")] VMULH)
>    (match_operand:VFULLI_D 2 "vector_merge_operand" "vu, 0, vu,  0")))]
>    "TARGET_VECTOR"
> -  "vmulh<v_su>.vx\t%0,%3,%z4%p1"
> +  "%^vmulh<v_su>.vx\t%0,%3,%z4%p1"
>    [(set_attr "type" "vimul")
>     (set_attr "mode" "<MODE>")])
>
> +  if (letter == '^')
> +    {
> +      if (TARGET_XTHEADVECTOR)
> + fputs ("th.", file);
> +      return;
> +    }
>
>
> For almost all patterns, you just simply append "th." in the ASM prefix.
> like change "vmulh.vv" -> "th.vmulh.vv"
>
> Almost all theadvector instructions are not new features,  all same as RVV1.0.
> Why do you invent the such ISA doesn't include any features that RVV1.0 doesn't satisfy ?
>
> I am not explicitly object this patch. But I should know the reason.
>
> Btw, stage 1 will close soon.  So I will review this patch on GCC-15 as long as all other RISC-V maintainers agree.
>
>
> ________________________________
> juzhe.zhong@rivai.ai