> > Since the parameter vl of XTheadVector does not support immediate numbers,
> > we need
> > to put it in the register in advance. That generates the initial code
> > correctly.
> >
> > PR 116593
> >
> > gcc/ChangeLog:
> >
> > * config/riscv/riscv-vector-builtins.cc
> > (function_expander::add_input_operand):
> > Put immediate for vl to GPR for XTheadVector.
>
> Generally both patches look reasonable to me and the change is less invasive
> than going via spec_restriction.
>
> How was this tested? The Rivos CI has already picked it up but please still
> always specify. Thanks.
I'm not sure what you mean, but I'll do my best to understand.
If you're referring to how to test the logic of this code to ensure it works
as intended, it can be quite challenging to explain. Testing this particular
code is not straightforward. Even without any specific processing, the
predicate
"vector_length_operand" still constrains the XTheadVector, and the immediate
value for VL will be stored in the register. In this sense, the code itself
may seem redundant.
However, I submitted the patch to generate the correct pattern in generate_insn
rather than relying solely on "vector_length_operand" to ensure correctness.
This approach aims to make the code more robust and reliable.
As for how to test, I don't have a good idea yet. At present, it is just a
simple test to store the immediate number of Vl in the register. Obviously,
this coincides with the function of "vector_length_operand.
In fact, I was also hesitating whether we needed this patch, and finally
submitted it in order to see your opinion.
> > + /* Since the parameter vl of XTheadVector does not support
> > + immediate numbers, we need to put it in the register
> > + in advance. */
> > + if (TARGET_XTHEADVECTOR
> > + && CONST_INT_P (x)
> > + && base->apply_vl_p ()
> > + && argno == (unsigned) (call_expr_nargs (exp) - 1)
> > + && x != CONST0_RTX (GET_MODE (x)))
> > + {
> > + x = force_reg (word_mode, x);
> > + add_input_operand (TYPE_MODE (TREE_TYPE (arg)), x);
> > + }
> > + else
> > + add_input_operand (TYPE_MODE (TREE_TYPE (arg)), x);
> > }
>
> To me it feels as if this would better fit one level higher rather than
> checking ARGNO and apply_vl_p here. Is that somehow too cumbersome?
While it is generally preferable to handle operations at the upper layer,
involving
multiple upper layers could complicate maintenance. Additionally, the
processing for
VL has already been largely completed by ARGNO and apply_vl_p. Therefore, I
prefer
not to introduce further changes at the upper level, even though this means
that every
call to add_input_operand will be checked, potentially leading to a slight
decrease in
compilation efficiency.
> > +# GCC testsuite that uses the `dg.exp' driver.
> > +
> > +# Test the front-end for C++.
> > +# We don't need to test back-end code-gen in RV32 system for C++
> > +# Because it is already tested in C.
> > +# Exit immediately if this isn't a RISC-V target.
> > +if ![istarget riscv*-*-*] then {
> > + return
> > +}
>
> Might want to adjust those comments slightly ;)
>
> Apologies, I mixed up patches. Still it's not really a front-end test IMHO
> but just a minor nit anyway.
Here I simply copied it form rvv.exp and didn't notice the comments, my fault.
I will modify, thanks. :)
Regards,
Jin
>
> --
> Regards
> Robin
