Re: [66/77] Use scalar_mode for constant integers

2017-08-30 Thread Richard Sandiford
Jeff Law  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


Re: [66/77] Use scalar_mode for constant integers

2017-08-24 Thread Jeff Law
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  
>   Alan Hayward  
>   David Sherwood  
> 
> gcc/
>   * emit-rtl.c (immed_double_const): Use is_a  instead
>   of separate mode class checks.  Do not allow vector modes here.
>   (immed_wide_int_const): Use as_a .
>   * explow.c (trunc_int_for_mode): Likewise.
>   * rtl.h (wi::int_traits::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