This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [Patch][Aarch64] Implement Aarch64 SIMD ABI and aarch64_vector_pcs attribute
- From: Segher Boessenkool <segher at kernel dot crashing dot org>
- To: Steve Ellcey <sellcey at cavium dot com>
- Cc: Kyrill Tkachov <kyrylo dot tkachov at foss dot arm dot com>, gcc-patches <gcc-patches at gcc dot gnu dot org>, Wilco Dijkstra <Wilco dot Dijkstra at arm dot com>, Richard Sandiford <Richard dot Sandiford at arm dot com>, Richard Earnshaw <Richard dot Earnshaw at arm dot com>, James Greenhalgh <James dot Greenhalgh at arm dot com>, Marcus Shawcroft <Marcus dot Shawcroft at arm dot com>
- Date: Tue, 7 Aug 2018 12:15:09 -0500
- Subject: Re: [Patch][Aarch64] Implement Aarch64 SIMD ABI and aarch64_vector_pcs attribute
- References: <1533075888.3879.14.camel@cavium.com> <5B61A40E.1040501@foss.arm.com> <1533593632.3879.90.camel@cavium.com>
Hi!
Some very trivial comments...
On Mon, Aug 06, 2018 at 03:13:52PM -0700, Steve Ellcey wrote:
> (aarch64_components_for_bb): Check for simd function.
> (aarch64_epilogue_uses): New function.
> (aarch64_process_components): Ditto.
> (aarch64_expand_prologue): Ditto.
> (aarch64_expand_epilogue): Ditto.
> (aarch64_expand_call): Ditto.
Those "Ditto"s are meant to read "Check for simd function", not "New
function".
> +int
> +aarch64_epilogue_uses (int regno)
> +{
> + if (epilogue_completed && (regno) == LR_REGNUM)
Those parens around "regno" are a bit superfluous, makes the reader think
there is more going on than there is ;-)
> +(define_insn "store_pair_dw_tftf"
> + [(set (match_operand:TF 0 "aarch64_mem_pair_operand" "=Ump")
> + (match_operand:TF 1 "register_operand" "w"))
> + (set (match_operand:TF 2 "memory_operand" "=m")
> + (match_operand:TF 3 "register_operand" "w"))]
> + "TARGET_SIMD &&
> + rtx_equal_p (XEXP (operands[2], 0),
The && should be on the continuation line.
> +(define_insn "loadwb_pair<TX:mode>_<P:mode>"
> + [(parallel
> + [(set (match_operand:P 0 "register_operand" "=k")
> + (plus:P (match_operand:P 1 "register_operand" "0")
> + (match_operand:P 4 "aarch64_mem_pair_offset" "n")))
> + (set (match_operand:TX 2 "register_operand" "=w")
> + (mem:TX (match_dup 1)))
> + (set (match_operand:TX 3 "register_operand" "=w")
> + (mem:TX (plus:P (match_dup 1)
> + (match_operand:P 5 "const_int_operand" "n"))))])]
> + "TARGET_SIMD && INTVAL (operands[5]) == GET_MODE_SIZE (<TX:MODE>mode)"
> + "ldp\\t%q2, %q3, [%1], %4"
> + [(set_attr "type" "neon_ldp_q")]
> +)
Here you don't indent with tabs.
> +/* { dg-final { scan-assembler "\[ \t\]stp\tq10, q11" } } */
If you use {} instead of "" you don't need to backtick everything.
Also, instead of [ \t] you can probably use [[:space:]] which has
shorthand \s .
> +/* { dg-final { scan-assembler-not "\[ \t\]stp\tq\[01234567\]" } } */
That's [0-7] but maybe you find [01234567] more readable here.
> +/* { dg-final { scan-assembler-not "\[ \t\]str\t" } } */
You can also use \m and \M for start resp. end of word:
/* { dg-final { scan-assembler-not {\mstr\M} } } */
(or if you like double quotes better that is:
/* { dg-final { scan-assembler-not "\\mstr\\M" } } */
but why would you want that ;-) )
Segher