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

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

[Heiko Stuebner]

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
 
-- 
2.35.1


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

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

[Palmer Dabbelt]

On Thu, 05 Jan 2023 11:26:10 PST (-0800), heiko@sntech.de 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

Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>

I'm going to hold off on this one for this week, though -- at least to 
give it some time on the lists, but it appears this hasn't landed at 
lore/patchwork yet and thus might be stuck.  It is in the mailman 
archives...

http://lists.infradead.org/pipermail/linux-riscv/2023-January/024682.html

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

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

[Heiko Stübner]

Am Donnerstag, 5. Januar 2023, 21:47:47 CET schrieb Palmer Dabbelt:
> On Thu, 05 Jan 2023 11:26:10 PST (-0800), heiko@sntech.de 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
> 
> Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>
> 
> I'm going to hold off on this one for this week, though -- at least to 
> give it some time on the lists, but it appears this hasn't landed at 
> lore/patchwork yet and thus might be stuck.  It is in the mailman 
> archives...

No worries - there isn't any real-word breakage happening with 6.2-rc
So we're in no particular rush to fix somebodies broken boardfarm :-)

Heiko

> http://lists.infradead.org/pipermail/linux-riscv/2023-January/024682.html
> 





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

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

[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
>  
> -- 
> 2.35.1
>

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.

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

Thanks,
drew

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

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

[Heiko Stübner]

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