On Fri, Apr 17, 2015 at 12:19:14PM +0100, Kugan wrote:
> >> My point is that adding your patch while keeping the logic at the top
> >> which claims to catch ALL vector operations makes for less readable
> >> code.
> >>
> >> At the very least you'll need to update this comment:
> >>
> >> /* TODO: The cost infrastructure currently does not handle
> >> vector operations. Assume that all vector operations
> >> are equally expensive. */
> >>
> >> to make it clear that this doesn't catch vector set operations.
> >>
> >> But fixing the comment doesn't improve the messy code so I'd certainly
> >> prefer to see one of the other approaches which have been discussed.
> >
> > I see your point. Let me work on this based on your suggestions above.
>
> Hi James,
>
> Here is an attempt along this line. Is this what you have in mind?
> Trying to keep functionality as before so that we can tune the
> parameters later. Not fully tested yet.
Hi Kugan,
Sorry to have dropped out of the thread for a while, I'm currently
travelling in the US.
This is along the lines of what I had in mind, thanks for digging through
and doing it. It needs a little polishing, just neaten up the rough edges
of comments and where they sit next to the new if conditionals, and of course,
testing, and I have a few comments below.
Thanks,
James
> diff --git a/gcc/config/aarch64/aarch64-cost-tables.h
> b/gcc/config/aarch64/aarch64-cost-tables.h
> index ae2b547..ed9432e 100644
> --- a/gcc/config/aarch64/aarch64-cost-tables.h
> +++ b/gcc/config/aarch64/aarch64-cost-tables.h
> @@ -121,7 +121,9 @@ const struct cpu_cost_table thunderx_extra_costs =
> },
> /* Vector */
> {
> - COSTS_N_INSNS (1) /* Alu. */
> + COSTS_N_INSNS (1), /* Alu. */
> + COSTS_N_INSNS (1), /* Load. */
> + COSTS_N_INSNS (1) /* Store. */
> }
> };
Can you push the Load/Stores in to the LD/ST section above and give
them a name like loadv/storev.
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index cba3c1a..c2d4a53 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
<snip>
> @@ -5570,6 +5569,7 @@ aarch64_rtx_costs (rtx x, int code, int outer
> ATTRIBUTE_UNUSED,
> && (GET_MODE_BITSIZE (GET_MODE (XEXP (op1, 0)))
> >= INTVAL (XEXP (op0, 1))))
> op1 = XEXP (op1, 0);
> + gcc_assert (!VECTOR_MODE_P (mode));
As Kyrill asked, please drop this.
