I realized we missed this on trunk, and I need this on adding -mcpu for sfive cores, so I'm gonna push this to trunk. Most concerns are around the assembler stuff, so I believe it's less controversial on the toolchain driver side.
On Wed, Nov 23, 2022 at 6:01 AM Palmer Dabbelt <pal...@rivosinc.com> wrote: > > On Tue, 22 Nov 2022 13:50:28 PST (-0800), jeffreya...@gmail.com wrote: > > > > On 11/22/22 08:29, Palmer Dabbelt wrote: > >> On Tue, 22 Nov 2022 07:20:15 PST (-0800), jeffreya...@gmail.com wrote: > >>> > >>> On 11/20/22 18:36, Kito Cheng wrote: > >>>>> So the idea here is just to define the extension so that it gets > >>>>> defined > >>>>> in the ISA strings and passed through to the assembler, right? > >>>> That will also define arch test marco: > >>>> > >>>> https://github.com/riscv-non-isa/riscv-c-api-doc/blob/master/riscv-c-api.md#architecture-extension-test-macro > >>>> > >>> > >>> Sorry I should have been clearer and included the test macro(s) as well. > >>> > >>> So a better summary would be that while it doesn't change the codegen > >>> behavior in the compiler, it does provide the mechanisms to pass along > >>> isa strings to other tools such as the assembler and signal via the test > >>> macros that this extension is available. > >> > >> IMO the important bit here is that we're not adding any compatibility > >> flags, like we did when fence.i was removed from the ISA. That's fine > >> as long as we never remove these instructions from the base ISA in the > >> software, but that's what's suggested by Andrew in the post. > > > > Right. IIUC these instructions were never supposed to be in the base > > ISA, but, in effect, snuck through. We're retro-actively adding them as > > an extension, at least in terms of ISA strings & test macros. We're > > currently (forever?) going to allow them in the assembler without > > strictly requiring the extension be on. > > That'd the the idea. > > >> It's a super weird one, but there's a bunch of cases in RISC-V where > >> we're told to just ignore words in the ISA manual. Definitely a trap > >> for users (and we already had some Linux folks get bit by the counter > >> changes here), but that's just how RISC-V works. > > > > Mistakes happen. The key is to adjust for them as best as we can. > > I'd lean towards a stricter enforcement, bringing these > > instructions/extension in line with how we handle the others. It'd > > potentially mean source incompatibilities that would need to be fixed, > > but they shouldn't be difficult and we're still early enough in the game > > that we *could* take that route. Andrew's position is more > > accommodating of existing code and while I may not 100% agree with his > > position, I understand it. > > > > > > So while I'd lean towards a stricter checking, I can live with this > > approach. I wouldn't mind hearing from Kito, Philipp and others though. > > That's the sort of thing we've traditionally done: essentially just read > the actual words in the PDF and produce implementations that match > those, tagging versions when things change (the fence.i stuff is a good > example). After some amount of time we can then move the default spec > version over to the new one. That's a little bit of churn for users, > but it shouldn't be all that bad. > > IMO that's the sane way to go, I'd certainly expect to be able to read > the words in the PDFs and go implement things according to them. It's > pretty clearly not what the ISA folks want, though. > > There's also the secondary issue of getting ISA strings to match between > the various bits of the software stack that uses them. We're trying to > move away from ISA strings as a stable uABI in Linux for exactly this > reason, but ISA strings have already ended up all over the place so > there's only so much we can do.
