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 ESMTP id 15DD2383603B for ; Thu, 10 Jun 2021 22:41:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 15DD2383603B Received: from mail-qv1-f71.google.com (mail-qv1-f71.google.com [209.85.219.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-294-uKukZhTgN8yIqVPfCdQstg-1; Thu, 10 Jun 2021 18:41:17 -0400 X-MC-Unique: uKukZhTgN8yIqVPfCdQstg-1 Received: by mail-qv1-f71.google.com with SMTP id i16-20020a0cf4900000b029022023514900so16847830qvm.11 for ; Thu, 10 Jun 2021 15:41:17 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to :references:user-agent:mime-version:content-transfer-encoding; bh=MLexIjf/e+hd03vC7sjOIedLnghmRIkwn+Y+lnojrx8=; b=GFDwA+JDo+0e7MQzW6Zu1K6wV45OXtfm3Um6Af7wquiwxkMvJ4lCaPZ3xLd1Tu6bCH 3N+UBH9Y+dhfL4tRODDlBtnP9JFaCbse1/Ftmp7doKKH+p5T1R7HS8ovJl2GRQ0Pam/V r+EzqBJWmeL43hzVhpCDe2Iphyy6Fu39yf1KzB+HVTprawQnq/VDleSlhPrsvJB1pv3+ bOPZjdwysFNMdkMS+EzL0O+dJ51uVN071vw1FEQOw8wOE4b3Wegf1r2Ob+CsHES2IMW7 q/8g1SB4wxMuYa7v3zFspc4jMJamVYf+Hevs/tv3yr61oKDseOEGbHOlJ6pPNh/G4Fx9 iY7w== X-Gm-Message-State: AOAM530Arm9EB94Rb2U/ZzOrY214e+fVgBAx53iqX5p0HICF2csVbUl5 8NYarffK56sEGPXI273prs7oMs92s8rrEcQx7Zrnw7SgA0anXq3ni2q9ZCZhXIymq6bVt26OOKi 0F5vBurk= X-Received: by 2002:a37:e44:: with SMTP id 65mr980062qko.283.1623364877308; Thu, 10 Jun 2021 15:41:17 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyryv5yC+5lVWLyLz82mjP+a+Y9H4SkYsyMkK45HoPNW8QERIbrXjtc778RglHIP4lue25hIA== X-Received: by 2002:a37:e44:: with SMTP id 65mr980050qko.283.1623364877108; Thu, 10 Jun 2021 15:41:17 -0700 (PDT) Received: from t14s.localdomain (c-73-69-212-193.hsd1.ma.comcast.net. [73.69.212.193]) by smtp.gmail.com with ESMTPSA id l18sm3106470qtq.85.2021.06.10.15.41.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 10 Jun 2021 15:41:16 -0700 (PDT) Message-ID: Subject: Re: [PATCH] libgccjit: add some reflection functions in the jit C api From: David Malcolm To: Antoni Boucher Cc: Andrea Corallo , Antoni Boucher via Jit , gcc-patches@gcc.gnu.org Date: Thu, 10 Jun 2021 18:41:15 -0400 In-Reply-To: References: <20200902010120.crnx55ev635ceiey@bouanto-desktop.localdomain> <6fe2ea355403eb177c9632e076c11c792f23c791.camel@redhat.com> <9cd00fe5cb5c03184e3a5bd939447b30450a8ca7.camel@redhat.com> <20201015160226.cka4iq3w22jztopm@bouanto-desktop.localdomain> <20201015173952.ft7mu5ajbaxb3gke@bouanto-desktop.localdomain> <54ba5c58dbd2b8c7b1f3276c2b87cddf55e258bd.camel@redhat.com> <20201103221324.xw3jbvrw2uwdc4rz@bouanto-desktop.localdomain> <3388bb8c84e68cd7b0698dc154e7a5666c0d2cde.camel@redhat.com> <0e8b6450bcb23182b85342d8010c3bea0c297ba2.camel@zoho.com> <534254132a841b75d555a52ce952f84418f168c2.camel@redhat.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: 7bit X-Spam-Status: No, score=-12.0 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_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=unavailable autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: Thu, 10 Jun 2021 22:41:21 -0000 On Thu, 2021-05-27 at 21:51 -0400, Antoni Boucher wrote: > I chose option A, so everything is a size_t, now. > I also renamed the dyncast functions. > Here's the new patch. Thanks, sorry again about the delays in reviewing your work. You didn't specify how you tested the patch; are you running the full jit regression test suite? This is looking close to done; a few nits inline below: [...snip...] > diff --git a/gcc/jit/docs/topics/functions.rst b/gcc/jit/docs/topics/functions.rst > index b2d9239aa0a..1d20e3045a0 100644 > --- a/gcc/jit/docs/topics/functions.rst > +++ b/gcc/jit/docs/topics/functions.rst [...snip...] > + The API entrypoints relating to getting info about parameters and return > + types: > + > + * :c:func:`gcc_jit_function_get_return_type` > + > + * :c:func:`gcc_jit_function_get_param_count` > + > + were added in :ref:`LIBGCCJIT_ABI_15`; you can test for their presence > + using 16, rather than 15. > + The API entrypoints related to the reflection API: > + > + * :c:func:`gcc_jit_function_type_get_return_type` > + [...snip...] > + > + * :c:func:`gcc_jit_struct_get_field_count` > + > + were added in :ref:`LIBGCCJIT_ABI_15`; you can test for their presence > + using 16, rather than 15 again. > + > + .. code-block:: c > + > + #ifdef LIBGCCJIT_HAVE_REFLECTION > + > + .. type:: gcc_jit_case [...snip...] > +gcc_jit_type * > +gcc_jit_function_type_get_param_type (gcc_jit_function_type *function_type, > + size_t index) > +{ > + RETURN_NULL_IF_FAIL (function_type, NULL, NULL, "NULL function_type"); > + size_t num_params = function_type->get_param_types ().length (); > + gcc::jit::recording::context *ctxt = function_type->m_ctxt; > + RETURN_NULL_IF_FAIL_PRINTF3 (index < num_params, > + ctxt, NULL, > + "index of %ld is too large (%s has %ld params)", > + index, > + function_type->get_debug_string (), > + num_params); > + return (gcc_jit_type *)function_type->get_param_types ()[index]; > +} > + I'm retaining the above, since... [...snip...] > > + > +/* Public entrypoint. See description in libgccjit.h. > + > + After error-checking, the real work is done by the > + gcc::jit::recording::fields::get_field method in > + jit-recording.c. */ > +extern gcc_jit_field * > +gcc_jit_struct_get_field (gcc_jit_struct *struct_type, > + size_t index) > +{ > + RETURN_NULL_IF_FAIL (struct_type, NULL, NULL, "NULL struct type"); > + RETURN_NULL_IF_FAIL (struct_type->get_fields (), NULL, NULL, > + "NULL struct fields"); > + RETURN_NULL_IF_FAIL ((int) index < struct_type->get_fields ()->length (), > + NULL, NULL, "NULL struct type"); ...copy&paste error here; the message for this kind of failure needs updating. Do it in a similar way to how you did gcc_jit_function_type_get_param_type above, using the ctxt of the struct_type. [...snip...] > +#define LIBGCCJIT_HAVE_REFLECTION > + > +/* Reflection functions to get the number of parameters, return type of > + a function and whether a type is a bool from the C API. > + > + This API entrypoint was added in LIBGCCJIT_ABI_15; you can test for its 16, rather than 15, again. > + presence using > + #ifdef LIBGCCJIT_HAVE_REFLECTION [...snip...] OK for trunk with the above nits fixed, assuming that you have run the full regression test suite (or do you need help with that?) I can't remember, sorry, do you have push rights to the gcc git repository? Do you have a preference as to which patch you want me to look at next? Otherwise I'll go through them in the order in https://github.com/antoyo/rustc_codegen_gcc/tree/master/gcc-patches Dave