[PATCH] RISC-V: fix compile error from decuplicated __ALTERNATIVE_CFG_2

[Andrew Jones]

On Tue, Jan 03, 2023 at 10:42:28PM +0100, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner at vrull.eu>
> 
> On the non-assembler-side wrapping alternative-macros inside other macros
> to prevent duplication of code works, as the end result will just be a
> string that gets fed to the asm instruction.
> 
> In real assembler code, wrapping .macro blocks inside other .macro blocks
> brings more restrictions on usage and the optimization done by
> commit 2ba8c7dc71c0 ("riscv: Don't duplicate __ALTERNATIVE_CFG in __ALTERNATIVE_CFG_2")
> results in a compile error like:
> 
> ../arch/riscv/lib/strcmp.S: Assembler messages:
> ../arch/riscv/lib/strcmp.S:15: Error: too many positional arguments
> ../arch/riscv/lib/strcmp.S:15: Error: backward ref to unknown label "886:"
> ../arch/riscv/lib/strcmp.S:15: Error: backward ref to unknown label "887:"
> ../arch/riscv/lib/strcmp.S:15: Error: backward ref to unknown label "886:"
> ../arch/riscv/lib/strcmp.S:15: Error: backward ref to unknown label "887:"
> ../arch/riscv/lib/strcmp.S:15: Error: backward ref to unknown label "886:"
> ../arch/riscv/lib/strcmp.S:15: Error: attempt to move .org backwards

Ouch. I thought I had tested that, but looking now at my test code I see I
only bothered to test ALTERNATIVE(), not ALTERNATIVE_2(). Adding the
ALTERNATIVE_2() test, I can reproduce this.

It appears the issue is that as macro arguments may be separated by commas
or spaces, the old and new instruction macro arguments, which have spaces
between their instructions and operands, get interpreted as extra macro
arguments. There's probably no way to convince the macro otherwise,
unfortunately.

> 
> Going back to the original code for the non-assembler-part makes that
> code work again. So this reverts the #ifdef ASSEMBLY part of that commit
> to the previous variant with duplicated base.
> 
> Fixes: 2ba8c7dc71c0 ("riscv: Don't duplicate __ALTERNATIVE_CFG in __ALTERNATIVE_CFG_2")
> Signed-off-by: Heiko Stuebner <heiko.stuebner at vrull.eu>
> ---
> I was of two minds about either to revert the full patch, or doing just
> this partial one for the ASSEMBLY part. I did go with this variant, as I
> still like the idea of deduplicating as much as possible :-)
> 
>  arch/riscv/include/asm/alternative-macros.h | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/include/asm/alternative-macros.h b/arch/riscv/include/asm/alternative-macros.h
> index 7226e2462584..e7bdb2a510a4 100644
> --- a/arch/riscv/include/asm/alternative-macros.h
> +++ b/arch/riscv/include/asm/alternative-macros.h
> @@ -44,9 +44,20 @@
>  	ALT_NEW_CONTENT \vendor_id, \errata_id, \enable, \new_c
>  .endm
>  
> +/*
> + * Using ALTERNATIVE_CFG inside ALTERNATIVE_CFG_2 results in compile errors.
> + * So the common code needs to stay duplicated.
> + */
>  .macro ALTERNATIVE_CFG_2 old_c, new_c_1, vendor_id_1, errata_id_1, enable_1,	\
>  				new_c_2, vendor_id_2, errata_id_2, enable_2
> -	ALTERNATIVE_CFG \old_c, \new_c_1, \vendor_id_1, \errata_id_1, \enable_1
> +886 :
> +	.option push
> +	.option norvc
> +	.option norelax
> +	\old_c
> +	.option pop
> +887 :

We could still share this by creating another macro which only takes old_c,
and then invoke that from both ALTERNATIVE_CFG and ALTERNATIVE_CFG_2, I
think.

> +	ALT_NEW_CONTENT \vendor_id_1, \errata_id_1, \enable_1, \new_c_1
>  	ALT_NEW_CONTENT \vendor_id_2, \errata_id_2, \enable_2, \new_c_2
>  .endm
>  
> -- 
> 2.35.1
> 

Thanks,
drew