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 01DA03858421 for ; Thu, 11 Jan 2024 22:38:54 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 01DA03858421 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 01DA03858421 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1705012771; cv=none; b=EMmG+WWUTA9VeHyfE0PerzoVuqnZ1LL/QEvIH8PU4Mn39iY1rG2fvjYGk9AazemR9fC1UpAgi6+QhbNC19Ubb8G5DDn+4hVt754gI/8Fi6mlQSDBvdPjKWVasfo/UaVM1nY3je5R+PP2MGbyTLBAD87q/GI15N3Fboh0BrIY0Dw= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1705012771; c=relaxed/simple; bh=aA1l2N//JIxJUcFsLVPhQLQN2lHzScHbDHNClAVyfco=; h=DKIM-Signature:Message-ID:Subject:From:To:Date:MIME-Version; b=brSNL9dvUYc162HAap2Zq6/3YinjmBHL4QKwzT1+o6mZDyqiFkpQRdY/dsgLAcCyg4pF5jiia5UgaipiNfBuEzYGhd+oTYSmDoXrdictuNtx8K2BRTlKIN8+Ieec5XBz2ie6XJTZovX1ej0nfyafiEVo8Uji+OSSrplGdzrzRNs= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1705012734; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=aA1l2N//JIxJUcFsLVPhQLQN2lHzScHbDHNClAVyfco=; b=Gh5gjhxeDdtii9LKmN27/BTofvPD7Wq5exxPVxE6lIyI5peySRkjrVdPg18/BwS75kbVnp 3JbCvoBdW76XJx7SeneYm6NzNCTgOdGiGQhU9DFM6mEJnWbJ0U+t6qzU/pBkV8q5Kv6udi JtuDpc66a3hHO3AFmpGwdY0W+kZ6Rbc= Received: from mail-qt1-f197.google.com (mail-qt1-f197.google.com [209.85.160.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-185-gbHHDU42P922QJCw8ZiBwQ-1; Thu, 11 Jan 2024 17:38:53 -0500 X-MC-Unique: gbHHDU42P922QJCw8ZiBwQ-1 Received: by mail-qt1-f197.google.com with SMTP id d75a77b69052e-4299df5e5f0so52291071cf.1 for ; Thu, 11 Jan 2024 14:38:52 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705012732; x=1705617532; h=mime-version:user-agent:content-transfer-encoding:references :in-reply-to:date:cc:to:from:subject:message-id:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=aA1l2N//JIxJUcFsLVPhQLQN2lHzScHbDHNClAVyfco=; b=HrHSgBGNt64EhjKnihQxBomBGeEjJcUHmT1k6IaSf366CarFEh66JFEJNfNnnmifnE G5EpHqDkJCqi710Y3CRl5y8fabbMMY8Ck/iRI/QnobmTjnRqCIlfgdWKmESKv4sxpJ7/ M1SUvPaxr8xnf0l0F/8wWWmfxMaeEaqFyAEC5kVI5N2cmtS+OHnC0l5a1M83cPLiJl9x u9IkhhH9jJREW+Kj3Yv+1bW6fILqEHNE/xajwayWMH1irFnbAUDHF/0noAIP98x7u3Uy ISg+s0nOX0W9mNZIdw3hpYan7uigmDuIogKeE8DZbqaA6NNdc+JGVuW+OcsHhB7LlT6H RCBQ== X-Gm-Message-State: AOJu0YyXVHnLv+jjVHzDw5vFSvdXdLuMKmf8nWj3xPGuovca5HlAUhZj Ea0Cro7U8uu+RAyozdv/+d4xTduXF5TWsbTCG7IyEZlp0IXUto0sGRO+pgxbtJlQDkFg+gAACUI M4c392LwUndweRIQ3K77HAaU= X-Received: by 2002:a05:622a:11c9:b0:429:7d0c:6046 with SMTP id n9-20020a05622a11c900b004297d0c6046mr581239qtk.68.1705012731747; Thu, 11 Jan 2024 14:38:51 -0800 (PST) X-Google-Smtp-Source: AGHT+IHsJ4V9tssBlyb5r8gCz9QO8piHncZG+R4qJbHX351JjZywfE7Mk/EAUuHmldA7za3ystnl0A== X-Received: by 2002:a05:622a:11c9:b0:429:7d0c:6046 with SMTP id n9-20020a05622a11c900b004297d0c6046mr581227qtk.68.1705012731366; Thu, 11 Jan 2024 14:38:51 -0800 (PST) Received: from t14s.localdomain (c-76-28-97-5.hsd1.ma.comcast.net. [76.28.97.5]) by smtp.gmail.com with ESMTPSA id cd11-20020a05622a418b00b0042994b3c20dsm827255qtb.29.2024.01.11.14.38.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 11 Jan 2024 14:38:50 -0800 (PST) Message-ID: <686f67b56fd0a95a6b3ce8d2a60826aa9c446255.camel@redhat.com> Subject: Re: [PATCH] Add support for function attributes and variable attributes From: David Malcolm To: Guillaume Gomez Cc: jit@gcc.gnu.org, Antoni , gcc-patches@gcc.gnu.org Date: Thu, 11 Jan 2024 17:38:49 -0500 In-Reply-To: References: <319a998a3c63eaf0e28d7267a901ad2c016dba49.camel@redhat.com> 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=-10.9 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_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE 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 Thu, 2024-01-11 at 22:40 +0100, Guillaume Gomez wrote: > Hi David, >=20 > > The above looks correct, but the patch adds the entrypoint > > descriptions > > to topics/types.rst, which seems like the wrong place.=C2=A0 The > > function- > > related ones should be in topics/functions.rst in the "Functions" > > section and the lvalue/variable one in topics/expression.rst after > > the > > "Global variables" section. >=20 > Ah indeed. Mix-up on my end. Fixed it. >=20 > > test-restrict.c is a pre-existing testcase, so please don't delete > > its > > entry. >=20 > Ah indeed, I went too quickly and thought it was a test I renamed... >=20 > > BTW, the ChangeLog entry mentions adding test-restrict.c, but the > > patch > > doesn't add it, so that part of the proposed ChangeLog is wrong. > >=20 > > Does the patch pass ./contrib/gcc-changelog/git_check_commit.py ? >=20 > I messed up a bit, fixed it thanks to you. I didn't run the script in > my last > update but just did: >=20 > ``` > $ contrib/gcc-changelog/git_check_commit.py $(git log -1 --format=3D%h) > Checking 3849ee2eadf0eeec2b0080a5142ced00be96a60d: OK > ``` >=20 > > Otherwise, looks good, assuming that the patch has been tested with > > the > > full jit testsuite. >=20 > When rebasing on upstream yesterday I discovered that two tests > were not working anymore. For the first one, it was simply because of > the changes in `dummy-frontend.cc`. For the second one > (test-noinline-attribute.c), it was because the rules for inlining > changed > since we wrote this patch apparently (our fork is very late). Antoni > discovered > that we could just add a call to `asm` to prevent this from happening > so I > added it. >=20 > So yes, all jit tests are passing as expected. :) Good. It sounds like the patch you have locally is ready, but it has some nontrivial changes compared to the last version you posted to the list. Please post your latest version to the list. Do you have push rights, or do you need me to push it for you? Thanks Dave >=20 > Le jeu. 11 janv. 2024 =C3=A0 19:46, David Malcolm a > =C3=A9crit : > >=20 > > On Thu, 2024-01-11 at 01:00 +0100, Guillaume Gomez wrote: > > > Hi David. > > >=20 > > > Thanks for the review! > > >=20 > > > > > +.. function::=C2=A0 void\ > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0 gcc_jit_lvalue_add_string_attribute > > > > > (gcc_jit_lvalue *variable, > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 enum > > > > > gcc_jit_fn_attribute attribute, > > > >=20 > > > > ^^ > > > >=20 > > > > This got out of sync with the declaration in the header file; > > > > it > > > > should > > > > be enum gcc_jit_variable_attribute attribute > > >=20 > > > Indeed, good catch! > > >=20 > > > > I took a brief look through the handler functions and with the > > > > above > > > > caveat I didn't see anything obviously wrong.=C2=A0 I'm going to > > > > assume > > > > this > > > > code is OK given that presumably you've been testing it within > > > > rustc, > > > > right? > > >=20 > > > Both in rustc and in the JIT tests we added. > > >=20 > > > [..snip...] > > >=20 > > > I added all the missing `RETURN_IF_FAIL` you mentioned. None of > > > the > > > arguments should be `NULL` so it was a mistake not to check it. > > >=20 > > > [..snip...] > > >=20 > > > I removed the tests comments as you mentioned. > > >=20 > > > > Please update jit.dg/all-non-failing-tests.h for the new tests; > > > > it's > > > > meant to list all of the (non failing) tests alphabetically. > > >=20 > > > It's not always correctly sorted. Might be worth sending a patch > > > after this > > > one gets merged to fix that. > > >=20 > > > > I *think* all of the new tests aren't suitable to be run as > > > > part of > > > > a > > > > shared context (e.g. due to touching the optimization level or > > > > examining generated asm), so they should be listed in that > > > > header > > > > with > > > > comments explaining why. > > >=20 > > > I added them with a comment on top of each of them. > > >=20 > > > I joined the new patch version. > > >=20 > > > Thanks again for the review! > >=20 > > Thanks for the updated patch.=C2=A0 I noticed a few minor issues: > >=20 > > > diff --git a/gcc/jit/docs/topics/types.rst > > > b/gcc/jit/docs/topics/types.rst > > > index bb51f037b7e..b1aedc03787 100644 > > > --- a/gcc/jit/docs/topics/types.rst > > > +++ b/gcc/jit/docs/topics/types.rst > > > @@ -553,3 +553,80 @@ Reflection API > > > =C2=A0=C2=A0=C2=A0 .. code-block:: c > > >=20 > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 #ifdef LIBGCCJIT_HAVE_gcc_jit_ty= pe_get_restrict > > > + > > > +.. function::=C2=A0 void\ > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 gcc_jit_function_add_attribute (gcc_jit_function > > > *func, > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 enum > > > gcc_jit_fn_attribute attribute) > > > + > > > +=C2=A0=C2=A0=C2=A0=C2=A0 Add an attribute ``attribute`` to a functio= n ``func``. > > > + > > > +=C2=A0=C2=A0=C2=A0=C2=A0 This is equivalent to the following code: > > > + > > > +=C2=A0 .. code-block:: c > > > + > > > +=C2=A0=C2=A0=C2=A0 __attribute__((always_inline)) > > > + > > > +=C2=A0=C2=A0 This entrypoint was added in :ref:`LIBGCCJIT_ABI_26`; y= ou can > > > test for > > > +=C2=A0=C2=A0 its presence using > > > + > > > +=C2=A0=C2=A0 .. code-block:: c > > > + > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 #ifdef LIBGCCJIT_HAVE_ATTRIBUTES > > > + > > > +.. function::=C2=A0 void\ > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 gcc_jit_function_add_string_attribute > > > (gcc_jit_function *func, > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0 enum > > > gcc_jit_fn_attribute attribute, > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0 const char > > > *value) > > > + > > > +=C2=A0=C2=A0=C2=A0=C2=A0 Add a string attribute ``attribute`` with v= alue ``value`` > > > to a function > > > +=C2=A0=C2=A0=C2=A0=C2=A0 ``func``. > > > + > > > +=C2=A0=C2=A0=C2=A0=C2=A0 This is equivalent to the following code: > > > + > > > +=C2=A0 .. code-block:: c > > > + > > > +=C2=A0=C2=A0=C2=A0 __attribute__ ((alias ("xxx"))) > > > + > > > +=C2=A0=C2=A0 This entrypoint was added in :ref:`LIBGCCJIT_ABI_26`; y= ou can > > > test for > > > +=C2=A0=C2=A0 its presence using > > > + > > > +=C2=A0=C2=A0 .. code-block:: c > > > + > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 #ifdef LIBGCCJIT_HAVE_ATTRIBUTES > > > + > > > +.. function::=C2=A0 void\ > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 gcc_jit_function_add_integer_array_attribute > > > (gcc_jit_function *func, > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 > > > enum gcc_jit_fn_attribute attribute, > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 > > > const int *value, > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 > > > size_t length) > > > + > > > +=C2=A0=C2=A0=C2=A0=C2=A0 Add an attribute ``attribute`` with ``lengt= h`` integer > > > values ``value`` to a > > > +=C2=A0=C2=A0=C2=A0=C2=A0 function ``func``. The integer values must = be the same as > > > you would write > > > +=C2=A0=C2=A0=C2=A0=C2=A0 them in a C code. > > > + > > > +=C2=A0=C2=A0=C2=A0=C2=A0 This is equivalent to the following code: > > > + > > > +=C2=A0 .. code-block:: c > > > + > > > +=C2=A0=C2=A0=C2=A0 __attribute__ ((nonnull (1, 2))) > > > + > > > +=C2=A0=C2=A0 This entrypoint was added in :ref:`LIBGCCJIT_ABI_26`; y= ou can > > > test for > > > +=C2=A0=C2=A0 its presence using > > > + > > > +=C2=A0=C2=A0 .. code-block:: c > > > + > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 #ifdef LIBGCCJIT_HAVE_ATTRIBUTES > > > + > > > +.. function::=C2=A0 void\ > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 gcc_jit_lvalue_add_string_attribute > > > (gcc_jit_lvalue *variable, > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0 enum > > > gcc_jit_variable_attribute attribute, > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0 const char > > > *value) > > > + > > > +=C2=A0=C2=A0=C2=A0=C2=A0 Add an attribute ``attribute`` with value `= `value`` to a > > > variable ``variable``. > > > + > > > +=C2=A0=C2=A0 This entrypoint was added in :ref:`LIBGCCJIT_ABI_26`; y= ou can > > > test for > > > +=C2=A0=C2=A0 its presence using > > > + > > > +=C2=A0=C2=A0 .. code-block:: c > > > + > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 #ifdef LIBGCCJIT_HAVE_ATTRIBUTES > >=20 > > The above looks correct, but the patch adds the entrypoint > > descriptions > > to topics/types.rst, which seems like the wrong place.=C2=A0 The > > function- > > related ones should be in topics/functions.rst in the "Functions" > > section and the lvalue/variable one in topics/expression.rst after > > the > > "Global variables" section. > >=20 > > > diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h > > b/gcc/testsuite/jit.dg/all-non-failing-tests.h > > > index e762563f9bd..84001203352 100644 > > > --- a/gcc/testsuite/jit.dg/all-non-failing-tests.h > > > +++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h > >=20 > > [...snip...] > >=20 > > > @@ -313,7 +334,7 @@ > > > =C2=A0#undef create_code > > > =C2=A0#undef verify_code > > >=20 > > > -/* test-restrict.c: This can't be in the testcases array as it > > > needs > > > +/* test-restrict-attribute.c: This can't be in the testcases > > > array as it needs > > > =C2=A0=C2=A0=C2=A0 the `-O3` flag.=C2=A0 */ > >=20 > > test-restrict.c is a pre-existing testcase, so please don't delete > > its > > entry. > > BTW, the ChangeLog entry mentions adding test-restrict.c, but the > > patch > > doesn't add it, so that part of the proposed ChangeLog is wrong. > >=20 > > Does the patch pass ./contrib/gcc-changelog/git_check_commit.py ? > >=20 > > [...snip...] > >=20 > > > diff --git a/gcc/testsuite/jit.dg/test-cold-attribute.c > > > b/gcc/testsuite/jit.dg/test-cold-attribute.c > > > new file mode 100644 > > > index 00000000000..8dc7ec5a34b > > > --- /dev/null > > > +++ b/gcc/testsuite/jit.dg/test-cold-attribute.c > > > @@ -0,0 +1,54 @@ > > > +/* { dg-do compile { target x86_64-*-* } } */ > > > + > > > +#include > > > +#include > > > + > > > +#include "libgccjit.h" > > > + > > > +/* We don't want set_options() in harness.h to set -O2 to see > > > that the cold > > > +=C2=A0=C2=A0 attribute affects the optimizations. */ > >=20 > > Please delete the above comment. > >=20 > > > +#define TEST_ESCHEWS_SET_OPTIONS > > > +static void set_options (gcc_jit_context *ctxt, const char > > > *argv0) > > > +{ > > > +=C2=A0 // Set "-O2". > > > +=C2=A0 gcc_jit_context_set_int_option(ctxt, > > > GCC_JIT_INT_OPTION_OPTIMIZATION_LEVEL, 2); > > > +} > >=20 > > [...snip...] > >=20 > > > diff --git a/gcc/testsuite/jit.dg/test-const-attribute.c > > > b/gcc/testsuite/jit.dg/test-const-attribute.c > > > new file mode 100644 > > > index 00000000000..c06742d163f > > > --- /dev/null > > > +++ b/gcc/testsuite/jit.dg/test-const-attribute.c > > > @@ -0,0 +1,134 @@ > > > +/* { 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 const > > > +=C2=A0=C2=A0 attribute affects the optimizations. */ > >=20 > > Please delete the above comment. > >=20 > > > +#define TEST_ESCHEWS_SET_OPTIONS > > > +static void set_options (gcc_jit_context *ctxt, const char > > > *argv0) > > > +{ > > > +=C2=A0 // Set "-O3". > > > +=C2=A0 gcc_jit_context_set_int_option(ctxt, > > > GCC_JIT_INT_OPTION_OPTIMIZATION_LEVEL, 3); > > > +} > >=20 > > [...snip...] > >=20 > > > diff --git a/gcc/testsuite/jit.dg/test-noinline-attribute.c > > > b/gcc/testsuite/jit.dg/test-noinline-attribute.c > > > new file mode 100644 > > > index 00000000000..a455b4493fd > > > --- /dev/null > > > +++ b/gcc/testsuite/jit.dg/test-noinline-attribute.c > > > @@ -0,0 +1,121 @@ > > > +/* { dg-do compile { target x86_64-*-* } } */ > > > + > > > +#include > > > +#include > > > + > > > +#include "libgccjit.h" > > > + > > > +/* We don't want set_options() in harness.h to set -O2 to see > > > that the `noinline` > > > +=C2=A0=C2=A0 attribute affects the optimizations. */ > >=20 > > Please delete the above comment. > >=20 > > > +#define TEST_ESCHEWS_SET_OPTIONS > > > +static void set_options (gcc_jit_context *ctxt, const char > > > *argv0) > > > +{ > > > +=C2=A0 // Set "-O2". > > > +=C2=A0 gcc_jit_context_set_int_option(ctxt, > > > GCC_JIT_INT_OPTION_OPTIMIZATION_LEVEL, 2); > > > +} > >=20 > > [...snip...] > >=20 > > > diff --git a/gcc/testsuite/jit.dg/test-nonnull-attribute.c > > > b/gcc/testsuite/jit.dg/test-nonnull-attribute.c > > > new file mode 100644 > > > index 00000000000..3306f890657 > > > --- /dev/null > > > +++ b/gcc/testsuite/jit.dg/test-nonnull-attribute.c > > > @@ -0,0 +1,94 @@ > > > +/* { dg-do compile { target x86_64-*-* } } */ > > > + > > > +#include > > > +#include > > > + > > > +#include "libgccjit.h" > > > + > > > +/* We don't want set_options() in harness.h to set -O2 to see > > > that the nonnull > > > +=C2=A0=C2=A0 attribute affects the optimizations. */ > >=20 > > Please delete the above comment. > >=20 > >=20 > > > +#define TEST_ESCHEWS_SET_OPTIONS > > > +static void set_options (gcc_jit_context *ctxt, const char > > > *argv0) > > > +{ > > > +=C2=A0 // Set "-O2". > > > +=C2=A0 gcc_jit_context_set_int_option(ctxt, > > > GCC_JIT_INT_OPTION_OPTIMIZATION_LEVEL, 2); > > > +} > >=20 > > [...snip...] > >=20 > > > diff --git a/gcc/testsuite/jit.dg/test-pure-attribute.c > > > b/gcc/testsuite/jit.dg/test-pure-attribute.c > > > new file mode 100644 > > > index 00000000000..0c9ba1366e0 > > > --- /dev/null > > > +++ b/gcc/testsuite/jit.dg/test-pure-attribute.c > > > @@ -0,0 +1,134 @@ > > > +/* { 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 pure > > > +=C2=A0=C2=A0 attribute affects the optimizations. */ > >=20 > > Please delete the above comment. > >=20 > > > +#define TEST_ESCHEWS_SET_OPTIONS > > > +static void set_options (gcc_jit_context *ctxt, const char > > > *argv0) > > > +{ > > > +=C2=A0 // Set "-O3". > > > +=C2=A0 gcc_jit_context_set_int_option(ctxt, > > > GCC_JIT_INT_OPTION_OPTIMIZATION_LEVEL, 3); > > > +} > > > + > >=20 > > [...snip...] > >=20 > > > diff --git a/gcc/testsuite/jit.dg/test-restrict-attribute.c > > > b/gcc/testsuite/jit.dg/test-restrict-attribute.c > > > new file mode 100644 > > > index 00000000000..7d7444b624f > > > --- /dev/null > > > +++ b/gcc/testsuite/jit.dg/test-restrict-attribute.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 restrict > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 attribute affects the optimizations. = */ > >=20 > > Please delete this comment. > >=20 > > > +#define TEST_ESCHEWS_SET_OPTIONS > > > +static void set_options (gcc_jit_context *ctxt, const char > > > *argv0) > > > +{ > > > +=C2=A0=C2=A0=C2=A0=C2=A0 // Set "-O3". > > > +=C2=A0=C2=A0=C2=A0=C2=A0 gcc_jit_context_set_int_option(ctxt, > > > GCC_JIT_INT_OPTION_OPTIMIZATION_LEVEL, 3); > > > +} > > > + > >=20 > > [...snip...] > >=20 > > Otherwise, looks good, assuming that the patch has been tested with > > the > > full jit testsuite. > >=20 > > Thanks again > > Dave > >=20 >=20