Re: [PATCH v2] RISC-V: fix compile error from deduplicated __ALTERNATIVE_CFG_2

["Heiko Stübner" <heiko@sntech.de>]

Am Freitag, 6. Januar 2023, 07:54:49 CET schrieb Andrew Jones:
> On Thu, Jan 05, 2023 at 08:26:10PM +0100, Heiko Stuebner wrote:
> > From: Heiko Stuebner <heiko.stuebner@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 it seems 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
> > 
> > Wrapping the variables containing assembler code in quotes solves this issue,
> > compilation and the code in question still works and objdump also shows sane
> > decompiled results of the affected code.
> > 
> > Fixes: 2ba8c7dc71c0 ("riscv: Don't duplicate __ALTERNATIVE_CFG in __ALTERNATIVE_CFG_2")
> > Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> > ---
> > changes in v2:
> > - don't revert the affected cleanup but use quotes around the parts
> > 
> > Tested on qemu + Allwinner D1 + specially created test-cases using
> > ALTERNATIVE_2 in full assembler .S files
> > 
> >  arch/riscv/include/asm/alternative-macros.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/riscv/include/asm/alternative-macros.h b/arch/riscv/include/asm/alternative-macros.h
> > index 7226e2462584..2c0f4c887289 100644
> > --- a/arch/riscv/include/asm/alternative-macros.h
> > +++ b/arch/riscv/include/asm/alternative-macros.h
> > @@ -46,7 +46,7 @@
> >  
> >  .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
> > +	ALTERNATIVE_CFG "\old_c", "\new_c_1", \vendor_id_1, \errata_id_1, \enable_1
> >  	ALT_NEW_CONTENT \vendor_id_2, \errata_id_2, \enable_2, \new_c_2
> >  .endm
> >  
> >
> 
> I was sort of hoping we'd change the vararg "hack" at the same time, with
> a separate patch, but I guess we can focus on getting this breakage fixed
> first and worry about the vararg later.

I was debating with myself about that :-) . But then did go with the
minimal variant, as I expect this to go into 6.2-rc and thus not wanting
to introduce too much changes here.


> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

thanks :-)


Heiko



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