| Message ID | 20220402050041.21302-1-palmer@rivosinc.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | RISC-V -march handling improvements | expand |
On 01.04.2022 22:00:35, Palmer Dabbelt wrote: > As pointed out recently [1], sparse is parsing -march on RISC-V in order > to obtain the default set of preprocessor macros to use. Back when this > was written ISA string was a simple affair, but these days it's a lot > more complicated. It's going to be a big chunk of work to get a proper > ISA string parser into sparse, but we can at least fix the breakages for > the subset of legal ISA strings that Linux currently uses (and are > breaking users). > > This patch set does three things: > > * Stops die()ing on unknown ISA strings, unless the user has passed > -Wsparse-error. This prints a warning and guesses at the macros to > use, which is probably fine for Linux. > * Cleans up some of the differences between GCC's -march parsing and > sparse's. None of this should really matter for Linux, as GCC will > blow up on bad ISA strings, but it just seemed worth doing when I was > in there. > * Adds support for the Zicsr and Zifencei extensions, which were > recently enabled. With these the unknown ISA string warning goes away > for Linux builds. > > They're all sort of independent (and happen in this order), but they're > all touching the same code so I'm just sending it as a series. It's my > first time touching sparse. > > I've poked around with the first patch on its own and it seems to > largely work as expected: I'm still getting a bunch of sparse-related > warnings when I turn on sparse in my builds, but at least I don't get an > error (after updating to a binutils that supports the new arguments, so > Linux detects them). I tried CF="-Wsparse-error", which also behaves as > expected (that trinary boolean tripped me up for a bit). > > The first patch alone should be a sufficient band-aid for systems that > are actively broken right now, the rest are cleanups -- these may be > necessary to get the RISC-V port sparse-clean, but that's a WIP so there > might be more. I'm going to play around with that, but just looking at > the volume of spew it's probably going to take a while. I gave these > patches a bit of testing one-by-one, but not nearly as much as the > first. > > I just spun up a sparse repo [2] at kernel.org, these are on the riscv > branch if that helps for anyone. I've also started messing around with > parsing a few more of the multi-letter extensions, but there's so much > coupling I got fed up -- it's on riscv-wip, but I definitely don't like > that last patch. I figured it's better to send out these bits, as they > look solid to me and builds are broken. The new stuff (B, K, and V) are > all in GCC-12 anyway, so we have a bit of time before they're useful. > > [1]: https://lore.kernel.org/linux-sparse/mhng-c280d48c-477d-4589-baee-255c774b5a51@palmer-mbp2014/T/#maef705f448e4a1f12d853c0d8bc756f037ce1ce0 > [2]: https://git.kernel.org/pub/scm/linux/kernel/git/palmer/sparse.git Works without warnings on Debian testing, with gcc-riscv64-linux-gnu 4:11.2.0--1. Tested-by: Marc Kleine-Budde <mkl@pengutronix.de> regards, Marc
On Mon, 04 Apr 2022 10:52:53 PDT (-0700), mkl@pengutronix.de wrote: > On 01.04.2022 22:00:35, Palmer Dabbelt wrote: >> As pointed out recently [1], sparse is parsing -march on RISC-V in order >> to obtain the default set of preprocessor macros to use. Back when this >> was written ISA string was a simple affair, but these days it's a lot >> more complicated. It's going to be a big chunk of work to get a proper >> ISA string parser into sparse, but we can at least fix the breakages for >> the subset of legal ISA strings that Linux currently uses (and are >> breaking users). >> >> This patch set does three things: >> >> * Stops die()ing on unknown ISA strings, unless the user has passed >> -Wsparse-error. This prints a warning and guesses at the macros to >> use, which is probably fine for Linux. >> * Cleans up some of the differences between GCC's -march parsing and >> sparse's. None of this should really matter for Linux, as GCC will >> blow up on bad ISA strings, but it just seemed worth doing when I was >> in there. >> * Adds support for the Zicsr and Zifencei extensions, which were >> recently enabled. With these the unknown ISA string warning goes away >> for Linux builds. >> >> They're all sort of independent (and happen in this order), but they're >> all touching the same code so I'm just sending it as a series. It's my >> first time touching sparse. >> >> I've poked around with the first patch on its own and it seems to >> largely work as expected: I'm still getting a bunch of sparse-related >> warnings when I turn on sparse in my builds, but at least I don't get an >> error (after updating to a binutils that supports the new arguments, so >> Linux detects them). I tried CF="-Wsparse-error", which also behaves as >> expected (that trinary boolean tripped me up for a bit). >> >> The first patch alone should be a sufficient band-aid for systems that >> are actively broken right now, the rest are cleanups -- these may be >> necessary to get the RISC-V port sparse-clean, but that's a WIP so there >> might be more. I'm going to play around with that, but just looking at >> the volume of spew it's probably going to take a while. I gave these >> patches a bit of testing one-by-one, but not nearly as much as the >> first. >> >> I just spun up a sparse repo [2] at kernel.org, these are on the riscv >> branch if that helps for anyone. I've also started messing around with >> parsing a few more of the multi-letter extensions, but there's so much >> coupling I got fed up -- it's on riscv-wip, but I definitely don't like >> that last patch. I figured it's better to send out these bits, as they >> look solid to me and builds are broken. The new stuff (B, K, and V) are >> all in GCC-12 anyway, so we have a bit of time before they're useful. >> >> [1]: https://lore.kernel.org/linux-sparse/mhng-c280d48c-477d-4589-baee-255c774b5a51@palmer-mbp2014/T/#maef705f448e4a1f12d853c0d8bc756f037ce1ce0 >> [2]: https://git.kernel.org/pub/scm/linux/kernel/git/palmer/sparse.git > > Works without warnings on Debian testing, with gcc-riscv64-linux-gnu > 4:11.2.0--1. > > Tested-by: Marc Kleine-Budde <mkl@pengutronix.de> Thanks. IIRC you were actually getting the failures from the other thread, so I think this is OK, but jus for everyone else: Unfortunately there's a few more variables than just the GCC version, this depends on the binutils version (and IIUC the binutils version GCC was compiled with) and how Linux was configured. I was testing with a V=1 build to make sure "-march=rv64imafdc_zicsr_zifence" showed up, it should be the same for all files so I was just poking one. > > regards, > Marc > > -- > Pengutronix e.K. | Marc Kleine-Budde | > Embedded Linux | https://www.pengutronix.de | > Vertretung West/Dortmund | Phone: +49-231-2826-924 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Fri, Apr 01, 2022 at 10:00:35PM -0700, Palmer Dabbelt wrote: Hi, Thank you for these patches and please accept my apologies for this much delayed reply. I've just pushed the Zicsr and Zifencei patches since these solve an immediate problem and I've some minor questions/objections for the other ones. -- Luc
On Sat, 21 May 2022 14:23:25 PDT (-0700), luc.vanoostenryck@gmail.com wrote: > On Fri, Apr 01, 2022 at 10:00:35PM -0700, Palmer Dabbelt wrote: > > Hi, > > Thank you for these patches and please accept my apologies > for this much delayed reply. No worries, looks like we're about equally slow ;) > I've just pushed the Zicsr and Zifencei patches since these solve > an immediate problem and I've some minor questions/objections for > the other ones. From looking at sparse/master, it looks like a lot of those comments got resolved and merged. That all looks good to me, I pointed my local sparse over to ce1a6720 ("Merge branches 'unreplaced' and 'inline'") and will be using that (via `make ... C=1 CF="-Wno-sparse-error"`) in my pre-merge testing until something changes. Thanks!