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

[Palmer Dabbelt <palmer@rivosinc.com>]

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