From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by sourceware.org (Postfix) with ESMTPS id 70FB53896C0E; Thu, 13 May 2021 08:34:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 70FB53896C0E Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mliska@suse.cz X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 62284AD9F; Thu, 13 May 2021 08:34:37 +0000 (UTC) Subject: Re: [PATCH] libgccjit: Handle truncation and extension for casts [PR 95498] To: Antoni Boucher , Tom Tromey , David Malcolm Cc: jit@gcc.gnu.org, Antoni Boucher via Gcc-patches References: <20200713003002.bs5hwv4gav6ml5rt@bouanto-laptop.localdomain> <874ki6txcc.fsf@tromey.com> <20210220221706.ewfq2b3i2mzx4ice@bouanto-desktop.localdomain> From: =?UTF-8?Q?Martin_Li=c5=a1ka?= Message-ID: <8e63c261-8daf-af1f-a552-88e1d1d09f83@suse.cz> Date: Thu, 13 May 2021 10:34:36 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 MIME-Version: 1.0 In-Reply-To: <20210220221706.ewfq2b3i2mzx4ice@bouanto-desktop.localdomain> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-3.8 required=5.0 tests=BAYES_00, BODY_8BITS, KAM_DMARC_STATUS, KAM_SHORT, NICE_REPLY_A, RCVD_IN_MSPIKE_H3, 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 08:34:39 -0000 @David: PING On 2/20/21 11:17 PM, Antoni Boucher via Gcc-patches wrote: > Hi. > Thanks for your feedback! > > See answers below: > > On Sat, Feb 20, 2021 at 11:20:35AM -0700, Tom Tromey wrote: >>>>>>> "Antoni" == Antoni Boucher via Gcc-patches 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) >> 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. > >> 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!