See answers below.
Le lundi 24 janvier 2022 à 18:20 -0500, David Malcolm a écrit :
> 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?
rustc_codegen_gcc does not have releases. It is merged from time to
time into rustc, so we can as well say that I point them to trunk.
I can feature gate the future features/patches I will need so that
people can still use the project with an unpatched gcc.
There's some interest in having it work with an unpatched gcc because
that would allow people to start working on the infra to get the
project distributed via rustup so that it could be distributed as a
preview component.
>
> [...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.
Thanks. I'll update the patch to do this.
>
> > >
> > > 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
>
