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.133.124]) by sourceware.org (Postfix) with ESMTPS id 9C97338582B0 for ; Wed, 16 Aug 2023 23:06:04 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 9C97338582B0 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1692227164; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=WsfLO7UeTLO9BJEHaXoSMA87r2WZbAgQ3pVLjiG84T8=; b=PtKWeA5Yc21Z3HZxVIcg4qciprHzEy5WhQKvHowcTWRxodfVvtyUMVu5ADNm1sccDJ8l5F ex7W9r+rbPa6pThXukrzOEExzLEXb6uUcFjEhzP5vGfmv++Z5UI3uegqkMnfYDc2qofRH2 SPVzDgIbAYSk+XPZiq4nxZ+BxGfmSy8= Received: from mail-qt1-f199.google.com (mail-qt1-f199.google.com [209.85.160.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-228-5WwNpSNlMMK87yfz6sGtpw-1; Wed, 16 Aug 2023 19:06:02 -0400 X-MC-Unique: 5WwNpSNlMMK87yfz6sGtpw-1 Received: by mail-qt1-f199.google.com with SMTP id d75a77b69052e-40ff67467c9so82075041cf.2 for ; Wed, 16 Aug 2023 16:06:02 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1692227162; x=1692831962; h=mime-version:user-agent:content-transfer-encoding:references :in-reply-to:date:to:from:subject:message-id:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=EVgxejfFXURC6fm1JFdaTt1gpFWnr8wFf2pXSNWme9w=; b=B1WTwIaNPNaZoT92YP6hqUHDe5v5g6PTLT0wv3OwZNBAf6YzDeIPtu4qI2CQzHPQe7 klajB54YCA7PZLzNbFdeeQEVSWN/vpVMcozHwOY5O6iUvtDbsrIFF7Iq+k4hoXrpoAr1 gkxeoUKrcLaxySri7xSwmlGr8G2luRFH7G5WbmB3gPz8+coQ0wg3YkY5QLVMK8G4YBil CfIqM9TU4ky0Ljb8hn0iylQjl84/MvZSae49quTE7QbX8EbHes9R0YxJ8YIOon/+GKLa ZjZQkE7ftq9d89+hkqTvBtx3aMWSQZhR97CUfM2PtbXLoUXTxLJyNa/fGJCPBoS4zkEI mDrw== X-Gm-Message-State: AOJu0Yzmqzj7uPiDWFxABx6Q0QX3iL5Wdt3u4xjnBKh33jhN4oTRe8rW 6v2C+CD+deLh9ldS/HhIOmM4ljkJsdrzavc4jnvtZ/CG+ifI4by4XnVcDETHb/0MXBfXDdwApMu 5JwURIQQ= X-Received: by 2002:a05:622a:50b:b0:400:9896:b0fa with SMTP id l11-20020a05622a050b00b004009896b0famr3304704qtx.64.1692227162361; Wed, 16 Aug 2023 16:06:02 -0700 (PDT) X-Google-Smtp-Source: AGHT+IENd+zYT0QMVU2IJPkRLosF2RwlNG6I+yiELH/cBFzoQ8l2+54EeE16ExyPKp0i475Toa71LQ== X-Received: by 2002:a05:622a:50b:b0:400:9896:b0fa with SMTP id l11-20020a05622a050b00b004009896b0famr3304691qtx.64.1692227162040; Wed, 16 Aug 2023 16:06:02 -0700 (PDT) Received: from t14s.localdomain (c-76-28-97-5.hsd1.ma.comcast.net. [76.28.97.5]) by smtp.gmail.com with ESMTPSA id r25-20020ac84259000000b0040fe0fdf555sm4759744qtm.22.2023.08.16.16.06.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 16 Aug 2023 16:06:01 -0700 (PDT) Message-ID: <059ffebd230df2dbbac3f138ec85016bb7a7306a.camel@redhat.com> Subject: Re: [PATCH] libgccjit: Add support for `restrict` attribute on function parameters From: David Malcolm To: Guillaume Gomez , gcc-patches@gcc.gnu.org, jit@gcc.gnu.org Date: Wed, 16 Aug 2023 19:06:00 -0400 In-Reply-To: References: User-Agent: Evolution 3.44.4 (3.44.4-2.fc36) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-11.3 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Wed, 2023-08-16 at 22:06 +0200, Guillaume Gomez via Jit wrote: > My apologies, forgot to run the commit checkers. Here's the commit > with the errors fixed. >=20 > Le mer. 16 ao=C3=BBt 2023 =C3=A0 18:32, Guillaume Gomez > a =C3=A9crit : > >=20 > > Hi, Hi Guillaume, thanks for the patch. > >=20 > > This patch adds the possibility to specify the __restrict__ > > attribute > > for function parameters. It is used by the Rust GCC backend. What kind of testing has the patch had? (e.g. did you run "make check- jit" ? Has this been in use on real Rust code?) Overall, this patch looks close to being ready, but some nits below... [...] > diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h > index 60eaf39bff6..2e0d08a06d8 100644 > --- a/gcc/jit/libgccjit.h > +++ b/gcc/jit/libgccjit.h > @@ -635,6 +635,10 @@ gcc_jit_type_get_const (gcc_jit_type *type); > extern gcc_jit_type * > gcc_jit_type_get_volatile (gcc_jit_type *type); > =20 > +/* Given type "T", get type "restrict T". */ > +extern gcc_jit_type * > +gcc_jit_type_get_restrict (gcc_jit_type *type); > + > #define LIBGCCJIT_HAVE_SIZED_INTEGERS > =20 > /* Given types LTYPE and RTYPE, return non-zero if they are compatible. Please add a feature macro: #define LIBGCCJIT_HAVE_gcc_jit_type_get_restrict (see the similar ones in the header). > diff --git a/gcc/jit/libgccjit.map b/gcc/jit/libgccjit.map > index e52de0057a5..b7289b13845 100644 > --- a/gcc/jit/libgccjit.map > +++ b/gcc/jit/libgccjit.map > @@ -104,6 +104,7 @@ LIBGCCJIT_ABI_0 > gcc_jit_type_as_object; > gcc_jit_type_get_const; > gcc_jit_type_get_pointer; > + gcc_jit_type_get_restrict; > gcc_jit_type_get_volatile; Please add a new ABI tag (LIBGCCJIT_ABI_25 ?), rather than adding this to ABI_0. > diff --git a/gcc/testsuite/jit.dg/test-restrict.c b/gcc/testsuite/jit.dg/test-restrict.c > new file mode 100644 > index 00000000000..4c8c4407f91 > --- /dev/null > +++ b/gcc/testsuite/jit.dg/test-restrict.c > @@ -0,0 +1,77 @@ > +/* { dg-do compile { target x86_64-*-* } } */ > + > +#include > +#include > + > +#include "libgccjit.h" > + > +/* We don't want set_options() in harness.h to set -O3 to see that the cold > +=09 attribute affects the optimizations. */ This refers to a "cold attribute"; is this a vestige of a copy-and- paste from a different test case? I see that the test scans the generated assembler. Does the test actually verify that restrict has an effect, or was that another vestige from a different test case? > +#define TEST_ESCHEWS_SET_OPTIONS > +static void set_options (gcc_jit_context *ctxt, const char *argv0) > +{ > +=09// Set "-O3". > +=09gcc_jit_context_set_int_option(ctxt, GCC_JIT_INT_OPTION_OPTIMIZATION_LEVEL, 3); > +} > + > +#define TEST_COMPILING_TO_FILE > +#define OUTPUT_KIND GCC_JIT_OUTPUT_KIND_ASSEMBLER > +#define OUTPUT_FILENAME "output-of-test-restrict.c.s" > +#include "harness.h" > + > +void > +create_code (gcc_jit_context *ctxt, void *user_data) > +{ > +=09/* Let's try to inject the equivalent of: > +void t(int *__restrict__ a, int *__restrict__ b, char *__restrict__ c) { > +=09*a +=3D *c; > +=09*b +=3D *c; > +} > +=09*/ > +=09gcc_jit_type *int_type =3D > +=09=09gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT); > +=09gcc_jit_type *pint_type =3D gcc_jit_type_get_pointer(int_type); > +=09gcc_jit_type *pint_restrict_type =3D gcc_jit_type_get_restrict(pint_type); > + > +=09gcc_jit_type *void_type =3D > +=09=09gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_VOID); > + > +=09gcc_jit_param *a =3D > +=09=09gcc_jit_context_new_param (ctxt, NULL, pint_restrict_type, "a"); > +=09gcc_jit_param *b =3D > +=09=09gcc_jit_context_new_param (ctxt, NULL, pint_restrict_type, "b"); > +=09gcc_jit_param *c =3D > +=09=09gcc_jit_context_new_param (ctxt, NULL, pint_restrict_type, "c"); > +=09gcc_jit_param *params[3] =3D {a, b, c}; > + > +=09gcc_jit_function *func_t =3D > +=09=09gcc_jit_context_new_function (ctxt, NULL, > +=09=09=09=09=09GCC_JIT_FUNCTION_EXPORTED, > +=09=09=09=09=09void_type, > +=09=09=09=09=09"t", > +=09=09=09=09=093, params, > +=09=09=09=09=090); > + > +=09gcc_jit_block *block =3D gcc_jit_function_new_block (func_t, NULL); > + > +=09/* *a +=3D *c; */ > +=09gcc_jit_block_add_assignment_op ( > +=09=09block, NULL, > +=09=09gcc_jit_rvalue_dereference (gcc_jit_param_as_rvalue (a), NULL), > +=09=09GCC_JIT_BINARY_OP_PLUS, > +=09=09gcc_jit_lvalue_as_rvalue ( > +=09=09=09gcc_jit_rvalue_dereference (gcc_jit_param_as_rvalue (c), NULL))); > +=09/* *b +=3D *c; */ > +=09gcc_jit_block_add_assignment_op ( > +=09=09block, NULL, > +=09=09gcc_jit_rvalue_dereference (gcc_jit_param_as_rvalue (b), NULL), > +=09=09GCC_JIT_BINARY_OP_PLUS, > +=09=09gcc_jit_lvalue_as_rvalue ( > +=09=09=09gcc_jit_rvalue_dereference (gcc_jit_param_as_rvalue (c), NULL))); > + > +=09gcc_jit_block_end_with_void_return (block, NULL); > +} > + > +/* { dg-final { jit-verify-output-file-was-created "" } } */ > +/* { dg-final { jit-verify-assembler-output "addl=09%eax, (%rdi) > +=09addl=09%eax, (%rsi)" } } */ > --=20 > 2.34.1 >=20 If this test is meant to run at -O3 and thus can't be part of test- combination.c, please add a comment about it to gcc/testsuite/jit.dg/all-non-failing-tests.h (in the alphabetical place). The patch also needs to add documentation for the new entrypoint (in=20 topics/types.rst), and for the new ABI tag (in topics/compatibility.rst). Thanks again for the patch; hope the above is constructive Dave