Message ID | 20230906150605.10221-2-palmer@rivosinc.com |
---|---|
State | New |
Headers | show |
Series | build-many-glibcs: Add a RISC-V config with most of the B extensions | expand |
* Palmer Dabbelt: > + self.add_config(arch='riscv64', > + os_name='linux-gnu', > + variant='rv64imafdcb-lp64d', > + gcc_cfg=['--with-arch=rv64imafdc_zba_zbb_zbs', '--with-abi=lp64d', > + '--disable-multilib']) I doubt we need a separate GCC configuration, you should be able to use the existing compiler for that and just change the glibc build flags. I expect we need some sort of version check because these flags are rather recent, right? To what extend do they actually impact code generation for glibc? Thanks, Florian
On Wed, 06 Sep 2023 13:43:08 PDT (-0700), fweimer@redhat.com wrote: > * Palmer Dabbelt: > >> + self.add_config(arch='riscv64', >> + os_name='linux-gnu', >> + variant='rv64imafdcb-lp64d', >> + gcc_cfg=['--with-arch=rv64imafdc_zba_zbb_zbs', '--with-abi=lp64d', >> + '--disable-multilib']) > > I doubt we need a separate GCC configuration, you should be able to use > the existing compiler for that and just change the glibc build flags. Right now we're building a different GCC for each target, setting the default arch at GCC configure time. I agree that's super inefficient, but it's what the other tagets do. Sharing GCCs will also result in mixing up things like libgcc, which is kind of a double-edged sword. I'm not sure what the right answer is here. For the GCC CI we're trending towards a single target that contains most of the new extensions, maybe that's the right thing to do for glibc as well? It's looking like there'll be a generation of hardware that has B but doesn't have V, though, which is sort of what this target is aimed at. Also +Jeff, in case he has an opinion. > I expect we need some sort of version check because these flags are > rather recent, right? To what extend do they actually impact code > generation for glibc? We've got two inline asm routines that use the B extensions (both Zbb): sysdeps/riscv/string-fza.h:#if defined __riscv_zbb || defined __riscv_xtheadbb sysdeps/riscv/string-fzi.h:#if defined __riscv_zbb || defined __riscv_xtheadbb That's a pretty small diff, but it is code we're not testing -- not sure if that's worth a whole test config, though. On the compiler side the B extensions have a pretty big impact on codegen: they add a bunch of common bit manipulation patterns (sign/zero extension, bit fields, C strings, etc). None of that should change the ABI, so in theory we should be safe if the GCC test suite passes. We do glibc builds as part of the GCC testing, but that usually targets released glibc versions so stuff might slip through. So I guess again kind of a grey area. > > Thanks, > Florian
* Palmer Dabbelt: > On Wed, 06 Sep 2023 13:43:08 PDT (-0700), fweimer@redhat.com wrote: >> * Palmer Dabbelt: >> >>> + self.add_config(arch='riscv64', >>> + os_name='linux-gnu', >>> + variant='rv64imafdcb-lp64d', >>> + gcc_cfg=['--with-arch=rv64imafdc_zba_zbb_zbs', '--with-abi=lp64d', >>> + '--disable-multilib']) >> >> I doubt we need a separate GCC configuration, you should be able to use >> the existing compiler for that and just change the glibc build flags. > > Right now we're building a different GCC for each target, setting the > default arch at GCC configure time. I agree that's super inefficient, > but it's what the other tagets do. Sharing GCCs will also result in > mixing up things like libgcc, which is kind of a double-edged sword. It's more mixed, see the power4 variant of powerpc-linux-gnu for an example. >> I expect we need some sort of version check because these flags are >> rather recent, right? To what extend do they actually impact code >> generation for glibc? > > We've got two inline asm routines that use the B extensions (both Zbb): > > sysdeps/riscv/string-fza.h:#if defined __riscv_zbb || defined __riscv_xtheadbb > sysdeps/riscv/string-fzi.h:#if defined __riscv_zbb || defined __riscv_xtheadbb > > That's a pretty small diff, but it is code we're not testing -- not > sure if that's worth a whole test config, though. You can add IFUNCs and compile the affected string functions twice, then code *will* be compiled in a default build, revealing syntax and other errors. > On the compiler side the B extensions have a pretty big impact on > codegen: they add a bunch of common bit manipulation patterns > (sign/zero extension, bit fields, C strings, etc). None of that > should change the ABI, so in theory we should be safe if the GCC test > suite passes. We do glibc builds as part of the GCC testing, but that > usually targets released glibc versions so stuff might slip through. Do the B extensions change the relocation footprint because they add new instruction encodes? That's an area where we've sometimes encountered problems with changes in ISA baselines/compiler flags. Thanks, Florian
On Thu, 07 Sep 2023 08:50:49 PDT (-0700), fweimer@redhat.com wrote: > * Palmer Dabbelt: > >> On Wed, 06 Sep 2023 13:43:08 PDT (-0700), fweimer@redhat.com wrote: >>> * Palmer Dabbelt: >>> >>>> + self.add_config(arch='riscv64', >>>> + os_name='linux-gnu', >>>> + variant='rv64imafdcb-lp64d', >>>> + gcc_cfg=['--with-arch=rv64imafdc_zba_zbb_zbs', '--with-abi=lp64d', >>>> + '--disable-multilib']) >>> >>> I doubt we need a separate GCC configuration, you should be able to use >>> the existing compiler for that and just change the glibc build flags. >> >> Right now we're building a different GCC for each target, setting the >> default arch at GCC configure time. I agree that's super inefficient, >> but it's what the other tagets do. Sharing GCCs will also result in >> mixing up things like libgcc, which is kind of a double-edged sword. > > It's more mixed, see the power4 variant of powerpc-linux-gnu for an > example. If I'm reading the script correctly, the power4 build is using the same GCC as the powerpc64 hardfloat build? ie this one self.add_config(arch='powerpc64', os_name='linux-gnu', gcc_cfg=['--disable-multilib', '--enable-secureplt']) I think we could do that for this configuration (reuse the rv64gc toolchain), we'd just end up with the libgcc cross-linking issues (which might even be a good thing, as distros will probably have rv64gc libgcc). IIUC we'd need multilib support to get us down to a single GCC build, though. >>> I expect we need some sort of version check because these flags are >>> rather recent, right? To what extend do they actually impact code >>> generation for glibc? >> >> We've got two inline asm routines that use the B extensions (both Zbb): >> >> sysdeps/riscv/string-fza.h:#if defined __riscv_zbb || defined __riscv_xtheadbb >> sysdeps/riscv/string-fzi.h:#if defined __riscv_zbb || defined __riscv_xtheadbb >> >> That's a pretty small diff, but it is code we're not testing -- not >> sure if that's worth a whole test config, though. > > You can add IFUNCs and compile the affected string functions twice, then > code *will* be compiled in a default build, revealing syntax and other > errors. +Evan, who's working on the IFUNC support. I hadn't though of using that to test the assembly, but that seems like the best way to go -- not only will it sort out the testing issues, but users will go faster ;) I think we're pretty close? >> On the compiler side the B extensions have a pretty big impact on >> codegen: they add a bunch of common bit manipulation patterns >> (sign/zero extension, bit fields, C strings, etc). None of that >> should change the ABI, so in theory we should be safe if the GCC test >> suite passes. We do glibc builds as part of the GCC testing, but that >> usually targets released glibc versions so stuff might slip through. > > Do the B extensions change the relocation footprint because they add new > instruction encodes? That's an area where we've sometimes encountered > problems with changes in ISA baselines/compiler flags. We don't have any new relocations for B, at least not yet -- they're all just register/register ops, so while one could imagine some addressing patterns that take advantage we don't have them yet. So I think we're safe on that front. > > Thanks, > Florian
On Thu, Sep 7, 2023 at 9:39 AM Palmer Dabbelt <palmer@rivosinc.com> wrote: > > On Thu, 07 Sep 2023 08:50:49 PDT (-0700), fweimer@redhat.com wrote: > > * Palmer Dabbelt: > > > >> On Wed, 06 Sep 2023 13:43:08 PDT (-0700), fweimer@redhat.com wrote: > >>> * Palmer Dabbelt: > >>> > >>>> + self.add_config(arch='riscv64', > >>>> + os_name='linux-gnu', > >>>> + variant='rv64imafdcb-lp64d', > >>>> + gcc_cfg=['--with-arch=rv64imafdc_zba_zbb_zbs', '--with-abi=lp64d', > >>>> + '--disable-multilib']) > >>> > >>> I doubt we need a separate GCC configuration, you should be able to use > >>> the existing compiler for that and just change the glibc build flags. > >> > >> Right now we're building a different GCC for each target, setting the > >> default arch at GCC configure time. I agree that's super inefficient, > >> but it's what the other tagets do. Sharing GCCs will also result in > >> mixing up things like libgcc, which is kind of a double-edged sword. > > > > It's more mixed, see the power4 variant of powerpc-linux-gnu for an > > example. > > If I'm reading the script correctly, the power4 build is using the same > GCC as the powerpc64 hardfloat build? ie this one > > self.add_config(arch='powerpc64', > os_name='linux-gnu', > gcc_cfg=['--disable-multilib', '--enable-secureplt']) > > I think we could do that for this configuration (reuse the rv64gc > toolchain), we'd just end up with the libgcc cross-linking issues (which > might even be a good thing, as distros will probably have rv64gc libgcc). > > IIUC we'd need multilib support to get us down to a single GCC build, > though. > > >>> I expect we need some sort of version check because these flags are > >>> rather recent, right? To what extend do they actually impact code > >>> generation for glibc? > >> > >> We've got two inline asm routines that use the B extensions (both Zbb): > >> > >> sysdeps/riscv/string-fza.h:#if defined __riscv_zbb || defined __riscv_xtheadbb > >> sysdeps/riscv/string-fzi.h:#if defined __riscv_zbb || defined __riscv_xtheadbb > >> > >> That's a pretty small diff, but it is code we're not testing -- not > >> sure if that's worth a whole test config, though. > > > > You can add IFUNCs and compile the affected string functions twice, then > > code *will* be compiled in a default build, revealing syntax and other > > errors. > > +Evan, who's working on the IFUNC support. I hadn't though of using > that to test the assembly, but that seems like the best way to go -- not > only will it sort out the testing issues, but users will go faster ;) > > I think we're pretty close? I hope so! No one's got any corrections yet on my v8, so we'll see. I haven't dug through this thread, so I'm coming in with only the trimmed context of what was in my inbox. Won't using ifuncs end up being a fairly big runtime penalty, since you're changing "static __always_inline" functions into calls through a pointer? Or is the idea something like: create two files just for test, like string-fzi-zbb.c, and string-fzi-nozbb.c that each force the __riscv_zbb one way, and each define non-inline functions like index_first_with_zbb() that turn around and use the inline ones. Then another just-for-test file with an ifunc selector between index_first_with_zbb() and index_first_no_zbb(). Then you exercise the ifunced index_first_for_test() in test code? -Evan > > >> On the compiler side the B extensions have a pretty big impact on > >> codegen: they add a bunch of common bit manipulation patterns > >> (sign/zero extension, bit fields, C strings, etc). None of that > >> should change the ABI, so in theory we should be safe if the GCC test > >> suite passes. We do glibc builds as part of the GCC testing, but that > >> usually targets released glibc versions so stuff might slip through. > > > > Do the B extensions change the relocation footprint because they add new > > instruction encodes? That's an area where we've sometimes encountered > > problems with changes in ISA baselines/compiler flags. > > We don't have any new relocations for B, at least not yet -- they're all > just register/register ops, so while one could imagine some addressing > patterns that take advantage we don't have them yet. So I think we're > safe on that front. > > > > > Thanks, > > Florian
On Thu, 07 Sep 2023 10:00:47 PDT (-0700), Evan Green wrote: > On Thu, Sep 7, 2023 at 9:39 AM Palmer Dabbelt <palmer@rivosinc.com> wrote: >> >> On Thu, 07 Sep 2023 08:50:49 PDT (-0700), fweimer@redhat.com wrote: >> > * Palmer Dabbelt: >> > >> >> On Wed, 06 Sep 2023 13:43:08 PDT (-0700), fweimer@redhat.com wrote: >> >>> * Palmer Dabbelt: >> >>> >> >>>> + self.add_config(arch='riscv64', >> >>>> + os_name='linux-gnu', >> >>>> + variant='rv64imafdcb-lp64d', >> >>>> + gcc_cfg=['--with-arch=rv64imafdc_zba_zbb_zbs', '--with-abi=lp64d', >> >>>> + '--disable-multilib']) >> >>> >> >>> I doubt we need a separate GCC configuration, you should be able to use >> >>> the existing compiler for that and just change the glibc build flags. >> >> >> >> Right now we're building a different GCC for each target, setting the >> >> default arch at GCC configure time. I agree that's super inefficient, >> >> but it's what the other tagets do. Sharing GCCs will also result in >> >> mixing up things like libgcc, which is kind of a double-edged sword. >> > >> > It's more mixed, see the power4 variant of powerpc-linux-gnu for an >> > example. >> >> If I'm reading the script correctly, the power4 build is using the same >> GCC as the powerpc64 hardfloat build? ie this one >> >> self.add_config(arch='powerpc64', >> os_name='linux-gnu', >> gcc_cfg=['--disable-multilib', '--enable-secureplt']) >> >> I think we could do that for this configuration (reuse the rv64gc >> toolchain), we'd just end up with the libgcc cross-linking issues (which >> might even be a good thing, as distros will probably have rv64gc libgcc). >> >> IIUC we'd need multilib support to get us down to a single GCC build, >> though. >> >> >>> I expect we need some sort of version check because these flags are >> >>> rather recent, right? To what extend do they actually impact code >> >>> generation for glibc? >> >> >> >> We've got two inline asm routines that use the B extensions (both Zbb): >> >> >> >> sysdeps/riscv/string-fza.h:#if defined __riscv_zbb || defined __riscv_xtheadbb >> >> sysdeps/riscv/string-fzi.h:#if defined __riscv_zbb || defined __riscv_xtheadbb >> >> >> >> That's a pretty small diff, but it is code we're not testing -- not >> >> sure if that's worth a whole test config, though. >> > >> > You can add IFUNCs and compile the affected string functions twice, then >> > code *will* be compiled in a default build, revealing syntax and other >> > errors. >> >> +Evan, who's working on the IFUNC support. I hadn't though of using >> that to test the assembly, but that seems like the best way to go -- not >> only will it sort out the testing issues, but users will go faster ;) >> >> I think we're pretty close? > > I hope so! No one's got any corrections yet on my v8, so we'll see. > > I haven't dug through this thread, so I'm coming in with only the > trimmed context of what was in my inbox. Won't using ifuncs end up > being a fairly big runtime penalty, since you're changing "static > __always_inline" functions into calls through a pointer? I think we'll just have to IFUNC the uses of these functions, looks like it's all in the string ops. > Or is the idea something like: create two files just for test, like > string-fzi-zbb.c, and string-fzi-nozbb.c that each force the > __riscv_zbb one way, and each define non-inline functions like > index_first_with_zbb() that turn around and use the inline ones. Then > another just-for-test file with an ifunc selector between > index_first_with_zbb() and index_first_no_zbb(). Then you exercise the > ifunced index_first_for_test() in test code? > > -Evan > > >> >> >> On the compiler side the B extensions have a pretty big impact on >> >> codegen: they add a bunch of common bit manipulation patterns >> >> (sign/zero extension, bit fields, C strings, etc). None of that >> >> should change the ABI, so in theory we should be safe if the GCC test >> >> suite passes. We do glibc builds as part of the GCC testing, but that >> >> usually targets released glibc versions so stuff might slip through. >> > >> > Do the B extensions change the relocation footprint because they add new >> > instruction encodes? That's an area where we've sometimes encountered >> > problems with changes in ISA baselines/compiler flags. >> >> We don't have any new relocations for B, at least not yet -- they're all >> just register/register ops, so while one could imagine some addressing >> patterns that take advantage we don't have them yet. So I think we're >> safe on that front. >> >> > >> > Thanks, >> > Florian
diff --git a/scripts/build-many-glibcs.py b/scripts/build-many-glibcs.py index 3082522140..bbc153831e 100755 --- a/scripts/build-many-glibcs.py +++ b/scripts/build-many-glibcs.py @@ -396,6 +396,11 @@ class Context(object): variant='rv64imafdc-lp64d', gcc_cfg=['--with-arch=rv64imafdc', '--with-abi=lp64d', '--disable-multilib']) + self.add_config(arch='riscv64', + os_name='linux-gnu', + variant='rv64imafdcb-lp64d', + gcc_cfg=['--with-arch=rv64imafdc_zba_zbb_zbs', '--with-abi=lp64d', + '--disable-multilib']) self.add_config(arch='s390x', os_name='linux-gnu', glibcs=[{},
We've had some B-related string optimizaitons show up over the past year, but we don't have a configuration in build-many-glibcs that tests them. This adds Zba, Zbc, and Zbs -- it doesn't add Zbc, but IIUC that's the common configuration. To avoid blowing up the build count it just adds rv64/lp64d. Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com> --- I haven't tested this at all, but figured I'd send it up for comments. I'm not really sure what the bar should be for builds here, we kind of just started with everything and then didn't add any more. --- scripts/build-many-glibcs.py | 5 +++++ 1 file changed, 5 insertions(+)