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

Heiko Stübner
2023-01-05T22:20:04Z

Am Donnerstag, 5. Januar 2023, 16:00:45 CET schrieb Andrew Jones:
> On Wed, Jan 04, 2023 at 04:58:00PM +0100, Heiko Stübner wrote:
> > Hi again,
> >
> > Am Mittwoch, 4. Januar 2023, 15:08:33 CET schrieb Andrew Jones:
> > > On Wed, Jan 04, 2023 at 01:29:25PM +0000, Conor Dooley wrote:
> > > > On Tue, Jan 03, 2023 at 10:42:28PM +0100, Heiko Stuebner wrote:
> > > > > From: Heiko Stuebner <heiko.stuebner@vrull.eu>
> > > >
> > > > > RISC-V: fix compile error from decuplicated __ALTERNATIVE_CFG_2
> > > >
> > > > Hey Heiko/Palmer,
> > > >
> > > > Was a little hard to tell from Drew's mail if he was objecting to this
> > > > variant of the patch, but FWIW s/decup/dedup/ if this gets applied
> > > > as-is.
> > >
> > > I wasn't completely objecting, but rather suggesting we still try to
> > > factor out what we can. However, after reading a bit more about macros
> > > I tried this
> > >
> > > .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
> > >
> > > which appears to work for my simple test. Does that work for you too,
> > > Heiko? If so, then I think I'd prefer we do that. Also, we may want to add
> > > quotes to all macro arguments which may contain spaces, even if things
> > > seem to work now, e.g. the \new_c_2 argument passed to ALT_NEW_CONTENT.
> >
> > Yay ... adding quotes really seems to work - at least a GNU toolchain
>
> clang isn't complaining for my test either.

Nice

>
> >
> > I guess that may have been the reason to move new_c at the end and making
> > it vararg in ALT_NEW_CONTENT originally.
> >
> > And in fact with added quotes the varargs argument wouldn't be necessary
> > anymore.
> >
> > Just removing the varargs argument alone results in
> > Error: too many positional arguments
> > erros, but wrapping the "\new_c*" in quotes lets it compile again.
>
> Excellent news. Do you plan to post the patch? Or should I post with your
> Reported-by tag?

I was planning on posting this today. Did the v2 an hour ago :-)

Heiko