On Mon, Jan 25, 2021 at 04:51:43PM -0600, Segher Boessenkool wrote:
> Hi!
>
> On Thu, Oct 08, 2020 at 09:27:57AM +1030, Alan Modra wrote:
> > * config/rs6000/rs6000.c (rotate_insert_cost): New function.
> > (rs6000_rtx_costs): Cost IOR.
> >
> > diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> > index 383d2901c9f..15a806fe307 100644
> > --- a/gcc/config/rs6000/rs6000.c
> > +++ b/gcc/config/rs6000/rs6000.c
> > @@ -21206,6 +21206,91 @@ rs6000_cannot_copy_insn_p (rtx_insn *insn)
> > && get_attr_cannot_copy (insn);
> > }
> >
> > +/* Handle rtx_costs for scalar integer rotate and insert insns. */
>
> You need to document here what the return value means, and what the
> preconditions for "left" (and "right") are.
Done, and I moved the preconditions on "left" into the new function.
> > +static bool
> > +rotate_insert_cost (rtx left, rtx right, machine_mode mode, bool speed,
> > + int *total)
> > +{
> > + if (GET_CODE (right) == AND
>
> ... because you never check the CODE of "left".
>
> > + && CONST_INT_P (XEXP (right, 1))
> > + && UINTVAL (XEXP (left, 1)) + UINTVAL (XEXP (right, 1)) + 1 == 0)
>
> HOST_WIDE_INT is always exactly 64 bits now, so you could do "== -1".
Yes, but this is exactly the way the expression occurs in rotl*_insert*
instruction patterns. I think it's better to keep them the same.
> > + {
> > + rtx leftop = XEXP (left, 0);
> > + rtx rightop = XEXP (right, 0);
> > +
> > + /* rotlsi3_insert_5. */
> > + if (REG_P (leftop)
> > + && REG_P (rightop)
> > + && mode == SImode
> > + && UINTVAL (XEXP (left, 1)) != 0
> > + && UINTVAL (XEXP (right, 1)) != 0
> > + && rs6000_is_valid_mask (XEXP (left, 1), NULL, NULL, mode))
> > + return true;
>
> Empty line after return please.
Done, here and elsewhere.
> > + /* rotldi3_insert_6. */
> > + if (REG_P (leftop)
> > + && REG_P (rightop)
> > + && mode == DImode
> > + && exact_log2 (-UINTVAL (XEXP (left, 1))) > 0)
> > + return true;
> > + /* rotldi3_insert_7. */
> > + if (REG_P (leftop)
> > + && REG_P (rightop)
> > + && mode == DImode
> > + && exact_log2 (-UINTVAL (XEXP (right, 1))) > 0)
> > + return true;
>
> Those could just use rs6000_is_valid_mask as well?
This is taken straight from rotldi3_insert_7, so it really ought to
stay that way.
>
> Please wait this until stage 1. Sorry.
OK, I'll leave all the rs6000_rtx_costs changes until then.
--
Alan Modra
Australia Development Lab, IBM
