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 C5C9C3857C4A for ; Fri, 8 Apr 2022 19:22:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C5C9C3857C4A Received: from mail-qv1-f69.google.com (mail-qv1-f69.google.com [209.85.219.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-637-E9jwYk6INbatt1d8IrgsPg-1; Fri, 08 Apr 2022 15:22:13 -0400 X-MC-Unique: E9jwYk6INbatt1d8IrgsPg-1 Received: by mail-qv1-f69.google.com with SMTP id kj4-20020a056214528400b0044399a9bb4cso10113212qvb.15 for ; Fri, 08 Apr 2022 12:22:13 -0700 (PDT) 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=BrYozbg2L1OdTx2L2BAn5OFtky9jYz6nwysf26SjQGY=; b=WBYHahQTg3f952TvGEJfpmK/D50doM7XHiNqOCvLSo4h8SgFgv1oJWy1Yg7JGNHyDt B1N1QNRPVSixAHKxq4s9QEvoFiBWF272AHgpE8K8r3v+OPniDpi/uzqKP3T1m23gayoC h6DMIfOqq0RJrfVf94Ge0jj+HOxov+v2ri06feqDTjpRkEJDpmX1QJictJ9viTMwCTND nqc8pYA8XLh/ozJhO6cdRtiV69K6XHZcPMyl9QnluPfiFVmGI/U+PK2lYiOE3x/kabDN JYt6x8STshKdeiKE6oFBAKuV0aAquwITUARApmITbu1UMQIzSENj0H+brJIEmmWsbFWh Srag== X-Gm-Message-State: AOAM530cfFPUMadZdnGbtIg3zJ0NDK0kpjFx0vxTQDs/2n6rZOc5iyPz mLv52buxEJp8M2IXJxT/mWt54WW4WQ/2rUwzMcGyV66Ta7/oZU8SIvI4myRIvpjm+JJ6Gji804f 4/Jta/Tg= X-Received: by 2002:a05:622a:124b:b0:2eb:c9d4:7208 with SMTP id z11-20020a05622a124b00b002ebc9d47208mr11124766qtx.166.1649445732974; Fri, 08 Apr 2022 12:22:12 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyyO0nh19uejP2jCpBL3+PaCZygbeu6+ldg4j3+uiXB6JWQMid7mHfRKg3s/qr4Uz4elE4yOQ== X-Received: by 2002:a05:622a:124b:b0:2eb:c9d4:7208 with SMTP id z11-20020a05622a124b00b002ebc9d47208mr11124744qtx.166.1649445732614; Fri, 08 Apr 2022 12:22:12 -0700 (PDT) Received: from t14s.localdomain (c-73-69-212-193.hsd1.nh.comcast.net. [73.69.212.193]) by smtp.gmail.com with ESMTPSA id e7-20020ac85987000000b002e1b7fa2201sm18871180qte.56.2022.04.08.12.22.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 08 Apr 2022 12:22:12 -0700 (PDT) Message-ID: Subject: Re: [PATCH] libgccjit: Add support for bitcasts [PR104071] From: David Malcolm To: Antoni Boucher , gcc-patches@gcc.gnu.org, jit@gcc.gnu.org Date: Fri, 08 Apr 2022 15:22:11 -0400 In-Reply-To: <1cb2b6d3e9a1afb6135a9dc1a97e85ba8c3a4a0d.camel@zoho.com> References: <13bdd05cfd7006a332ec785ab371356f0354db4d.camel@zoho.com> <19fe391c6c77e0e38acdcaf7e864651849913d46.camel@zoho.com> <1cb2b6d3e9a1afb6135a9dc1a97e85ba8c3a4a0d.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: 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_NONE, TXREP, T_SCC_BODY_TEXT_LINE 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: Fri, 08 Apr 2022 19:22:17 -0000 On Fri, 2022-01-21 at 18:41 -0500, Antoni Boucher wrote: > Hi. > Here's the updated patch. > Thanks. Review below: [...snip...] > diff --git a/gcc/jit/libgccjit.cc b/gcc/jit/libgccjit.cc > index 4c352e8c93d..6bf1e1ceee0 100644 > --- a/gcc/jit/libgccjit.cc > +++ b/gcc/jit/libgccjit.cc > @@ -2405,6 +2405,34 @@ gcc_jit_context_new_cast (gcc_jit_context *ctxt, > return static_cast (ctxt->new_cast (loc, rvalue, type)); > } > > +/* Public entrypoint. See description in libgccjit.h. > + > + After error-checking, the real work is done by the > + gcc::jit::recording::context::new_bitcast method in jit-recording.c. */ > + > +gcc_jit_rvalue * > +gcc_jit_context_new_bitcast (gcc_jit_context *ctxt, > + gcc_jit_location *loc, > + gcc_jit_rvalue *rvalue, > + gcc_jit_type *type) > +{ > + RETURN_NULL_IF_FAIL (ctxt, NULL, loc, "NULL context"); > + JIT_LOG_FUNC (ctxt->get_logger ()); > + /* LOC can be NULL. */ > + RETURN_NULL_IF_FAIL (rvalue, ctxt, loc, "NULL rvalue"); > + RETURN_NULL_IF_FAIL (type, ctxt, loc, "NULL type"); > + // TODO: check the sizes. > + /*RETURN_NULL_IF_FAIL_PRINTF3 ( > + is_valid_cast (rvalue->get_type (), type), > + ctxt, loc, > + "cannot cast %s from type: %s to type: %s", > + rvalue->get_debug_string (), > + rvalue->get_type ()->get_debug_string (), > + type->get_debug_string ());*/ I think we agreed that we can't check the sizes at this point, so this commented-out check would be better replaced with a comment explaining that we have to defer the check to playback time, when we have the trees. > + > + return static_cast (ctxt->new_bitcast (loc, rvalue, type)); > +} > + > /* Public entrypoint. See description in libgccjit.h. > > After error-checking, the real work is done by the [...snip...] > diff --git a/gcc/testsuite/jit.dg/test-bitcast.c b/gcc/testsuite/jit.dg/test-bitcast.c > new file mode 100644 > index 00000000000..a092fa117e6 > --- /dev/null > +++ b/gcc/testsuite/jit.dg/test-bitcast.c > @@ -0,0 +1,60 @@ > +#include > +#include > +#include > + > +#include "libgccjit.h" > + > +#include "harness.h" > + > +void > +create_code (gcc_jit_context *ctxt, void *user_data) > +{ > + /* Let's try to inject the equivalent of: > +int > +my_bitcast (double x) > +{ > + return bitcast(x, int); > +} > + */ > + gcc_jit_type *int_type = > + gcc_jit_context_get_int_type (ctxt, 4, 1); > + gcc_jit_type *float_type = > + gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_FLOAT); This uses GCC_JIT_TYPE_FLOAT for the param... > + > + gcc_jit_param *x = > + gcc_jit_context_new_param ( > + ctxt, > + NULL, > + float_type, "x"); > + gcc_jit_param *params[1] = {x}; > + gcc_jit_function *func = > + gcc_jit_context_new_function (ctxt, > + NULL, > + GCC_JIT_FUNCTION_EXPORTED, > + int_type, > + "my_bitcast", > + 1, params, 0); [..snip...] > + > +void > +verify_code (gcc_jit_context *ctxt, gcc_jit_result *result) > +{ > + typedef int (*my_bitcast_fn_type) (double); ...but this uses "double". Presumably these should agree, and have the same sizeof as the integer type. > + CHECK_NON_NULL (result); > + my_bitcast_fn_type my_bitcast = > + (my_bitcast_fn_type)gcc_jit_result_get_code (result, "my_bitcast"); > + CHECK_NON_NULL (my_bitcast); > + int val = my_bitcast (-5.1298714); > + note ("my_bitcast returned: %d", val); > + CHECK_VALUE (val, 35569201); Out of curiosity, is there any particular significance for these values? FWIW I rather like: http://evanw.github.io/float-toy/ for directly manipulating the bits of floating point numbers. [...snip...] > diff --git a/gcc/toplev.cc b/gcc/toplev.cc > index 534da1462e8..bc4921974eb 100644 > --- a/gcc/toplev.cc > +++ b/gcc/toplev.cc > @@ -2368,6 +2368,7 @@ toplev::finalize (void) > gcse_c_finalize (); > ipa_cp_c_finalize (); > ira_costs_c_finalize (); > + tree_cc_finalize (); > > /* save_decoded_options uses opts_obstack, so these must > be cleaned up together. */ > diff --git a/gcc/tree.cc b/gcc/tree.cc > index ae159ee20ce..fe9d9083026 100644 > --- a/gcc/tree.cc > +++ b/gcc/tree.cc > @@ -6963,6 +6963,15 @@ build_reference_type (tree to_type) > (HOST_BITS_PER_WIDE_INT > 64 ? HOST_BITS_PER_WIDE_INT : 64) > static GTY(()) tree nonstandard_integer_type_cache[2 * MAX_INT_CACHED_PREC + 2]; > > +static void > +clear_nonstandard_integer_type_cache (void) > +{ > + for (size_t i = 0 ; i < 2 * MAX_INT_CACHED_PREC + 2 ; i++) > + { > + nonstandard_integer_type_cache[i] = NULL; > + } > +} > + > /* Builds a signed or unsigned integer type of precision PRECISION. > Used for C bitfields whose precision does not match that of > built-in target types. */ > @@ -14565,6 +14574,12 @@ get_attr_nonstring_decl (tree expr, tree *ref) > return NULL_TREE; > } > > +void > +tree_cc_finalize (void) > +{ > + clear_nonstandard_integer_type_cache (); > +} > + > #if CHECKING_P > > namespace selftest { > diff --git a/gcc/tree.h b/gcc/tree.h > index 30bc53c2996..bf886fc2472 100644 > --- a/gcc/tree.h > +++ b/gcc/tree.h > @@ -5385,6 +5385,7 @@ extern bool real_minus_onep (const_tree); > extern void init_ttree (void); > extern void build_common_tree_nodes (bool); > extern void build_common_builtin_nodes (void); > +extern void tree_cc_finalize (void); > extern tree build_nonstandard_integer_type (unsigned HOST_WIDE_INT, int); > extern tree build_nonstandard_boolean_type (unsigned HOST_WIDE_INT); > extern tree build_range_type (tree, tree, tree); Looks OK to me, but am not officially a maintainer of these parts. LGTM with those nits fixed - for next stage 1, or for trunk now if the release maintainers are OK with it. Thanks again for the patch, and sorry about the belated review. Dave