> So how about this. I'll ack this for the trunk, but not for gcc-14 (at > this time). We can revisit for gcc-14 after it's been on the trunk a > bit. Realistically that likely means gcc-14.2 at the end of the summer > rather than gcc-14.1 which is due in roughly a week.
Thanks Jeff, I will prepare a v3 for this with full and well tested results. Pan -----Original Message----- From: Jeff Law <jeffreya...@gmail.com> Sent: Monday, April 29, 2024 11:20 PM To: Li, Pan2 <pan2...@intel.com>; Robin Dapp <rdapp....@gmail.com>; gcc-patches@gcc.gnu.org Cc: juzhe.zh...@rivai.ai; kito.ch...@gmail.com; richard.guent...@gmail.com; Wang, Yanzhang <yanzhang.w...@intel.com>; Liu, Hongtao <hongtao....@intel.com> Subject: Re: [PATCH v2] DSE: Bugfix ICE after allow vector type in get_stored_val On 3/22/24 11:45 PM, Li, Pan2 wrote: > Thanks Jeff for comments. > >> As Richi noted using validate_subreg here isn't great. Does it work to >> factor out this code from extract_low_bits >> >>> if (!int_mode_for_mode (src_mode).exists (&src_int_mode) >>> || !int_mode_for_mode (mode).exists (&int_mode)) >>> return NULL_RTX; >>> >>> if (!targetm.modes_tieable_p (src_int_mode, src_mode)) >>> return NULL_RTX; >>> if (!targetm.modes_tieable_p (int_mode, mode)) >>> return NULL_RTX; > >> And use that in the condition (and in extract_low_bits rather than >> duplicating the code)? > > It can solve the ICE but will forbid all vector modes goes gen_lowpart. > Actually only the vector mode size is less than reg nature size will trigger > the ICE. > Thus, how about just add one more condition before goes to gen_lowpart as > below? > > Feel free to correct me if any misunderstandings. 😉! > > diff --git a/gcc/dse.cc b/gcc/dse.cc > index edc7a1dfecf..258d2ccc299 100644 > --- a/gcc/dse.cc > +++ b/gcc/dse.cc > @@ -1946,7 +1946,9 @@ get_stored_val (store_info *store_info, machine_mode > read_mode, > copy_rtx (store_info->const_rhs)); > else if (VECTOR_MODE_P (read_mode) && VECTOR_MODE_P (store_mode) > && known_le (GET_MODE_BITSIZE (read_mode), GET_MODE_BITSIZE > (store_mode)) > - && targetm.modes_tieable_p (read_mode, store_mode)) > + && targetm.modes_tieable_p (read_mode, store_mode) > + /* It's invalid in validate_subreg if read_mode size is < reg natural. > */ > + && known_ge (GET_MODE_SIZE (read_mode), REGMODE_NATURAL_SIZE > (read_mode))) > read_reg = gen_lowpart (read_mode, copy_rtx (store_info->rhs)); > else > read_reg = extract_low_bits (read_mode, store_mode, So how about this. I'll ack this for the trunk, but not for gcc-14 (at this time). We can revisit for gcc-14 after it's been on the trunk a bit. Realistically that likely means gcc-14.2 at the end of the summer rather than gcc-14.1 which is due in roughly a week. Jeff
