From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id B75603858D3C for ; Mon, 24 Jan 2022 23:27:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org B75603858D3C Received: from mail-io1-f69.google.com (mail-io1-f69.google.com [209.85.166.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-445-KiZN78VzPEi67ddCAn5BeQ-1; Mon, 24 Jan 2022 18:27:46 -0500 X-MC-Unique: KiZN78VzPEi67ddCAn5BeQ-1 Received: by mail-io1-f69.google.com with SMTP id y6-20020a5e9206000000b0061002a03b66so4877288iop.21 for ; Mon, 24 Jan 2022 15:27:46 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:subject:from:to:date:in-reply-to :references:user-agent:mime-version:content-transfer-encoding; bh=7O+5w82V8JefH6aYWTNTdkteeFcDbVpom2Ok0yz8UN4=; b=CczZbNlUHGHE+W5hRMtfZLW8JnhzzFox0UyEO2CbZD0GQ11eiHQtBYFWIHZlRdpc8j HEFSAtWg6oIz46Yww7La8DlDHuDG2ubgDGbZDaDsQVMWEOS5FOy46Cckg0lyjsbQVUs+ EehhVX+l0Y1pn4ld3Cd5XnblSr5Yw27NwS7e+QGAb2bbhSY68dZrm88AedcbAGjIqhBK R/HShX+JEb30vwzzBWcRVZtBwEuGxZOHzHs9eyGC6hln9/XaKdX7WdOUPEv6udwnXcRW e/bhhX9ivIXeWshhnnXvGQhIX5UUl49ybq7jZK2G/38Ak+M2pjsslDNoFDatiq63Qv0l d4jw== X-Gm-Message-State: AOAM531pa+XUOcEHYcs8PDrHl/qeGodUx1wdZ4r+f5u3MP351AXvXvkr OYffFEXt26N/6zpNOS909mAXRWQ4/SVJa/Zy6i40IoJncZau1KGr9itGN6LyAb7Jzq8afj+Dgw3 HEH+3Zc4= X-Received: by 2002:a02:2424:: with SMTP id f36mr7129056jaa.227.1643066865343; Mon, 24 Jan 2022 15:27:45 -0800 (PST) X-Google-Smtp-Source: ABdhPJzmyOpTkfr989EZyRWLKaAhiwCg49hopMKow/qacBBdd69sez/23mZNXId04uj45fpxBqIASg== X-Received: by 2002:ac8:5809:: with SMTP id g9mr14434231qtg.469.1643066436387; Mon, 24 Jan 2022 15:20:36 -0800 (PST) Received: from t14s.localdomain (c-73-69-212-193.hsd1.nh.comcast.net. [73.69.212.193]) by smtp.gmail.com with ESMTPSA id x13sm8260972qko.114.2022.01.24.15.20.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 24 Jan 2022 15:20:35 -0800 (PST) Message-ID: Subject: Re: [PATCH] libgccjit: Add support for register variables [PR104072] From: David Malcolm To: Antoni Boucher , gcc-patches@gcc.gnu.org, jit@gcc.gnu.org Date: Mon, 24 Jan 2022 18:20:34 -0500 In-Reply-To: <9f0e398fc80a28c4a95313f7b1e16fb4651ebe1d.camel@zoho.com> References: <5a254fe52188d76ae6e292288b05f9c99994ff23.camel@zoho.com> <734c3e099c819286df512eb618b2f94bd11f4a87.camel@redhat.com> <9f0e398fc80a28c4a95313f7b1e16fb4651ebe1d.camel@zoho.com> User-Agent: Evolution 3.38.4 (3.38.4-1.fc33) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-10.5 required=5.0 tests=BAYES_00, BODY_8BITS, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=unavailable 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: Mon, 24 Jan 2022 23:27:49 -0000 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