From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out2.suse.de (smtp-out2.suse.de [IPv6:2001:67c:2178:6::1d]) by sourceware.org (Postfix) with ESMTPS id 1E7903858D37 for ; Tue, 9 May 2023 10:26:04 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 1E7903858D37 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id 076D91F45A; Tue, 9 May 2023 10:26:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1683627962; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=J0SusQIfSGf39bxP/fNwLt07XlcY+gpk/VNtfzheqP8=; b=NaZ4rjhOW7mPpUPJfypl8phAQWTA7CHnl+IqgyWm7eQAaNK9FS1PETD5z/OGDYB+YVVKbJ cktk98WLb42GcUA14yBC3lO8NQTdVckp0oljcM7lOeUhoSpJYpRYy8h15WB5Ipn08ApC/o Ooi4adaRPuMdGoE/qR1vqKTWlTQNlEo= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1683627962; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=J0SusQIfSGf39bxP/fNwLt07XlcY+gpk/VNtfzheqP8=; b=UeYyDjeyZvPghzQsxgob5/jWcqIJRgyLmhCLbCdJvhYyL3Nwtouz326+AxktKksd+bX+qz l6x19CzvSQg2CBBg== Received: from wotan.suse.de (wotan.suse.de [10.160.0.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id 939692C143; Tue, 9 May 2023 10:26:01 +0000 (UTC) Date: Tue, 9 May 2023 10:26:01 +0000 (UTC) From: Richard Biener To: Richard Sandiford cc: "Li, Pan2" , Jeff Law , Kito Cheng , "juzhe.zhong@rivai.ai" , gcc-patches , palmer , jakub Subject: Re: [PATCH] machine_mode type size: Extend enum size from 8-bit to 16-bit In-Reply-To: Message-ID: References: <20230410144808.324346-1-juzhe.zhong@rivai.ai> <436847c8-0c15-24de-5925-f56d78caf540@gmail.com> User-Agent: Alpine 2.22 (LSU 394 2020-01-19) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-5.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS,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 Tue, 9 May 2023, Richard Sandiford wrote: > "Li, Pan2" writes: > > After the bits patch like below. > > > > rtx_def code 16 => 8 bits. > > rtx_def mode 8 => 16 bits. > > tree_base code unchanged. > > > > The structure layout of both the rtx_def and tree_base will be something similar as below. As I understand, the lower 8-bits of tree_base will be inspected when 'dv' is a tree for the rtx conversion. > > > > tree_base rtx_def > > code: 16 code: 8 > > side_effects_flag: 1 mode: 16 > > I think we should try hard to avoid that though. The 16-bit value should > be aligned to 16 bits if at all possible. decl_or_value doesn't seem > like something that should be dictating our approach here. > > Perhaps we can use pointer_mux for decl_or_value instead? pointer_mux is > intended to be a standands-compliant (hah!) way of switching between two > pointer types in a reasonably efficient way. Ah, I wasn't aware of that - yes, that looks good to use I think. Pan, can you prepare a patch only doing such conversion of the var-tracking decl_or_value type? Aka make it typedef pointer_mux decl_or_value; and adjust uses? Thanks, Richard. > Thanks, > Richard > > > constant_flag: 1 > > addressable_flag: 1 > > volatile_flag: 1 > > readonly_flag: 1 > > asm_written_flag: 1 > > nowarning_flag: 1 > > visited: 1 > > used_flag: 1 > > nothrow_flag: 1 > > static_flag: 1 > > public_flag: 1 > > private_flag: 1 > > protected_flag: 1 > > deprecated_flag: 1 > > default_def_flag: 1 > > > > I have a try a similar approach (as below) as you mentioned, aka shrink tree_code as 1:1 overlap to rtx_code. And completed one memory allocated bytes test in another email. > > > > rtx_def code 16 => 12 bits. > > rtx_def mode 8 => 12 bits. > > tree_base code 16 => 12 bits. > > > > Pan > > > > -----Original Message----- > > From: Richard Biener > > Sent: Monday, May 8, 2023 3:38 PM > > To: Li, Pan2 > > Cc: Jeff Law ; Kito Cheng ; juzhe.zhong@rivai.ai; richard.sandiford ; gcc-patches ; palmer ; jakub > > Subject: RE: [PATCH] machine_mode type size: Extend enum size from 8-bit to 16-bit > > > > On Mon, 8 May 2023, Li, Pan2 wrote: > > > >> return !dv || (int) GET_CODE ((rtx) dv) != (int) VALUE; } is able to > >> fix this ICE after mode bits change. > > > > Can you check which bits this will inspect when 'dv' is a tree after your patch? VALUE is 1 and would map to IDENTIFIER_NODE on the tree side when there was a 1:1 overlap. > > > > I think for all cases but struct loc_exp_dep we could find a bit to record wheter we deal with a VALUE or a decl, but for loc_exp_dep it's going to be difficult (unless we start to take bits from pointer representations). > > > > That said, I agree with Jeff that the code is ugly, but a simplistic conversion isn't what we want. > > > > An alternative "solution" might be to also shrink tree_code when we shrink rtx_code and keep the 1:1 overlap. > > > > Richard. > > > >> I will re-trigger the memory allocate bytes test with below changes > >> for X86. > >> > >> rtx_def code 16 => 8 bits. > >> rtx_def mode 8 => 16 bits. > >> tree_base code unchanged. > >> > >> Pan > >> > >> -----Original Message----- > >> From: Li, Pan2 > >> Sent: Monday, May 8, 2023 2:42 PM > >> To: Richard Biener ; Jeff Law > >> > >> Cc: Kito Cheng ; juzhe.zhong@rivai.ai; > >> richard.sandiford ; gcc-patches > >> ; palmer ; jakub > >> > >> Subject: RE: [PATCH] machine_mode type size: Extend enum size from > >> 8-bit to 16-bit > >> > >> Oops. Actually I am patching a version as you mentioned like storage allocation. Thank you Richard, will try your suggestion and keep you posted. > >> > >> Pan > >> > >> -----Original Message----- > >> From: Richard Biener > >> Sent: Monday, May 8, 2023 2:30 PM > >> To: Jeff Law > >> Cc: Li, Pan2 ; Kito Cheng ; > >> juzhe.zhong@rivai.ai; richard.sandiford ; > >> gcc-patches ; palmer ; > >> jakub > >> Subject: Re: [PATCH] machine_mode type size: Extend enum size from > >> 8-bit to 16-bit > >> > >> On Sun, 7 May 2023, Jeff Law wrote: > >> > >> > > >> > > >> > On 5/6/23 19:55, Li, Pan2 wrote: > >> > > It looks like we cannot simply swap the code and mode in rtx_def, > >> > > the code may have to be the same bits as the tree_code in tree_base. > >> > > Or we will meet ICE like below. > >> > > > >> > > rtx_def code 16 => 8 bits. > >> > > rtx_def mode 8 => 16 bits. > >> > > > >> > > static inline decl_or_value > >> > > dv_from_value (rtx value) > >> > > { > >> > > decl_or_value dv; > >> > > dv = value; > >> > > gcc_checking_assert (dv_is_value_p (dv)); <= ICE > >> > > return dv; > >> > Ugh. We really just need to fix this code. It assumes particular > >> > structure layouts and that's just wrong/dumb. > >> > >> Well, it's a neat trick ... we just need to adjust it to > >> > >> static inline bool > >> dv_is_decl_p (decl_or_value dv) > >> { > >> return !dv || (int) GET_CODE ((rtx) dv) != (int) VALUE; } > >> > >> I think (and hope for the 'decl' case the bits inspected are never 'VALUE'). Of course the above stinks from a TBAA perspective ... > >> > >> Any "real" fix would require allocating storage for a discriminator and thus hurt the resource constrained var-tracking a lot. > >> > >> Richard. > >> > > > > -- > > Richard Biener > > SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman; HRB 36809 (AG Nuernberg) > -- Richard Biener SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman; HRB 36809 (AG Nuernberg)