Re: optabs: Variable index vec_set
On Sat, Nov 5, 2022 at 12:25 PM Richard Biener
wrote:
>
> On Wed, Nov 2, 2022 at 1:46 PM Uros Bizjak wrote:
> >
> > On Wed, Nov 2, 2022 at 1:45 PM Robin Dapp wrote:
> > >
> > > > IIRC, I was trying to "fix" modeless operand by giving it a mode, but
> > > > since it made no difference for x86, I later dropped the patch.
> > > > However, operand with a known mode is preferred, so if it works for
> > > > you, just include my patch in your submission. My patch is somehow
> > > > trivial if we want operand to have known mode.
> > >
> > > I'd prefer to push it separately as my patch changes several things in
> > > the s390 backend that are kind of unrelated. Is it OK to do an x86
> > > bootstrap and regtest and push it if everything looks good? You can of
> > > course also do it yourself :)
> >
> > It is a middle-end patch, someone will have to approve it.
>
> The patch is OK
Thanks, pushed with the following ChangeLog:
optabs: Use operand[2] mode in can_vec_set_var_idx_p
Use operand[2] mode in can_vec_set_var_idx_p when checking vec_set_optab.
This change allows non-VOID index operand in vec_set_optab.
2022-11-06 Uroš Bizjak
gcc/ChangeLog:
* optabs.cc (can_vec_set_var_idx_p): Use operand[2]
mode when checking vec_set_optab.
Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.
Uros.
diff --git a/gcc/optabs.cc b/gcc/optabs.cc
index c2a6f971d74..9fc9b1fc6e9 100644
--- a/gcc/optabs.cc
+++ b/gcc/optabs.cc
@@ -4344,12 +4344,17 @@ can_vec_set_var_idx_p (machine_mode vec_mode)
return false;
machine_mode inner_mode = GET_MODE_INNER (vec_mode);
+
rtx reg1 = alloca_raw_REG (vec_mode, LAST_VIRTUAL_REGISTER + 1);
rtx reg2 = alloca_raw_REG (inner_mode, LAST_VIRTUAL_REGISTER + 2);
- rtx reg3 = alloca_raw_REG (VOIDmode, LAST_VIRTUAL_REGISTER + 3);
enum insn_code icode = optab_handler (vec_set_optab, vec_mode);
+ const struct insn_data_d *data = _data[icode];
+ machine_mode idx_mode = data->operand[2].mode;
+
+ rtx reg3 = alloca_raw_REG (idx_mode, LAST_VIRTUAL_REGISTER + 3);
+
return icode != CODE_FOR_nothing && insn_operand_matches (icode, 0, reg1)
&& insn_operand_matches (icode, 1, reg2)
&& insn_operand_matches (icode, 2, reg3);
Re: optabs: Variable index vec_set
On Wed, Nov 2, 2022 at 1:46 PM Uros Bizjak wrote: > > On Wed, Nov 2, 2022 at 1:45 PM Robin Dapp wrote: > > > > > IIRC, I was trying to "fix" modeless operand by giving it a mode, but > > > since it made no difference for x86, I later dropped the patch. > > > However, operand with a known mode is preferred, so if it works for > > > you, just include my patch in your submission. My patch is somehow > > > trivial if we want operand to have known mode. > > > > I'd prefer to push it separately as my patch changes several things in > > the s390 backend that are kind of unrelated. Is it OK to do an x86 > > bootstrap and regtest and push it if everything looks good? You can of > > course also do it yourself :) > > It is a middle-end patch, someone will have to approve it. The patch is OK > Uros.
Re: optabs: Variable index vec_set
On Wed, Nov 2, 2022 at 1:45 PM Robin Dapp wrote: > > > IIRC, I was trying to "fix" modeless operand by giving it a mode, but > > since it made no difference for x86, I later dropped the patch. > > However, operand with a known mode is preferred, so if it works for > > you, just include my patch in your submission. My patch is somehow > > trivial if we want operand to have known mode. > > I'd prefer to push it separately as my patch changes several things in > the s390 backend that are kind of unrelated. Is it OK to do an x86 > bootstrap and regtest and push it if everything looks good? You can of > course also do it yourself :) It is a middle-end patch, someone will have to approve it. Uros.
Re: optabs: Variable index vec_set
> IIRC, I was trying to "fix" modeless operand by giving it a mode, but > since it made no difference for x86, I later dropped the patch. > However, operand with a known mode is preferred, so if it works for > you, just include my patch in your submission. My patch is somehow > trivial if we want operand to have known mode. I'd prefer to push it separately as my patch changes several things in the s390 backend that are kind of unrelated. Is it OK to do an x86 bootstrap and regtest and push it if everything looks good? You can of course also do it yourself :) Thanks Robin
Re: optabs: Variable index vec_set
On Wed, Nov 2, 2022 at 1:12 PM Robin Dapp wrote: > > Hi, > > > With the patch my local changes to make better use of vec_set work > > nicely even though I haven't done a full bootstrap yet. Were there > > other issues with the patch or can it still be applied? > > I performed a bootstrap as well as a regtest with -march=z16 on s390. > There is no new fallout. IIRC, I was trying to "fix" modeless operand by giving it a mode, but since it made no difference for x86, I later dropped the patch. However, operand with a known mode is preferred, so if it works for you, just include my patch in your submission. My patch is somehow trivial if we want operand to have known mode. Uros.
Re: optabs: Variable index vec_set
Hi, > With the patch my local changes to make better use of vec_set work > nicely even though I haven't done a full bootstrap yet. Were there > other issues with the patch or can it still be applied? I performed a bootstrap as well as a regtest with -march=z16 on s390. There is no new fallout. Regards Robin
optabs: Variable index vec_set
Hi, I'm looking into vec_set with variable index on s390. Uros posted a patch [1] that did not make it upstream in Nov 2020. It changed the mode of the index operand to whatever the target supports in can_vec_set_var_idx_p. I missed it back then but we indeed do not make proper use of vec_set with an index register. With the patch my local changes to make better use of vec_set work nicely even though I haven't done a full bootstrap yet. Were there other issues with the patch or can it still be applied? Regards Robin [1] https://gcc.gnu.org/pipermail/gcc-patches/2020-November/559213.html
