From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from sender4-pp-o90.zoho.com (sender4-pp-o90.zoho.com [136.143.188.90]) by sourceware.org (Postfix) with ESMTPS id 6B3163858400; Tue, 25 Jan 2022 17:13:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 6B3163858400 ARC-Seal: i=1; a=rsa-sha256; t=1643130795; cv=none; d=zohomail.com; s=zohoarc; b=bzcLNoGR+sVrqBhWC3Y0JhMFnk4R73QmsSHeJmF2b7jMI1/yHB4YgaCfmCnqi67pkXDqsJqN+uPXznpobBwJIYZvNBhV1hZ590PMzrtTDaVYj/uzZEXwy9Mh/CabdBV4qvltJV+Oz23h6l44/8fNufj8HO6WGoIDP11WTtTs3Zk= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1643130795; h=Content-Type:Content-Transfer-Encoding:Date:From:In-Reply-To:MIME-Version:Message-ID:References:Subject:To; bh=/iZnbSs7/F9V0b1C0ObxMP5K4wKZ8ls5voSR/AD11Vs=; b=egQw7FcVamIo1Qtyjbu1+7OQD4ywVsGHjO1fk9glrn2h0q30FwPWpHqVAYmMkfl9CjRdl9CCxUxaGavUnDj9t+DlX6a63+c2f9+pV4/jlOc8HVOqHU2Ydec76zyiBf9tL6PE3babX6NJd9iepk/RQSZULPw87ZBQ/GCFJdeLP/c= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass header.i=zoho.com; spf=pass smtp.mailfrom=bouanto@zoho.com; dmarc=pass header.from= Received: from [192.168.1.174] (38.87.11.6 [38.87.11.6]) by mx.zohomail.com with SMTPS id 1643130793380151.75393520981902; Tue, 25 Jan 2022 09:13:13 -0800 (PST) Message-ID: <10b23be1a7be24134dfbc610f7a4ea12e54f5869.camel@zoho.com> Subject: Re: [PATCH] libgccjit: Add support for register variables [PR104072] From: Antoni Boucher To: David Malcolm , gcc-patches@gcc.gnu.org, jit@gcc.gnu.org Date: Tue, 25 Jan 2022 12:13:11 -0500 In-Reply-To: References: <5a254fe52188d76ae6e292288b05f9c99994ff23.camel@zoho.com> <734c3e099c819286df512eb618b2f94bd11f4a87.camel@redhat.com> <9f0e398fc80a28c4a95313f7b1e16fb4651ebe1d.camel@zoho.com> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.42.3 MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-ZohoMailClient: External X-Spam-Status: No, score=-10.6 required=5.0 tests=BAYES_00, BODY_8BITS, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: jit@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Jit mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 25 Jan 2022 17:13:23 -0000 See answers below. Le lundi 24 janvier 2022 =C3=A0 18:20 -0500, David Malcolm a =C3=A9crit=C2= =A0: > On Sat, 2022-01-22 at 19:29 -0500, Antoni Boucher wrote: > > Hi. > >=20 > > Le mardi 18 janvier 2022 =C3=A0 18:49 -0500, David Malcolm a =C3=A9crit= =C2=A0: > > > 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. > > >=20 > > > Thanks for the patch. > > > >=20 > > > > Le lundi 17 janvier 2022 =C3=A0 19:24 -0500, Antoni Boucher via Jit > > > > a > > > > =C3=A9crit=C2=A0: > > > > > Hi. > > > > > This patch add supports for register variables in libgccjit. > > > > >=20 > > > > > It passes the JIT tests, but since I added a function in > > > > > reginfo.c, > > > > > I > > > > > wonder if I should run the whole testsuite. > > >=20 > > > 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. > > >=20 > > > How close is gcc 12's libgccjit to being usable with your rustc > > > backend?=C2=A0 If we're almost there, I'm willing to make our case fo= r > > > 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. > >=20 > > 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. >=20 > Thanks.=C2=A0 Once the relevant patches look good to me, I'll approach th= e > release managers with the proposal. >=20 > >=20 > > 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. >=20 > Thanks, though I don't know enough about your project's features to > make the judgement call.=C2=A0 Does rustc_codegen_gcc have releases yet, > or > are you just pointing people at the trunk of your repo?=C2=A0 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. >=20 > [...snip various corrections...] >=20 > >=20 > > > > 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 > > > > + >=20 > [...snip...] >=20 > > > > +/* { dg-final { jit-verify-output-file-was-created "" } } */ > > > > +/* { dg-final { jit-verify-assembler-output "movl=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0\\\$1, > > > > %r12d" } } */ > > > > +/* { dg-final { jit-verify-assembler-output "movl=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0\\\$2, > > > > %r13d" } } */ > > >=20 > > > How target-specific is this test? > >=20 > > It will only work on x86-64. Should I feature-gate the test > > somehow? >=20 >=20 > Yes; I think you can do this by adding this to the top of the test: >=20 > =C2=A0=C2=A0 /* { dg-do compile { target x86_64-*-* } } */ >=20 > like test-asm.c does. Thanks. I'll update the patch to do this. >=20 > > >=20 > > > We should have test coverage for at least these two errors: > > >=20 > > > - 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) > >=20 > > Done. >=20 > Thanks. >=20 > Is the updated patch available for review? It looks like you didn't > attach it. >=20 > Dave >=20