> On Feb 26, 2015, at 10:36 AM, Xingxing Pan <xxing...@marvell.com> wrote:
>
> On 02/25/2015 09:32 PM, Xingxing Pan wrote:
>> Hi,
>>
>> This patch merges pipeline description for marvell-whitney to latest
>> code base.
>> Is it OK for trunk?
>>
> Refactor the commit message.
>
...
> Add pipeline description for marvell-whitney.
>
> 2015-02-26 Xingxing Pan <xxing...@marvell.com>
>
> * config/arm/arm-cores.def: Add new core marvell-whitney.
> * config/arm/arm-protos.h:
> (marvell_whitney_vector_mode_qi): Declare.
> (marvell_whitney_inner_shift): Ditto.
> * config/arm/arm-tables.opt: Regenerated.
> * config/arm/arm-tune.md: Regenerated.
> * config/arm/arm.c (arm_marvell_whitney_tune): New structure.
> (arm_issue_rate): Add marvell_whitney.
> (marvell_whitney_vector_mode_qi): New function.
> (marvell_whitney_inner_shift): Ditto.
> * config/arm/arm.md: Include marvell-whitney.md.
> (generic_sched): Add marvell_whitney.
> (generic_vfp): Ditto.
> * config/arm/bpabi.h (BE8_LINK_SPEC): Add marvell-whitney.
> * config/arm/t-arm (MD_INCLUDES): Add marvell-whitney.md.
> * config/arm/marvell-whitney.md: New file.
> * doc/invoke.texi: Document marvell-whitney.
>
...
> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> index 7bf5b4d..e68287f 100644
> --- a/gcc/config/arm/arm.c
> +++ b/gcc/config/arm/arm.c
> @@ -2000,6 +2000,25 @@ const struct tune_params arm_cortex_a9_tune =
> ARM_SCHED_AUTOPREF_OFF /* Sched L2 autopref. */
> };
>
> +const struct tune_params arm_marvell_whitney_tune =
> +{
> + arm_9e_rtx_costs,
> + &cortexa9_extra_costs,
> + cortex_a9_sched_adjust_cost,
> + 1, /* Constant limit. */
> + 5, /* Max cond insns. */
> + ARM_PREFETCH_BENEFICIAL(4,32,32),
> + false, /* Prefer constant pool. */
> + arm_default_branch_cost,
> + false, /* Prefer LDRD/STRD. */
> + {true, true}, /* Prefer non short
> circuit. */
> + &arm_default_vec_cost, /* Vectorizer costs. */
> + false, /* Prefer Neon for 64-bits
> bitops. */
> + false, false, /* Prefer 32-bit encodings.
> */
> + false, /* Prefer Neon for stringops.
> */
> + 8 /* Maximum insns to inline
> memset. */
> +};
> +
You need to bootstrap GCC with your patch applied. As it is now, the above
tune table is missing at least two entries (one for insn fusing, and one for
sched L2 autoprefetcher). It should cause a bootstrap fail.
> const struct tune_params arm_cortex_a12_tune =
> {
> arm_9e_rtx_costs,
> @@ -11717,6 +11736,51 @@ fa726te_sched_adjust_cost (rtx_insn *insn, rtx link,
> rtx_insn *dep, int * cost)
> return true;
> }
>
> +/* Return true if vector element size is byte. */
> +bool
> +marvell_whitney_vector_mode_qi (rtx_insn *insn)
> +{
> + machine_mode mode;
> +
> + if (GET_CODE (PATTERN (insn)) == SET)
> + {
> + mode = GET_MODE (SET_DEST (PATTERN (insn)));
> + if (VECTOR_MODE_P (mode) && GET_MODE_INNER (mode) == QImode)
> + return true;
> + }
> +
> + return false;
> +}
It is preferable to avoid using predicates in DFA definitions. Predicates
prevent optimizations of the DFA, which causes them to be bigger and slower.
This predicate can be easily replaced by an attribute (set_attr
"dest_vect_mode" "<mode>") on affected instructions (the proper name for the
attribute needs some thinking though).
That said, I don't a have strong opinion on this case. It may be a worthwhile
tradeoff to use the predicate to avoid adding new attribute to a dozen of
instructions. If you / other reviewers prefer to keep the predicate -- please
add a comment noting that it is used by the DFA.
> +
> +/* Return true if a non-shift insn has a shift operand. */
> +bool
> +marvell_whitney_inner_shift (rtx_insn *insn)
> +{
> + rtx pat = PATTERN (insn);
> +
> + if (GET_CODE (pat) != SET)
> + return false;
> +
> + /* Is not a shift insn. */
> + rtx rvalue = SET_SRC (pat);
> + RTX_CODE code = GET_CODE (rvalue);
> + if (code == ASHIFT || code == ASHIFTRT
> + || code == LSHIFTRT || code == ROTATERT)
> + return false;
> +
> + subrtx_iterator::array_type array;
> + FOR_EACH_SUBRTX (iter, array, rvalue, ALL)
> + {
> + /* Has shift operation. */
> + RTX_CODE code = GET_CODE (*iter);
> + if (code == ASHIFT || code == ASHIFTRT
> + || code == LSHIFTRT || code == ROTATERT)
> + return true;
> + }
> +
> + return false;
> +}
Same comment as above.
Otherwise looks good.
Thanks!
--
Maxim Kuvyrkov
www.linaro.org
