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 [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id 5A7EA3857022 for ; Thu, 13 May 2021 22:13:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 5A7EA3857022 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-319-uk39p51qN5q-nZlJIv_VNQ-1; Thu, 13 May 2021 18:13:16 -0400 X-MC-Unique: uk39p51qN5q-nZlJIv_VNQ-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id A9FAA8018A1; Thu, 13 May 2021 22:13:15 +0000 (UTC) Received: from t14s.localdomain (ovpn-112-65.phx2.redhat.com [10.3.112.65]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8D6105C290; Thu, 13 May 2021 22:13:14 +0000 (UTC) Message-ID: <3ae55b5acc33b38c579ac9c293993c29adc55ae9.camel@redhat.com> Subject: Re: [PATCH] libgccjit: Handle truncation and extension for casts [PR 95498] From: David Malcolm To: Antoni Boucher , Tom Tromey Cc: jit@gcc.gnu.org, Antoni Boucher via Gcc-patches Date: Thu, 13 May 2021 18:13:13 -0400 In-Reply-To: <20210220221706.ewfq2b3i2mzx4ice@bouanto-desktop.localdomain> References: <20200713003002.bs5hwv4gav6ml5rt@bouanto-laptop.localdomain> <874ki6txcc.fsf@tromey.com> <20210220221706.ewfq2b3i2mzx4ice@bouanto-desktop.localdomain> User-Agent: Evolution 3.38.4 (3.38.4-1.fc33) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 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=-6.1 required=5.0 tests=BAYES_00, BODY_8BITS, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, KAM_SHORT, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham 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, 13 May 2021 22:13:21 -0000 On Sat, 2021-02-20 at 17:17 -0500, Antoni Boucher via Gcc-patches wrote: > Hi. > Thanks for your feedback! > Sorry about the delay in responding. In the past I was hesitant about adding more cast support to libgccjit since I felt that the user could always just create a union to do the cast. Then I tried actually using the libgccjit API to do this, and realized how much work it adds, so I now think we do want to support casting more types. > See answers below: > > On Sat, Feb 20, 2021 at 11:20:35AM -0700, Tom Tromey wrote: > > > > > > > "Antoni" == Antoni Boucher via Gcc-patches < > > > > > > > gcc-patches@gcc.gnu.org> writes: > > > > Antoni> gcc/jit/ > > Antoni>         PR target/95498 > > Antoni>         * jit-playback.c: Add support to handle truncation > > and extension > > Antoni>         in the convert function. > > > > Antoni> +  switch (dst_code) > > Antoni> +    { > > Antoni> +    case INTEGER_TYPE: > > Antoni> +    case ENUMERAL_TYPE: > > Antoni> +      t_ret = convert_to_integer (dst_type, expr); > > Antoni> +      goto maybe_fold; > > Antoni> + > > Antoni> +    default: > > Antoni> +      gcc_assert (gcc::jit::active_playback_ctxt); > > Antoni> +      gcc::jit::active_playback_ctxt->add_error (NULL, > > "unhandled conversion"); > > Antoni> +      fprintf (stderr, "input expression:\n"); > > Antoni> +      debug_tree (expr); > > Antoni> +      fprintf (stderr, "requested type:\n"); > > Antoni> +      debug_tree (dst_type); > > Antoni> +      return error_mark_node; > > Antoni> + > > Antoni> +    maybe_fold: > > Antoni> +      if (TREE_CODE (t_ret) != C_MAYBE_CONST_EXPR) Do we even get C_MAYBE_CONST_EXPR in libgccjit? That tree code is defined in c-family/c-common.def; how can nodes of that kind be created outside of the c-family? > > Antoni> +       t_ret = fold (t_ret); > > Antoni> +      return t_ret; > > > > It seems weird to have a single 'goto' to maybe_fold, especially > > inside > > a switch like this. > > > > If you think the maybe_fold code won't be reused, then it should just > > be > > hoisted up and the 'goto' removed. > > This actually depends on how the support for cast between integers and > pointers will be implemented (see below). > If we will support truncating pointers (does that even make sense? and > I > guess we cannot extend a pointer unless we add the support for > uint128_t), that label will be reused for that case. > Otherwise, it might not be reused. > > So, please tell me which option to choose and I'll update my patch. FWIW I don't think we'll want to support truncating or extending pointers. > > > On the other hand, if the maybe_fold code might be reused for some > > other > > case, then I suppose I would have the case end with 'break' and then > > have this code outside the switch. > > > > > > In another message, you wrote: > > > > Antoni> For your question, the current code already works with > > boolean and > > Antoni> reals and casts between integers and pointers is currently > > not > > Antoni> supported. > > > > I am curious why this wasn't supported.  It seems like something that > > one might want to do. > > I have no idea as this is my first contribution to gcc. > But this would be indeed very useful and I opened an issue about this: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95438 > > > thanks, > > Tom > > Thanks! >