On Sat, Apr 25, 2015 at 12:26:16AM +0100, Kugan wrote:
>
> Thanks for the review. I have updated the patch based on the comments
> with some other minor changes. Bootstrapped and regression tested on
> aarch64-none-linux-gnu with no-new regressions. Is this OK for trunk?
>
>
> Thanks,
> Kugan
>
>
> gcc/ChangeLog:
>
> 2015-04-24 Kugan Vivekanandarajah <kug...@linaro.org>
> Jim Wilson <jim.wil...@linaro.org>
>
> * config/arm/aarch-common-protos.h (struct mem_cost_table): Added
> new fields loadv and storev.
> * config/aarch64/aarch64-cost-tables.h (thunderx_extra_costs):
> Initialize loadv and storev.
> * config/arm/aarch-cost-tables.h (generic_extra_costs): Likewise.
> (cortexa53_extra_costs): Likewise.
> (cortexa57_extra_costs): Likewise.
> (xgene1_extra_costs): Likewise.
> * config/aarch64/aarch64.c (aarch64_rtx_costs): Update vector
> rtx_costs.
Hi Kugan,
Just a few syle comments, regarding the placements of comments in single-line
if statements. I know the current code does not neccesarily always follow the
comments below, I'll write a patch cleaning that up at some point when I'm back
at my desk.
Thanks,
James
> @@ -5667,6 +5668,14 @@ aarch64_rtx_costs (rtx x, int code, int outer
> ATTRIBUTE_UNUSED,
> case NEG:
> op0 = XEXP (x, 0);
>
> + if (VECTOR_MODE_P (mode))
> + {
> + if (speed)
> + /* FNEG. */
> + *cost += extra_cost->vect.alu;
> + return false;
> + }
> +
> if (GET_MODE_CLASS (GET_MODE (x)) == MODE_INT)
> {
> if (GET_RTX_CLASS (GET_CODE (op0)) == RTX_COMPARE
Personally, I find commented if statements without braces hard to
quickly parse. Something like this is much faster for me:
if (speed)
{
/* FNEG. */
*cost += extra_cost->vect.alu;
}
> @@ -5844,7 +5872,10 @@ cost_minus:
>
> if (speed)
> {
> - if (GET_MODE_CLASS (mode) == MODE_INT)
> + if (VECTOR_MODE_P (mode))
> + /* Vector SUB. */
> + *cost += extra_cost->vect.alu;
> + else if (GET_MODE_CLASS (mode) == MODE_INT)
> /* SUB(S). */
> *cost += extra_cost->alu.arith;
> else if (GET_MODE_CLASS (mode) == MODE_FLOAT)
As above.
> @@ -5888,7 +5919,6 @@ cost_plus:
> {
> if (speed)
> *cost += extra_cost->alu.arith_shift;
> -
> *cost += rtx_cost (XEXP (XEXP (op0, 0), 0),
> (enum rtx_code) GET_CODE (op0),
> 0, speed);
Drop this whitespace change.
> @@ -5913,7 +5943,10 @@ cost_plus:
>
> if (speed)
> {
> - if (GET_MODE_CLASS (mode) == MODE_INT)
> + if (VECTOR_MODE_P (mode))
> + /* Vector ADD. */
> + *cost += extra_cost->vect.alu;
> + else if (GET_MODE_CLASS (mode) == MODE_INT)
> /* ADD. */
> *cost += extra_cost->alu.arith;
> else if (GET_MODE_CLASS (mode) == MODE_FLOAT)
As above.
> @@ -6013,10 +6061,15 @@ cost_plus:
> return false;
>
> case NOT:
> - /* MVN. */
> if (speed)
> - *cost += extra_cost->alu.logical;
> -
> + {
> + /* Vector NOT. */
> + if (VECTOR_MODE_P (mode))
> + *cost += extra_cost->vect.alu;
> + /* MVN. */
> + else
> + *cost += extra_cost->alu.logical;
> + }
> /* The logical instruction could have the shifted register form,
> but the cost is the same if the shift is processed as a separate
> instruction, so we don't bother with it here. */
As above.
> @@ -6055,10 +6108,15 @@ cost_plus:
> return true;
> }
>
> - /* UXTB/UXTH. */
> if (speed)
> - *cost += extra_cost->alu.extend;
> -
> + {
> + if (VECTOR_MODE_P (mode))
> + /* UMOV. */
> + *cost += extra_cost->vect.alu;
> + else
> + /* UXTB/UXTH. */
> + *cost += extra_cost->alu.extend;
> + }
> return false;
>
> ca§se SIGN_EXTEND:
And again :)
> @@ -6087,10 +6150,16 @@ cost_plus:
>
> if (CONST_INT_P (op1))
> {
> - /* LSL (immediate), UBMF, UBFIZ and friends. These are all
> - aliases. */
> if (speed)
> - *cost += extra_cost->alu.shift;
> + {
> + /* Vector shift (immediate). */
> + if (VECTOR_MODE_P (mode))
> + *cost += extra_cost->vect.alu;
> + /* LSL (immediate), UBMF, UBFIZ and friends. These are all
> + aliases. */
> + else
> + *cost += extra_cost->alu.shift;
> + }
>
> /* We can incorporate zero/sign extend for free. */
> if (GET_CODE (op0) == ZERO_EXTEND
Again, the comment here makes it very difficult to spot the form of
the if/else statement.
> @@ -6102,10 +6171,15 @@ cost_plus:
> }
> else
> {
> - /* LSLV. */
> if (speed)
> - *cost += extra_cost->alu.shift_reg;
> -
> + {
> + /* Vector shift (register). */
> + if (VECTOR_MODE_P (mode))
> + *cost += extra_cost->vect.alu;
> + /* LSLV. */
> + else
> + *cost += extra_cost->alu.shift_reg;
> + }
> return false; /* All arguments need to be in registers. */
> }
>
Likewise here.
