Just migrated to the pointer_mux for the var-tracking, it works well even the bitsize of tree_base code is different from the rtl_def code. I will prepare the PATCH if there is no surprise from the X86 bootstrap test.
Thanks Richard for pointing out the pointer_mux, 😉! Pan -----Original Message----- From: Li, Pan2 Sent: Tuesday, May 9, 2023 7:51 PM To: Richard Biener <rguent...@suse.de>; Richard Sandiford <richard.sandif...@arm.com> Cc: Jeff Law <jeffreya...@gmail.com>; Kito Cheng <kito.ch...@gmail.com>; juzhe.zh...@rivai.ai; gcc-patches <gcc-patches@gcc.gnu.org>; palmer <pal...@dabbelt.com>; jakub <ja...@redhat.com> Subject: RE: [PATCH] machine_mode type size: Extend enum size from 8-bit to 16-bit Sure thing, I will have a try and keep you posted. Pan -----Original Message----- From: Richard Biener <rguent...@suse.de> Sent: Tuesday, May 9, 2023 6:26 PM To: Richard Sandiford <richard.sandif...@arm.com> Cc: Li, Pan2 <pan2...@intel.com>; Jeff Law <jeffreya...@gmail.com>; Kito Cheng <kito.ch...@gmail.com>; juzhe.zh...@rivai.ai; gcc-patches <gcc-patches@gcc.gnu.org>; palmer <pal...@dabbelt.com>; jakub <ja...@redhat.com> Subject: Re: [PATCH] machine_mode type size: Extend enum size from 8-bit to 16-bit On Tue, 9 May 2023, Richard Sandiford wrote: > "Li, Pan2" <pan2...@intel.com> 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<rtx_def, tree_node> 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 <rguent...@suse.de> > > Sent: Monday, May 8, 2023 3:38 PM > > To: Li, Pan2 <pan2...@intel.com> > > Cc: Jeff Law <jeffreya...@gmail.com>; Kito Cheng > > <kito.ch...@gmail.com>; juzhe.zh...@rivai.ai; richard.sandiford > > <richard.sandif...@arm.com>; gcc-patches <gcc-patches@gcc.gnu.org>; > > palmer <pal...@dabbelt.com>; jakub <ja...@redhat.com> > > 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 <rguent...@suse.de>; Jeff Law > >> <jeffreya...@gmail.com> > >> Cc: Kito Cheng <kito.ch...@gmail.com>; juzhe.zh...@rivai.ai; > >> richard.sandiford <richard.sandif...@arm.com>; gcc-patches > >> <gcc-patches@gcc.gnu.org>; palmer <pal...@dabbelt.com>; jakub > >> <ja...@redhat.com> > >> 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 <rguent...@suse.de> > >> Sent: Monday, May 8, 2023 2:30 PM > >> To: Jeff Law <jeffreya...@gmail.com> > >> Cc: Li, Pan2 <pan2...@intel.com>; Kito Cheng > >> <kito.ch...@gmail.com>; juzhe.zh...@rivai.ai; richard.sandiford > >> <richard.sandif...@arm.com>; gcc-patches <gcc-patches@gcc.gnu.org>; > >> palmer <pal...@dabbelt.com>; jakub <ja...@redhat.com> > >> 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 <rguent...@suse.de> > > 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 <rguent...@suse.de> SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman; HRB 36809 (AG Nuernberg)