From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id 9850F3858D37 for ; Tue, 9 May 2023 10:16:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 9850F3858D37 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id AAA0A1063; Tue, 9 May 2023 03:17:43 -0700 (PDT) Received: from localhost (e121540-lin.manchester.arm.com [10.32.110.72]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 119B23F67D; Tue, 9 May 2023 03:16:57 -0700 (PDT) From: Richard Sandiford To: "Li\, Pan2" Mail-Followup-To: "Li\, Pan2" ,Richard Biener , Jeff Law , Kito Cheng , "juzhe.zhong\@rivai.ai" , gcc-patches , palmer , jakub , richard.sandiford@arm.com Cc: Richard Biener , 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 References: <20230410144808.324346-1-juzhe.zhong@rivai.ai> <436847c8-0c15-24de-5925-f56d78caf540@gmail.com> Date: Tue, 09 May 2023 11:16:56 +0100 In-Reply-To: (Pan2 Li's message of "Mon, 8 May 2023 08:05:42 +0000") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-24.0 required=5.0 tests=BAYES_00,KAM_DMARC_NONE,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: "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. 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)