Re: [RFC PATCH v2 3/8] arm64/fpsimdmacros: Allow the macro "for" to be used in more cases

[Dave Martin <Dave.Martin@arm.com>]

On Thu, Jun 13, 2019 at 05:16:51PM +0100, Julien Grall wrote:
> The current version of the macro "for" is only able to work when the
> counter is used to generate registers using mnemonics. This is because

[*] Is this backwards?  Previously, we _can't_ substitute register
numbers directly into proper instruction mnemonics: we have to use
.insn with an expression to generate the opcode directly instead
(possibly via a macro).

With this change we can paste the _for number straight into human-
readable assembler mnemonics.

> gas is not able to evaluate the expression generated if used in
> registers name (i.e x\n).

Nit: maybe: "a register name" or "a register's name"

> Gas offers a way to evaluate macro arguments by using % in front of
> them under the alternate macro mode [1].

This reference is obviously useful to reviewers, but I'm not sure about
the life expectancy of such a URL.  It probably belongs after the
tearoff line rather than in the commit message.

> The implementation of "for" is updated to use the alternate macro mode
> and %, so we can use the macro in more cases. As the alternate macro mode
> may have side-effect, this is disabled when generating the body.

Nit: side-effects

Nit: I'd probably say "expanding the body", to be consistent with gas's
own terminology.

(These are pedantic, and inessential to fix.)

> While it is enough to prefix the argument of the macro "__for_body" with
> %, the arguments of "__for" are also prefixed to get a more bearable
> value in case of compilation error.
> 
> [1] https://sourceware.org/binutils/docs/as/Altmacro.html
> 
> Suggested-by: Dave Martin <dave.martin@arm.com>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> 
> ---
>     Changes in v2:
>         - Fix typo in the commit message
> ---
>  arch/arm64/include/asm/fpsimdmacros.h | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/fpsimdmacros.h b/arch/arm64/include/asm/fpsimdmacros.h
> index 46843515d77b..e2ab77dd9b4f 100644
> --- a/arch/arm64/include/asm/fpsimdmacros.h
> +++ b/arch/arm64/include/asm/fpsimdmacros.h
> @@ -177,19 +177,23 @@
>  
>  .macro __for from:req, to:req
>  	.if (\from) == (\to)
> -		_for__body \from
> +		_for__body %\from
>  	.else
> -		__for \from, (\from) + ((\to) - (\from)) / 2
> -		__for (\from) + ((\to) - (\from)) / 2 + 1, \to
> +		__for %\from, %((\from) + ((\to) - (\from)) / 2)
> +		__for %((\from) + ((\to) - (\from)) / 2 + 1), %\to
>  	.endif
>  .endm
>  
>  .macro _for var:req, from:req, to:req, insn:vararg
>  	.macro _for__body \var:req
> +		.noaltmacro
>  		\insn
> +		.altmacro
>  	.endm
>  
> +	.altmacro
>  	__for \from, \to
> +	.noaltmacro
>  
>  	.purgem _for__body
>  .endm

Looks OK to me.  With [*] addressed as appropriate, and modulo others'
comments (if any):

Reviewed-by: Dave Martin <Dave.Martin@arm.com>

Cheers
---Dave

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel