| Message ID | 878tjsd81e.fsf@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 07/13/2017 03:02 AM, Richard Sandiford wrote: > This patch treats the mode associated with an integer constant as a > scalar_mode. We can't use the more natural-sounding scalar_int_mode > because we also use (const_int 0) for bounds-checking modes. (It might > be worth adding a bounds-specific code instead, but that's for another > day.) Is that the only reason why we can't use scalar_int_mode here -- the bounds checking stuff? What if it were to just magically disappear? > > This exposes a latent bug in simplify_immed_subreg, which for > vectors of CONST_WIDE_INTs would pass the vector mode rather than > the element mode to rtx_mode_t. > > I think the: > > /* We can get a 0 for an error mark. */ > || GET_MODE_CLASS (mode) == MODE_VECTOR_INT > || GET_MODE_CLASS (mode) == MODE_VECTOR_FLOAT An interesting nugget. It appears Aldy's commit message, but not in the approved patch from June 2002. > > in immed_double_const is dead. trunc_int_mode (via gen_int_mode) > would go on to ICE if the mode fitted in a HWI, and surely plenty > of other code would be confused to see a const_int be interpreted > as a vector. We should instead be using CONST0_RTX (mode) if we > need a safe constant for a particular mode. Yea, if we wanted to use 0 as an error mark, we should be using it via CONST0_RTX (mode). > > We didn't try to make these functions take scalar_mode arguments > because in many cases that would be too invasive at this stage. > Maybe it would become feasible in future. Also, the long-term > direction should probably be to add modes to constant integers > rather than have then as VOIDmode odd-ones-out. That would remove > the need for rtx_mode_t and thus remove the question whether they > should use scalar_int_mode, scalar_mode or machine_mode. THe lack of a mode on CONST_INTs is a long standing wart. It's been eons since we really thought about how to fix it. I'd have to dig real deep to remember why we've let the wart stand so long > > The patch also uses scalar_mode for the CONST_DOUBLE handling > in loc_descriptor. In that case the mode can legitimately be > either floating-point or integral. > > 2017-07-13 Richard Sandiford <richard.sandiford@linaro.org> > Alan Hayward <alan.hayward@arm.com> > David Sherwood <david.sherwood@arm.com> > > gcc/ > * emit-rtl.c (immed_double_const): Use is_a <scalar_mode> instead > of separate mode class checks. Do not allow vector modes here. > (immed_wide_int_const): Use as_a <scalar_mode>. > * explow.c (trunc_int_for_mode): Likewise. > * rtl.h (wi::int_traits<rtx_mode_t>::get_precision): Likewise. > (wi::shwi): Likewise. > (wi::min_value): Likewise. > (wi::max_value): Likewise. > * dwarf2out.c (loc_descriptor): Likewise. > * simplify-rtx.c (simplify_immed_subreg): Fix rtx_mode_t argument > for CONST_WIDE_INT. OK. jeff
Jeff Law <law@redhat.com> writes: > On 07/13/2017 03:02 AM, Richard Sandiford wrote: >> This patch treats the mode associated with an integer constant as a >> scalar_mode. We can't use the more natural-sounding scalar_int_mode >> because we also use (const_int 0) for bounds-checking modes. (It might >> be worth adding a bounds-specific code instead, but that's for another >> day.) > Is that the only reason why we can't use scalar_int_mode here -- the > bounds checking stuff? What if it were to just magically disappear? :-) I *think* that was the only case, but it's possible that once we hit it, we didn't look much further. [...] >> We didn't try to make these functions take scalar_mode arguments >> because in many cases that would be too invasive at this stage. >> Maybe it would become feasible in future. Also, the long-term >> direction should probably be to add modes to constant integers >> rather than have then as VOIDmode odd-ones-out. That would remove >> the need for rtx_mode_t and thus remove the question whether they >> should use scalar_int_mode, scalar_mode or machine_mode. > THe lack of a mode on CONST_INTs is a long standing wart. It's been > eons since we really thought about how to fix it. I'd have to dig > real deep to remember why we've let the wart stand so long When I last looked at the history, I got the impression it was just lack of time. I have a vague plan for how we could transition to integers with modes, but then I've had the same plan for a while now and no realistic chance of getting time to do it. Thanks, Richard
Index: gcc/emit-rtl.c =================================================================== --- gcc/emit-rtl.c 2017-07-13 09:18:51.646771977 +0100 +++ gcc/emit-rtl.c 2017-07-13 09:18:54.682546579 +0100 @@ -599,7 +599,8 @@ lookup_const_wide_int (rtx wint) immed_wide_int_const (const wide_int_ref &v, machine_mode mode) { unsigned int len = v.get_len (); - unsigned int prec = GET_MODE_PRECISION (mode); + /* Not scalar_int_mode because we also allow pointer bound modes. */ + unsigned int prec = GET_MODE_PRECISION (as_a <scalar_mode> (mode)); /* Allow truncation but not extension since we do not know if the number is signed or unsigned. */ @@ -659,18 +660,10 @@ immed_double_const (HOST_WIDE_INT i0, HO (i.e., i1 consists only from copies of the sign bit, and sign of i0 and i1 are the same), then we return a CONST_INT for i0. 3) Otherwise, we create a CONST_DOUBLE for i0 and i1. */ - if (mode != VOIDmode) - { - gcc_assert (GET_MODE_CLASS (mode) == MODE_INT - || GET_MODE_CLASS (mode) == MODE_PARTIAL_INT - /* We can get a 0 for an error mark. */ - || GET_MODE_CLASS (mode) == MODE_VECTOR_INT - || GET_MODE_CLASS (mode) == MODE_VECTOR_FLOAT - || GET_MODE_CLASS (mode) == MODE_POINTER_BOUNDS); - - if (GET_MODE_BITSIZE (mode) <= HOST_BITS_PER_WIDE_INT) - return gen_int_mode (i0, mode); - } + scalar_mode smode; + if (is_a <scalar_mode> (mode, &smode) + && GET_MODE_BITSIZE (smode) <= HOST_BITS_PER_WIDE_INT) + return gen_int_mode (i0, mode); /* If this integer fits in one word, return a CONST_INT. */ if ((i1 == 0 && i0 >= 0) || (i1 == ~0 && i0 < 0)) Index: gcc/explow.c =================================================================== --- gcc/explow.c 2017-07-13 09:18:51.647771901 +0100 +++ gcc/explow.c 2017-07-13 09:18:54.682546579 +0100 @@ -49,14 +49,16 @@ static rtx break_out_memory_refs (rtx); HOST_WIDE_INT trunc_int_for_mode (HOST_WIDE_INT c, machine_mode mode) { - int width = GET_MODE_PRECISION (mode); + /* Not scalar_int_mode because we also allow pointer bound modes. */ + scalar_mode smode = as_a <scalar_mode> (mode); + int width = GET_MODE_PRECISION (smode); /* You want to truncate to a _what_? */ gcc_assert (SCALAR_INT_MODE_P (mode) || POINTER_BOUNDS_MODE_P (mode)); /* Canonicalize BImode to 0 and STORE_FLAG_VALUE. */ - if (mode == BImode) + if (smode == BImode) return c & 1 ? STORE_FLAG_VALUE : 0; /* Sign-extend for the requested mode. */ Index: gcc/rtl.h =================================================================== --- gcc/rtl.h 2017-07-13 09:18:51.662770770 +0100 +++ gcc/rtl.h 2017-07-13 09:18:54.682546579 +0100 @@ -2120,8 +2120,7 @@ typedef std::pair <rtx, machine_mode> rt inline unsigned int wi::int_traits <rtx_mode_t>::get_precision (const rtx_mode_t &x) { - gcc_checking_assert (x.second != BLKmode && x.second != VOIDmode); - return GET_MODE_PRECISION (x.second); + return GET_MODE_PRECISION (as_a <scalar_mode> (x.second)); } inline wi::storage_ref @@ -2166,7 +2165,7 @@ wi::int_traits <rtx_mode_t>::decompose ( inline wi::hwi_with_prec wi::shwi (HOST_WIDE_INT val, machine_mode mode) { - return shwi (val, GET_MODE_PRECISION (mode)); + return shwi (val, GET_MODE_PRECISION (as_a <scalar_mode> (mode))); } /* Produce the smallest number that is represented in MODE. The precision @@ -2174,7 +2173,7 @@ wi::shwi (HOST_WIDE_INT val, machine_mod inline wide_int wi::min_value (machine_mode mode, signop sgn) { - return min_value (GET_MODE_PRECISION (mode), sgn); + return min_value (GET_MODE_PRECISION (as_a <scalar_mode> (mode)), sgn); } /* Produce the largest number that is represented in MODE. The precision @@ -2182,7 +2181,7 @@ wi::min_value (machine_mode mode, signop inline wide_int wi::max_value (machine_mode mode, signop sgn) { - return max_value (GET_MODE_PRECISION (mode), sgn); + return max_value (GET_MODE_PRECISION (as_a <scalar_mode> (mode)), sgn); } extern void init_rtlanal (void); Index: gcc/dwarf2out.c =================================================================== --- gcc/dwarf2out.c 2017-07-13 09:18:51.644772127 +0100 +++ gcc/dwarf2out.c 2017-07-13 09:18:54.681546653 +0100 @@ -15781,10 +15781,11 @@ loc_descriptor (rtx rtl, machine_mode mo or a floating-point constant. A CONST_DOUBLE is used whenever the constant requires more than one word in order to be adequately represented. We output CONST_DOUBLEs as blocks. */ + scalar_mode smode = as_a <scalar_mode> (mode); loc_result = new_loc_descr (DW_OP_implicit_value, - GET_MODE_SIZE (mode), 0); + GET_MODE_SIZE (smode), 0); #if TARGET_SUPPORTS_WIDE_INT == 0 - if (!SCALAR_FLOAT_MODE_P (mode)) + if (!SCALAR_FLOAT_MODE_P (smode)) { loc_result->dw_loc_oprnd2.val_class = dw_val_class_const_double; loc_result->dw_loc_oprnd2.v.val_double @@ -15793,7 +15794,7 @@ loc_descriptor (rtx rtl, machine_mode mo else #endif { - unsigned int length = GET_MODE_SIZE (mode); + unsigned int length = GET_MODE_SIZE (smode); unsigned char *array = ggc_vec_alloc<unsigned char> (length); insert_float (rtl, array); Index: gcc/simplify-rtx.c =================================================================== --- gcc/simplify-rtx.c 2017-07-13 09:18:53.276650175 +0100 +++ gcc/simplify-rtx.c 2017-07-13 09:18:54.683546506 +0100 @@ -5785,7 +5785,7 @@ simplify_immed_subreg (machine_mode oute case CONST_WIDE_INT: { - rtx_mode_t val = rtx_mode_t (el, innermode); + rtx_mode_t val = rtx_mode_t (el, GET_MODE_INNER (innermode)); unsigned char extend = wi::sign_mask (val); int prec = wi::get_precision (val);