From: David Malcolm <dmalcolm@redhat.com>
To: Antoni Boucher <bouanto@zoho.com>,
gcc-patches@gcc.gnu.org, jit@gcc.gnu.org
Subject: Re: [PATCH] libgccjit: Add support for register variables [PR104072]
Date: Mon, 24 Jan 2022 18:20:34 -0500 [thread overview]
Message-ID: <ec5309bb58839688e7e659b3998452b40110655d.camel@redhat.com> (raw)
In-Reply-To: <9f0e398fc80a28c4a95313f7b1e16fb4651ebe1d.camel@zoho.com>
On Sat, 2022-01-22 at 19:29 -0500, Antoni Boucher wrote:
> Hi.
>
> Le mardi 18 janvier 2022 à 18:49 -0500, David Malcolm a écrit :
> > On Mon, 2022-01-17 at 19:46 -0500, Antoni Boucher via Gcc-patches
> > wrote:
> > > I missed the comment about the new define, so here's the updated
> > > patch.
> >
> > Thanks for the patch.
> > >
> > > Le lundi 17 janvier 2022 à 19:24 -0500, Antoni Boucher via Jit a
> > > écrit :
> > > > Hi.
> > > > This patch add supports for register variables in libgccjit.
> > > >
> > > > It passes the JIT tests, but since I added a function in
> > > > reginfo.c,
> > > > I
> > > > wonder if I should run the whole testsuite.
> >
> > We're in stage 4 for gcc 12, so we should be especially careful
> > about
> > changes right now, and we're not meant to be adding new GCC 12
> > features.
> >
> > How close is gcc 12's libgccjit to being usable with your rustc
> > backend? If we're almost there, I'm willing to make our case for
> > late-
> > breaking libgccjit changes to the release managers, but if you
> > think
> > rustc users are going to need to build a patched libgccjit, then
> > let's
> > queue this up for gcc 13.
>
> As I mentioned in my other email, if the 4 patches currently being
> reviewed (and listed here:
> https://github.com/antoyo/libgccjit-patches) were included in gcc 12,
> I'd be able to build rustc_codegen_gcc with an unpatched gcc.
Thanks. Once the relevant patches look good to me, I'll approach the
release managers with the proposal.
>
> It is to be noted however, that I'll need more patches for future
> work.
> Off the top of my head, I'll at least need a patch for the inline
> attribute, try/catch and target-specific builtins.
> The last 2 features will probably take some time to implement, so
> I'll
> let you judge if you think it's worth merging the 4 patches currently
> being reviewed for gcc 12.
Thanks, though I don't know enough about your project's features to
make the judgement call. Does rustc_codegen_gcc have releases yet, or
are you just pointing people at the trunk of your repo? I guess the
question is - are you hoping to be able to point people at distro
installs of gcc 12's libgccjit and have some version of
rustc_codegen_gcc "just work" with that, or are they going to have to
rebuild their own libgccjit to meaningfully use it?
[...snip various corrections...]
>
> > > diff --git a/gcc/testsuite/jit.dg/test-register-variable.c
> > > b/gcc/testsuite/jit.dg/test-register-variable.c
> > > new file mode 100644
> > > index 00000000000..3cea3f1668f
> > > --- /dev/null
> > > +++ b/gcc/testsuite/jit.dg/test-register-variable.c
> > > +
[...snip...]
> > > +/* { dg-final { jit-verify-output-file-was-created "" } } */
> > > +/* { dg-final { jit-verify-assembler-output "movl \\\$1,
> > > %r12d" } } */
> > > +/* { dg-final { jit-verify-assembler-output "movl \\\$2,
> > > %r13d" } } */
> >
> > How target-specific is this test?
>
> It will only work on x86-64. Should I feature-gate the test somehow?
Yes; I think you can do this by adding this to the top of the test:
/* { dg-do compile { target x86_64-*-* } } */
like test-asm.c does.
> >
> > We should have test coverage for at least these two errors:
> >
> > - gcc_jit_lvalue_set_register_name(global_variable,
> > "this_is_not_a_register");
> > - attempting to set the name for a var that doesn't fit in the
> > given
> > register (e.g. trying to use a register for an array that's way too
> > big)
>
> Done.
Thanks.
Is the updated patch available for review? It looks like you didn't
attach it.
Dave
next prev parent reply other threads:[~2022-01-24 23:27 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-18 0:24 Antoni Boucher
2022-01-18 0:46 ` Antoni Boucher
2022-01-18 15:54 ` Antoni Boucher
2022-01-18 23:49 ` David Malcolm
2022-01-23 0:29 ` Antoni Boucher
2022-01-24 23:20 ` David Malcolm [this message]
2022-01-24 23:28 ` Antoni Boucher
2022-01-25 17:13 ` Antoni Boucher
2022-02-10 1:33 ` Antoni Boucher
2022-04-12 21:43 ` David Malcolm
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=ec5309bb58839688e7e659b3998452b40110655d.camel@redhat.com \
--to=dmalcolm@redhat.com \
--cc=bouanto@zoho.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=jit@gcc.gnu.org \
/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).