From: Evan Green <evan@rivosinc.com>
To: Palmer Dabbelt <palmer@rivosinc.com>
Cc: fweimer@redhat.com, Jeff Law <jlaw@ventanamicro.com>,
libc-alpha@sourceware.org
Subject: Re: [PATCH] build-many-glibcs: Add a RISC-V config with most of the B extensions
Date: Thu, 7 Sep 2023 10:00:47 -0700 [thread overview]
Message-ID: <CALs-HstDxHOWSbPuJzVsOgsL73oYEJ+5dVOKTmqxceM=x0z6iQ@mail.gmail.com> (raw)
In-Reply-To: <mhng-a3a2cb96-3871-4769-83b5-f922e259c33f@palmer-ri-x1c9>
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
next prev parent reply other threads:[~2023-09-07 17:01 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-06 15:06 Palmer Dabbelt
2023-09-06 20:43 ` Florian Weimer
2023-09-07 15:24 ` Palmer Dabbelt
2023-09-07 15:50 ` Florian Weimer
2023-09-07 16:39 ` Palmer Dabbelt
2023-09-07 17:00 ` Evan Green [this message]
2023-09-07 17:05 ` Palmer Dabbelt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CALs-HstDxHOWSbPuJzVsOgsL73oYEJ+5dVOKTmqxceM=x0z6iQ@mail.gmail.com' \
--to=evan@rivosinc.com \
--cc=fweimer@redhat.com \
--cc=jlaw@ventanamicro.com \
--cc=libc-alpha@sourceware.org \
--cc=palmer@rivosinc.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).