On Thu, 2020-08-06 at 06:23 +0100, Richard Sandiford wrote: > luoxhu <luo...@linux.ibm.com> writes: > > Hi Richard, > > > > On 2020/8/3 22:01, Richard Sandiford wrote: > > > > /* Try a wider mode if truncating the store mode to NEW_MODE > > > > requires a real instruction. */ > > > > if (maybe_lt (GET_MODE_SIZE (new_mode), GET_MODE_SIZE > > > > (store_mode)) > > > > @@ -1779,6 +1780,25 @@ find_shift_sequence (poly_int64 access_size, > > > > && !targetm.modes_tieable_p (new_mode, store_mode)) > > > > continue; > > > > > > > > + if (multiple_p (GET_MODE_BITSIZE (new_mode), shift) > > > > > > Like I mentioned before, this should be the other way around: > > > > > > multiple_p (shift, GET_MODE_BITSIZE (new_mode)) > > > > > > i.e. for the subreg to be valid, the shift amount must be a multiple > > > of the outer mode size in bits. > > > > > > OK with that change, thanks, and sorry for the multiple review cycles. > > > > Just had another question is do we want to backport this patch to gcc-10 > > and gcc-9 branches? :) > > One for the RMs, but it looks like the PR was reported against GCC 7 and > isn't a regression. Given that it's also “just” a missed optimisation, > I'm guessing it should be trunk only. Not a release manager, but I agree, let's keep this on the trunk.
jeff >
