* [PATCH v2] RISC-V: fix compile error from deduplicated __ALTERNATIVE_CFG_2
@ 2023-01-05 19:26 Heiko Stuebner
2023-01-05 20:47 ` Palmer Dabbelt
2023-01-06 6:54 ` Andrew Jones
0 siblings, 2 replies; 5+ messages in thread
From: Heiko Stuebner @ 2023-01-05 19:26 UTC (permalink / raw)
To: linux-riscv, palmer
Cc: christoph.muellner, conor, philipp.tomsich, ajones, heiko,
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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v2] RISC-V: fix compile error from deduplicated __ALTERNATIVE_CFG_2
2023-01-05 19:26 [PATCH v2] RISC-V: fix compile error from deduplicated __ALTERNATIVE_CFG_2 Heiko Stuebner
@ 2023-01-05 20:47 ` Palmer Dabbelt
2023-01-05 21:07 ` Heiko Stübner
2023-01-06 6:54 ` Andrew Jones
1 sibling, 1 reply; 5+ messages in thread
From: Palmer Dabbelt @ 2023-01-05 20:47 UTC (permalink / raw)
To: heiko
Cc: linux-riscv, christoph.muellner, Conor Dooley, philipp.tomsich,
ajones, heiko, heiko.stuebner
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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] RISC-V: fix compile error from deduplicated __ALTERNATIVE_CFG_2
2023-01-05 20:47 ` Palmer Dabbelt
@ 2023-01-05 21:07 ` Heiko Stübner
0 siblings, 0 replies; 5+ messages in thread
From: Heiko Stübner @ 2023-01-05 21:07 UTC (permalink / raw)
To: Palmer Dabbelt
Cc: linux-riscv, christoph.muellner, Conor Dooley, philipp.tomsich, ajones
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] RISC-V: fix compile error from deduplicated __ALTERNATIVE_CFG_2
2023-01-05 19:26 [PATCH v2] RISC-V: fix compile error from deduplicated __ALTERNATIVE_CFG_2 Heiko Stuebner
2023-01-05 20:47 ` Palmer Dabbelt
@ 2023-01-06 6:54 ` Andrew Jones
2023-01-06 8:23 ` Heiko Stübner
1 sibling, 1 reply; 5+ messages in thread
From: Andrew Jones @ 2023-01-06 6:54 UTC (permalink / raw)
To: Heiko Stuebner
Cc: linux-riscv, palmer, christoph.muellner, conor, philipp.tomsich,
Heiko Stuebner
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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] RISC-V: fix compile error from deduplicated __ALTERNATIVE_CFG_2
2023-01-06 6:54 ` Andrew Jones
@ 2023-01-06 8:23 ` Heiko Stübner
0 siblings, 0 replies; 5+ messages in thread
From: Heiko Stübner @ 2023-01-06 8:23 UTC (permalink / raw)
To: Andrew Jones
Cc: linux-riscv, palmer, christoph.muellner, conor, philipp.tomsich
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-01-06 8:24 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-05 19:26 [PATCH v2] RISC-V: fix compile error from deduplicated __ALTERNATIVE_CFG_2 Heiko Stuebner
2023-01-05 20:47 ` Palmer Dabbelt
2023-01-05 21:07 ` Heiko Stübner
2023-01-06 6:54 ` Andrew Jones
2023-01-06 8:23 ` Heiko Stübner
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.