[PATCH] build-many-glibcs: Add a RISC-V config with most of the B extensions

[PATCH] build-many-glibcs: Add a RISC-V config with most of the B extensions

[Palmer Dabbelt]

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(+)

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=[{},
-- 
2.42.0

Re: [PATCH] build-many-glibcs: Add a RISC-V config with most of the B extensions

[Florian Weimer]

* 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

Re: [PATCH] build-many-glibcs: Add a RISC-V config with most of the B extensions

[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.

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

Re: [PATCH] build-many-glibcs: Add a RISC-V config with most of the B extensions

[Florian Weimer]

* 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

Re: [PATCH] build-many-glibcs: Add a RISC-V config with most of the B extensions

[Palmer Dabbelt]

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

Re: [PATCH] build-many-glibcs: Add a RISC-V config with most of the B extensions

[Evan Green]

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

Re: [PATCH] build-many-glibcs: Add a RISC-V config with most of the B extensions

[Palmer Dabbelt]

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