Re: [06/77] Make GET_MODE_WIDER return an opt_mode

[Jeff Law]

On 08/29/2017 09:36 AM, Richard Sandiford wrote:
> Jeff Law  writes:
>> On 08/29/2017 09:01 AM, Richard Sandiford wrote:
>>> Jeff Law  writes:
>>>
>>> OK, here's a patch that uses require ().  I've updated the following
>>> patches in the obvious way.
>> Thanks.
>>
>>>
>>> This does make me want to reconsider another decision though.
>>> Using opt_mode for iterators leads to things like:
>>>
>>>   opt_scalar_int_mode wider_mode_iter;
>>>   FOR_EACH_WIDER_MODE (wider_mode_iter, mode)
>>> {
>>>   scalar_int_mode wider_mode = wider_mode_iter.require ();
>>>   if (optab_handler (unoptab, wider_mode) != CODE_FOR_nothing)
>>> ...
>>>
>>> which isn't pretty.  It would be easy to support:
>> No ideal, but it's reasonably explicit in what it does, so I wouldn't
>> expect anyone to be surprised.
>>
>>
>>>
>>>   scalar_int_mode wider_mode;
>>>   FOR_EACH_WIDER_MODE (wider_mode, mode)
>>> if (optab_handler (unoptab, wider_mode) != CODE_FOR_nothing)
>>>   ...
>>>
>>> *but* this would mean that "wider_mode" is accessible but undefined
>>> after the loop (unlike the first loop, where wider_mode_iter is
>>> guaranteed to be empty if the loop runs to completion).  Is that OK?
>>> Or is it too suprising?
>> I think most folks would be surprised that they can't look at wider_mode
>> in a meaningful way after the loop.
>>
>>>
>>> Another alternative would be:
>>>
>>>   opt_scalar_int_mode wider_mode_iter;
>>>   scalar_int_mode wider_mode;
>>>   FOR_EACH_WIDER_MODE (wider_mode_iter, wider_mode, mode)
>>> if (optab_handler (unoptab, wider_mode) != CODE_FOR_nothing)
>>>   ...
>>>
>>> which gives both.  But perhaps this would be odd for plain machine_mode
>>> iterators, where there's no obvious distinction between the first and
>>> second arguments.
>> Well, this is like the first to me, except we've separated the iterator
>> from the mode.
>>
>> I slightly prefer the first and think the second is probably too
>> different from the traditional way we think about the state of loop
>> variables after the loop has terminated.
> 
> OK, that's easy then :-)  Is OK to apply the approved parts of the
> series with the "require" change then?  If the consensus ends up
> being that we should handle the iterators a different way, I'll
> volunteer to do a bulk update.
Yea, I think the approved parts are good to go for the trunk.  I think
that covers the whole series, except the aarch64 backend bits which I
left to the ARM folks.

> 
> Thanks again for all the reviews.
No problem.  Sorry it took so long.

jeff

Re: [06/77] Make GET_MODE_WIDER return an opt_mode

[Richard Sandiford]

Jeff Law  writes:
> On 08/29/2017 09:01 AM, Richard Sandiford wrote:
>> Jeff Law  writes:
>> 
>> OK, here's a patch that uses require ().  I've updated the following
>> patches in the obvious way.
> Thanks.
>
>> 
>> This does make me want to reconsider another decision though.
>> Using opt_mode for iterators leads to things like:
>> 
>>   opt_scalar_int_mode wider_mode_iter;
>>   FOR_EACH_WIDER_MODE (wider_mode_iter, mode)
>> {
>>   scalar_int_mode wider_mode = wider_mode_iter.require ();
>>   if (optab_handler (unoptab, wider_mode) != CODE_FOR_nothing)
>> ...
>> 
>> which isn't pretty.  It would be easy to support:
> No ideal, but it's reasonably explicit in what it does, so I wouldn't
> expect anyone to be surprised.
>
>
>> 
>>   scalar_int_mode wider_mode;
>>   FOR_EACH_WIDER_MODE (wider_mode, mode)
>> if (optab_handler (unoptab, wider_mode) != CODE_FOR_nothing)
>>   ...
>> 
>> *but* this would mean that "wider_mode" is accessible but undefined
>> after the loop (unlike the first loop, where wider_mode_iter is
>> guaranteed to be empty if the loop runs to completion).  Is that OK?
>> Or is it too suprising?
> I think most folks would be surprised that they can't look at wider_mode
> in a meaningful way after the loop.
>
>> 
>> Another alternative would be:
>> 
>>   opt_scalar_int_mode wider_mode_iter;
>>   scalar_int_mode wider_mode;
>>   FOR_EACH_WIDER_MODE (wider_mode_iter, wider_mode, mode)
>> if (optab_handler (unoptab, wider_mode) != CODE_FOR_nothing)
>>   ...
>> 
>> which gives both.  But perhaps this would be odd for plain machine_mode
>> iterators, where there's no obvious distinction between the first and
>> second arguments.
> Well, this is like the first to me, except we've separated the iterator
> from the mode.
>
> I slightly prefer the first and think the second is probably too
> different from the traditional way we think about the state of loop
> variables after the loop has terminated.

OK, that's easy then :-)  Is OK to apply the approved parts of the
series with the "require" change then?  If the consensus ends up
being that we should handle the iterators a different way, I'll
volunteer to do a bulk update.

Thanks again for all the reviews.

Richard

Re: [06/77] Make GET_MODE_WIDER return an opt_mode

[Jeff Law]

On 08/29/2017 09:01 AM, Richard Sandiford wrote:
> Jeff Law  writes:
> 
> OK, here's a patch that uses require ().  I've updated the following
> patches in the obvious way.
Thanks.

> 
> This does make me want to reconsider another decision though.
> Using opt_mode for iterators leads to things like:
> 
>   opt_scalar_int_mode wider_mode_iter;
>   FOR_EACH_WIDER_MODE (wider_mode_iter, mode)
> {
>   scalar_int_mode wider_mode = wider_mode_iter.require ();
>   if (optab_handler (unoptab, wider_mode) != CODE_FOR_nothing)
> ...
> 
> which isn't pretty.  It would be easy to support:
No ideal, but it's reasonably explicit in what it does, so I wouldn't
expect anyone to be surprised.


> 
>   scalar_int_mode wider_mode;
>   FOR_EACH_WIDER_MODE (wider_mode, mode)
> if (optab_handler (unoptab, wider_mode) != CODE_FOR_nothing)
>   ...
> 
> *but* this would mean that "wider_mode" is accessible but undefined
> after the loop (unlike the first loop, where wider_mode_iter is
> guaranteed to be empty if the loop runs to completion).  Is that OK?
> Or is it too suprising?
I think most folks would be surprised that they can't look at wider_mode
in a meaningful way after the loop.

> 
> Another alternative would be:
> 
>   opt_scalar_int_mode wider_mode_iter;
>   scalar_int_mode wider_mode;
>   FOR_EACH_WIDER_MODE (wider_mode_iter, wider_mode, mode)
> if (optab_handler (unoptab, wider_mode) != CODE_FOR_nothing)
>   ...
> 
> which gives both.  But perhaps this would be odd for plain machine_mode
> iterators, where there's no obvious distinction between the first and
> second arguments.
Well, this is like the first to me, except we've separated the iterator
from the mode.

I slightly prefer the first and think the second is probably too
different from the traditional way we think about the state of loop
variables after the loop has terminated.

Jeff

Re: [06/77] Make GET_MODE_WIDER return an opt_mode

[Richard Sandiford]

Jeff Law  writes:
> On 08/28/2017 01:05 PM, Richard Sandiford wrote:
>> 
>>> As for the name, get_nonvoid?  Ugh.  Not sure.  Open to suggestions.
>> 
>> I'd rather avoid "nonvoid", since the use of VOIDmode for "no mode" is
>> really an implementation detail in things like opt_mode .
>> Other possiblities might be:
> Yea, good point on encoding the implementation detail not being a good idea.
>
>> 
>>   - require
>>   - demand
>>   - mode
>>   - get_mode
>>   - require_mode
>>   - demand_mode
>>   - else_fail (to go with else_void and else_blk)
>>   - noelse
> require, demand with or without the _mode suffix seem good to me.

OK, here's a patch that uses require ().  I've updated the following
patches in the obvious way.

This does make me want to reconsider another decision though.
Using opt_mode for iterators leads to things like:

  opt_scalar_int_mode wider_mode_iter;
  FOR_EACH_WIDER_MODE (wider_mode_iter, mode)
{
  scalar_int_mode wider_mode = wider_mode_iter.require ();
  if (optab_handler (unoptab, wider_mode) != CODE_FOR_nothing)
...

which isn't pretty.  It would be easy to support:

  scalar_int_mode wider_mode;
  FOR_EACH_WIDER_MODE (wider_mode, mode)
if (optab_handler (unoptab, wider_mode) != CODE_FOR_nothing)
  ...

*but* this would mean that "wider_mode" is accessible but undefined
after the loop (unlike the first loop, where wider_mode_iter is
guaranteed to be empty if the loop runs to completion).  Is that OK?
Or is it too suprising?

Another alternative would be:

  opt_scalar_int_mode wider_mode_iter;
  scalar_int_mode wider_mode;
  FOR_EACH_WIDER_MODE (wider_mode_iter, wider_mode, mode)
if (optab_handler (unoptab, wider_mode) != CODE_FOR_nothing)
  ...

which gives both.  But perhaps this would be odd for plain machine_mode
iterators, where there's no obvious distinction between the first and
second arguments.

Thanks,
Richard


2017-08-29  Richard Sandiford  
Alan Hayward  
David Sherwood  

gcc/
* coretypes.h (opt_mode): New class.
* machmode.h (opt_mode): Likewise.
(opt_mode::else_void): New function.
(opt_mode::require): Likewise.
(opt_mode::exists): Likewise.
(GET_MODE_WIDER_MODE): Turn into a function and return an opt_mode.
(GET_MODE_2XWIDER_MODE): Likewise.
(mode_iterator::get_wider): Update accordingly.
(mode_iterator::get_2xwider): Likewise.
(mode_iterator::get_known_wider): Likewise, turning into a template.
* combine.c (make_extraction): Update use of GET_MODE_WIDER_MODE,
forcing a wider mode to exist.
* config/cr16/cr16.h (LONG_REG_P): Likewise.
* rtlanal.c (init_num_sign_bit_copies_in_rep): Likewise.
* config/c6x/c6x.c (c6x_rtx_costs): Update use of
GET_MODE_2XWIDER_MODE, forcing a wider mode to exist.
* lower-subreg.c (init_lower_subreg): Likewise.
* optabs-libfuncs.c (init_sync_libfuncs_1): Likewise, but not
on the final iteration.
* config/i386/i386.c (ix86_expand_set_or_movmem): Check whether
a wider mode exists before asking for a move pattern.
(get_mode_wider_vector): Update use of GET_MODE_WIDER_MODE,
forcing a wider mode to exist.
(expand_vselect_vconcat): Update use of GET_MODE_2XWIDER_MODE,
returning false if no such mode exists.
* config/ia64/ia64.c (expand_vselect_vconcat): Likewise.
* config/mips/mips.c (mips_expand_vselect_vconcat): Likewise.
* expmed.c (init_expmed_one_mode): Update use of GET_MODE_WIDER_MODE.
Avoid checking for a MODE_INT if we already know the mode is not a
SCALAR_INT_MODE_P.
(extract_high_half): Update use of GET_MODE_WIDER_MODE,
forcing a wider mode to exist.
(expmed_mult_highpart_optab): Likewise.
(expmed_mult_highpart): Likewise.
* expr.c (expand_expr_real_2): Update use of GET_MODE_WIDER_MODE,
using else_void.
* lto-streamer-in.c (lto_input_mode_table): Likewise.
* optabs-query.c (find_widening_optab_handler_and_mode): Likewise.
* stor-layout.c (bit_field_mode_iterator::next_mode): Likewise.
* internal-fn.c (expand_mul_overflow): Update use of
GET_MODE_2XWIDER_MODE.
* omp-low.c (omp_clause_aligned_alignment): Likewise.
* tree-ssa-math-opts.c (convert_mult_to_widen): Update use of
GET_MODE_WIDER_MODE.
(convert_plusminus_to_widen): Likewise.
* tree-switch-conversion.c (array_value_type): Likewise.
* var-tracking.c (emit_note_insn_var_location): Likewise.
* tree-vrp.c (simplify_float_conversion_using_ranges): Likewise.
Return false inside rather than outside the loop if no wider mode
exists
* optabs.c (expand_binop): Update use of GET_MODE_WIDER_MODE
and GET_MODE_2XWIDER_MODE
   

Re: [06/77] Make GET_MODE_WIDER return an opt_mode

[Jeff Law]

On 08/28/2017 01:05 PM, Richard Sandiford wrote:
> 
>> As for the name, get_nonvoid?  Ugh.  Not sure.  Open to suggestions.
> 
> I'd rather avoid "nonvoid", since the use of VOIDmode for "no mode" is
> really an implementation detail in things like opt_mode .
> Other possiblities might be:
Yea, good point on encoding the implementation detail not being a good idea.

> 
>   - require
>   - demand
>   - mode
>   - get_mode
>   - require_mode
>   - demand_mode
>   - else_fail (to go with else_void and else_blk)
>   - noelse
require, demand with or without the _mode suffix seem good to me.

jeff

Re: [06/77] Make GET_MODE_WIDER return an opt_mode

[Richard Sandiford]

Jeff Law  writes:
> On 08/11/2017 12:24 PM, Richard Sandiford wrote:
>> Jeff Law  writes:
>>> On 07/13/2017 02:40 AM, Richard Sandiford wrote:
 GET_MODE_WIDER previously returned VOIDmode if no wider mode existed.
 That would cause problems with stricter mode classes, since VOIDmode
 isn't for example a valid scalar integer or floating-point mode.
 This patch instead makes it return a new opt_mode class, which
 holds either a T or nothing.

 2017-07-13  Richard Sandiford  
Alan Hayward  
David Sherwood  

 gcc/
* coretypes.h (opt_mode): New class.
* machmode.h (opt_mode): Likewise.
(opt_mode::else_void): New function.
(opt_mode::operator *): Likewise.
(opt_mode::exists): Likewise.
(GET_MODE_WIDER_MODE): Turn into a function and return an opt_mode.
(GET_MODE_2XWIDER_MODE): Likewise.
(mode_iterator::get_wider): Update accordingly.
(mode_iterator::get_2xwider): Likewise.
(mode_iterator::get_known_wider): Likewise, turning into a template.
* combine.c (make_extraction): Update use of GET_MODE_WIDER_MODE,
forcing a wider mode to exist.
* config/cr16/cr16.h (LONG_REG_P): Likewise.
* rtlanal.c (init_num_sign_bit_copies_in_rep): Likewise.
* config/c6x/c6x.c (c6x_rtx_costs): Update use of
GET_MODE_2XWIDER_MODE, forcing a wider mode to exist.
* lower-subreg.c (init_lower_subreg): Likewise.
* optabs-libfuncs.c (init_sync_libfuncs_1): Likewise, but not
on the final iteration.
* config/i386/i386.c (ix86_expand_set_or_movmem): Check whether
a wider mode exists before asking for a move pattern.
(get_mode_wider_vector): Update use of GET_MODE_WIDER_MODE,
forcing a wider mode to exist.
(expand_vselect_vconcat): Update use of GET_MODE_2XWIDER_MODE,
returning false if no such mode exists.
* config/ia64/ia64.c (expand_vselect_vconcat): Likewise.
* config/mips/mips.c (mips_expand_vselect_vconcat): Likewise.
* expmed.c (init_expmed_one_mode): Update use of GET_MODE_WIDER_MODE.
Avoid checking for a MODE_INT if we already know the mode is not a
SCALAR_INT_MODE_P.
(extract_high_half): Update use of GET_MODE_WIDER_MODE,
forcing a wider mode to exist.
(expmed_mult_highpart_optab): Likewise.
(expmed_mult_highpart): Likewise.
* expr.c (expand_expr_real_2): Update use of GET_MODE_WIDER_MODE,
using else_void.
* lto-streamer-in.c (lto_input_mode_table): Likewise.
* optabs-query.c (find_widening_optab_handler_and_mode): Likewise.
* stor-layout.c (bit_field_mode_iterator::next_mode): Likewise.
* internal-fn.c (expand_mul_overflow): Update use of
GET_MODE_2XWIDER_MODE.
* omp-low.c (omp_clause_aligned_alignment): Likewise.
* tree-ssa-math-opts.c (convert_mult_to_widen): Update use of
GET_MODE_WIDER_MODE.
(convert_plusminus_to_widen): Likewise.
* tree-switch-conversion.c (array_value_type): Likewise.
* var-tracking.c (emit_note_insn_var_location): Likewise.
* tree-vrp.c (simplify_float_conversion_using_ranges): Likewise.
Return false inside rather than outside the loop if no wider mode
exists
* optabs.c (expand_binop): Update use of GET_MODE_WIDER_MODE
and GET_MODE_2XWIDER_MODE
(can_compare_p): Use else_void.
* gdbhooks.py (OptMachineModePrinter): New class.
(build_pretty_printer): Use it for opt_mode.

 gcc/ada/
* gcc-interface/decl.c (validate_size): Update use of
GET_MODE_WIDER_MODE, forcing a wider mode to exist.
>>> I'm not a big fan of the API here, particularly using operator* to
>>> handle asserting the mode exists.  I'd prefer to just use a member
>>> function rather than overloading operator*.
>>>
>>> What's the rationale behind using operator* to imply the assertion?
>>>
>>> THe changes themsleves look fine, it's really just a question of the API
>>> we present.
>> 
>> The original idea was to make opt_mode look pointer-ish, so that
>> the dyn_cast <...> result could be used in the same way as for
>> dyn_cast  etc.  The first cut therefore had operator bool ()
>> to test whether there was a mode and operator * to dereference it.
>> 
>> However, operator bool () created various subtle problems (as it always
>> seems to) so we dropped it in favour of exists ().  I was neutral
>> on whether we should keep '*' or switch to a function, so in the
>> end the status quo won out.  I'm happy to change it to a named
>> accessor though.
>> 
>> Any better ideas than "get ()" for the name?  Maybe something
>> to emphasis that it is asserting for non-nullness/non-emptiness
>> (which '*' does implicitly)?
>
> Yea, when I was 

Re: [06/77] Make GET_MODE_WIDER return an opt_mode

[Jeff Law]

On 08/11/2017 12:24 PM, Richard Sandiford wrote:
> Jeff Law  writes:
>> On 07/13/2017 02:40 AM, Richard Sandiford wrote:
>>> GET_MODE_WIDER previously returned VOIDmode if no wider mode existed.
>>> That would cause problems with stricter mode classes, since VOIDmode
>>> isn't for example a valid scalar integer or floating-point mode.
>>> This patch instead makes it return a new opt_mode class, which
>>> holds either a T or nothing.
>>>
>>> 2017-07-13  Richard Sandiford  
>>> Alan Hayward  
>>> David Sherwood  
>>>
>>> gcc/
>>> * coretypes.h (opt_mode): New class.
>>> * machmode.h (opt_mode): Likewise.
>>> (opt_mode::else_void): New function.
>>> (opt_mode::operator *): Likewise.
>>> (opt_mode::exists): Likewise.
>>> (GET_MODE_WIDER_MODE): Turn into a function and return an opt_mode.
>>> (GET_MODE_2XWIDER_MODE): Likewise.
>>> (mode_iterator::get_wider): Update accordingly.
>>> (mode_iterator::get_2xwider): Likewise.
>>> (mode_iterator::get_known_wider): Likewise, turning into a template.
>>> * combine.c (make_extraction): Update use of GET_MODE_WIDER_MODE,
>>> forcing a wider mode to exist.
>>> * config/cr16/cr16.h (LONG_REG_P): Likewise.
>>> * rtlanal.c (init_num_sign_bit_copies_in_rep): Likewise.
>>> * config/c6x/c6x.c (c6x_rtx_costs): Update use of
>>> GET_MODE_2XWIDER_MODE, forcing a wider mode to exist.
>>> * lower-subreg.c (init_lower_subreg): Likewise.
>>> * optabs-libfuncs.c (init_sync_libfuncs_1): Likewise, but not
>>> on the final iteration.
>>> * config/i386/i386.c (ix86_expand_set_or_movmem): Check whether
>>> a wider mode exists before asking for a move pattern.
>>> (get_mode_wider_vector): Update use of GET_MODE_WIDER_MODE,
>>> forcing a wider mode to exist.
>>> (expand_vselect_vconcat): Update use of GET_MODE_2XWIDER_MODE,
>>> returning false if no such mode exists.
>>> * config/ia64/ia64.c (expand_vselect_vconcat): Likewise.
>>> * config/mips/mips.c (mips_expand_vselect_vconcat): Likewise.
>>> * expmed.c (init_expmed_one_mode): Update use of GET_MODE_WIDER_MODE.
>>> Avoid checking for a MODE_INT if we already know the mode is not a
>>> SCALAR_INT_MODE_P.
>>> (extract_high_half): Update use of GET_MODE_WIDER_MODE,
>>> forcing a wider mode to exist.
>>> (expmed_mult_highpart_optab): Likewise.
>>> (expmed_mult_highpart): Likewise.
>>> * expr.c (expand_expr_real_2): Update use of GET_MODE_WIDER_MODE,
>>> using else_void.
>>> * lto-streamer-in.c (lto_input_mode_table): Likewise.
>>> * optabs-query.c (find_widening_optab_handler_and_mode): Likewise.
>>> * stor-layout.c (bit_field_mode_iterator::next_mode): Likewise.
>>> * internal-fn.c (expand_mul_overflow): Update use of
>>> GET_MODE_2XWIDER_MODE.
>>> * omp-low.c (omp_clause_aligned_alignment): Likewise.
>>> * tree-ssa-math-opts.c (convert_mult_to_widen): Update use of
>>> GET_MODE_WIDER_MODE.
>>> (convert_plusminus_to_widen): Likewise.
>>> * tree-switch-conversion.c (array_value_type): Likewise.
>>> * var-tracking.c (emit_note_insn_var_location): Likewise.
>>> * tree-vrp.c (simplify_float_conversion_using_ranges): Likewise.
>>> Return false inside rather than outside the loop if no wider mode
>>> exists
>>> * optabs.c (expand_binop): Update use of GET_MODE_WIDER_MODE
>>> and GET_MODE_2XWIDER_MODE
>>> (can_compare_p): Use else_void.
>>> * gdbhooks.py (OptMachineModePrinter): New class.
>>> (build_pretty_printer): Use it for opt_mode.
>>>
>>> gcc/ada/
>>> * gcc-interface/decl.c (validate_size): Update use of
>>> GET_MODE_WIDER_MODE, forcing a wider mode to exist.
>> I'm not a big fan of the API here, particularly using operator* to
>> handle asserting the mode exists.  I'd prefer to just use a member
>> function rather than overloading operator*.
>>
>> What's the rationale behind using operator* to imply the assertion?
>>
>> THe changes themsleves look fine, it's really just a question of the API
>> we present.
> 
> The original idea was to make opt_mode look pointer-ish, so that
> the dyn_cast <...> result could be used in the same way as for
> dyn_cast  etc.  The first cut therefore had operator bool ()
> to test whether there was a mode and operator * to dereference it.
> 
> However, operator bool () created various subtle problems (as it always
> seems to) so we dropped it in favour of exists ().  I was neutral
> on whether we should keep '*' or switch to a function, so in the
> end the status quo won out.  I'm happy to change it to a named
> accessor though.
> 
> Any better ideas than "get ()" for the name?  Maybe something
> to emphasis that it is asserting for non-nullness/non-emptiness
> (which '*' does implicitly)?Yea, when I was reading the first few patches it 
> felt like you trying to
do a pointer-ish 

Re: [06/77] Make GET_MODE_WIDER return an opt_mode

[Richard Sandiford]

Jeff Law  writes:
> On 07/13/2017 02:40 AM, Richard Sandiford wrote:
>> GET_MODE_WIDER previously returned VOIDmode if no wider mode existed.
>> That would cause problems with stricter mode classes, since VOIDmode
>> isn't for example a valid scalar integer or floating-point mode.
>> This patch instead makes it return a new opt_mode class, which
>> holds either a T or nothing.
>> 
>> 2017-07-13  Richard Sandiford  
>>  Alan Hayward  
>>  David Sherwood  
>> 
>> gcc/
>>  * coretypes.h (opt_mode): New class.
>>  * machmode.h (opt_mode): Likewise.
>>  (opt_mode::else_void): New function.
>>  (opt_mode::operator *): Likewise.
>>  (opt_mode::exists): Likewise.
>>  (GET_MODE_WIDER_MODE): Turn into a function and return an opt_mode.
>>  (GET_MODE_2XWIDER_MODE): Likewise.
>>  (mode_iterator::get_wider): Update accordingly.
>>  (mode_iterator::get_2xwider): Likewise.
>>  (mode_iterator::get_known_wider): Likewise, turning into a template.
>>  * combine.c (make_extraction): Update use of GET_MODE_WIDER_MODE,
>>  forcing a wider mode to exist.
>>  * config/cr16/cr16.h (LONG_REG_P): Likewise.
>>  * rtlanal.c (init_num_sign_bit_copies_in_rep): Likewise.
>>  * config/c6x/c6x.c (c6x_rtx_costs): Update use of
>>  GET_MODE_2XWIDER_MODE, forcing a wider mode to exist.
>>  * lower-subreg.c (init_lower_subreg): Likewise.
>>  * optabs-libfuncs.c (init_sync_libfuncs_1): Likewise, but not
>>  on the final iteration.
>>  * config/i386/i386.c (ix86_expand_set_or_movmem): Check whether
>>  a wider mode exists before asking for a move pattern.
>>  (get_mode_wider_vector): Update use of GET_MODE_WIDER_MODE,
>>  forcing a wider mode to exist.
>>  (expand_vselect_vconcat): Update use of GET_MODE_2XWIDER_MODE,
>>  returning false if no such mode exists.
>>  * config/ia64/ia64.c (expand_vselect_vconcat): Likewise.
>>  * config/mips/mips.c (mips_expand_vselect_vconcat): Likewise.
>>  * expmed.c (init_expmed_one_mode): Update use of GET_MODE_WIDER_MODE.
>>  Avoid checking for a MODE_INT if we already know the mode is not a
>>  SCALAR_INT_MODE_P.
>>  (extract_high_half): Update use of GET_MODE_WIDER_MODE,
>>  forcing a wider mode to exist.
>>  (expmed_mult_highpart_optab): Likewise.
>>  (expmed_mult_highpart): Likewise.
>>  * expr.c (expand_expr_real_2): Update use of GET_MODE_WIDER_MODE,
>>  using else_void.
>>  * lto-streamer-in.c (lto_input_mode_table): Likewise.
>>  * optabs-query.c (find_widening_optab_handler_and_mode): Likewise.
>>  * stor-layout.c (bit_field_mode_iterator::next_mode): Likewise.
>>  * internal-fn.c (expand_mul_overflow): Update use of
>>  GET_MODE_2XWIDER_MODE.
>>  * omp-low.c (omp_clause_aligned_alignment): Likewise.
>>  * tree-ssa-math-opts.c (convert_mult_to_widen): Update use of
>>  GET_MODE_WIDER_MODE.
>>  (convert_plusminus_to_widen): Likewise.
>>  * tree-switch-conversion.c (array_value_type): Likewise.
>>  * var-tracking.c (emit_note_insn_var_location): Likewise.
>>  * tree-vrp.c (simplify_float_conversion_using_ranges): Likewise.
>>  Return false inside rather than outside the loop if no wider mode
>>  exists
>>  * optabs.c (expand_binop): Update use of GET_MODE_WIDER_MODE
>>  and GET_MODE_2XWIDER_MODE
>>  (can_compare_p): Use else_void.
>>  * gdbhooks.py (OptMachineModePrinter): New class.
>>  (build_pretty_printer): Use it for opt_mode.
>> 
>> gcc/ada/
>>  * gcc-interface/decl.c (validate_size): Update use of
>>  GET_MODE_WIDER_MODE, forcing a wider mode to exist.
> I'm not a big fan of the API here, particularly using operator* to
> handle asserting the mode exists.  I'd prefer to just use a member
> function rather than overloading operator*.
>
> What's the rationale behind using operator* to imply the assertion?
>
> THe changes themsleves look fine, it's really just a question of the API
> we present.

The original idea was to make opt_mode look pointer-ish, so that
the dyn_cast <...> result could be used in the same way as for
dyn_cast  etc.  The first cut therefore had operator bool ()
to test whether there was a mode and operator * to dereference it.

However, operator bool () created various subtle problems (as it always
seems to) so we dropped it in favour of exists ().  I was neutral
on whether we should keep '*' or switch to a function, so in the
end the status quo won out.  I'm happy to change it to a named
accessor though.

Any better ideas than "get ()" for the name?  Maybe something
to emphasis that it is asserting for non-nullness/non-emptiness
(which '*' does implicitly)?

Thanks,
Richard

Re: [06/77] Make GET_MODE_WIDER return an opt_mode

[Jeff Law]

On 07/13/2017 02:40 AM, Richard Sandiford wrote:
> GET_MODE_WIDER previously returned VOIDmode if no wider mode existed.
> That would cause problems with stricter mode classes, since VOIDmode
> isn't for example a valid scalar integer or floating-point mode.
> This patch instead makes it return a new opt_mode class, which
> holds either a T or nothing.
> 
> 2017-07-13  Richard Sandiford  
>   Alan Hayward  
>   David Sherwood  
> 
> gcc/
>   * coretypes.h (opt_mode): New class.
>   * machmode.h (opt_mode): Likewise.
>   (opt_mode::else_void): New function.
>   (opt_mode::operator *): Likewise.
>   (opt_mode::exists): Likewise.
>   (GET_MODE_WIDER_MODE): Turn into a function and return an opt_mode.
>   (GET_MODE_2XWIDER_MODE): Likewise.
>   (mode_iterator::get_wider): Update accordingly.
>   (mode_iterator::get_2xwider): Likewise.
>   (mode_iterator::get_known_wider): Likewise, turning into a template.
>   * combine.c (make_extraction): Update use of GET_MODE_WIDER_MODE,
>   forcing a wider mode to exist.
>   * config/cr16/cr16.h (LONG_REG_P): Likewise.
>   * rtlanal.c (init_num_sign_bit_copies_in_rep): Likewise.
>   * config/c6x/c6x.c (c6x_rtx_costs): Update use of
>   GET_MODE_2XWIDER_MODE, forcing a wider mode to exist.
>   * lower-subreg.c (init_lower_subreg): Likewise.
>   * optabs-libfuncs.c (init_sync_libfuncs_1): Likewise, but not
>   on the final iteration.
>   * config/i386/i386.c (ix86_expand_set_or_movmem): Check whether
>   a wider mode exists before asking for a move pattern.
>   (get_mode_wider_vector): Update use of GET_MODE_WIDER_MODE,
>   forcing a wider mode to exist.
>   (expand_vselect_vconcat): Update use of GET_MODE_2XWIDER_MODE,
>   returning false if no such mode exists.
>   * config/ia64/ia64.c (expand_vselect_vconcat): Likewise.
>   * config/mips/mips.c (mips_expand_vselect_vconcat): Likewise.
>   * expmed.c (init_expmed_one_mode): Update use of GET_MODE_WIDER_MODE.
>   Avoid checking for a MODE_INT if we already know the mode is not a
>   SCALAR_INT_MODE_P.
>   (extract_high_half): Update use of GET_MODE_WIDER_MODE,
>   forcing a wider mode to exist.
>   (expmed_mult_highpart_optab): Likewise.
>   (expmed_mult_highpart): Likewise.
>   * expr.c (expand_expr_real_2): Update use of GET_MODE_WIDER_MODE,
>   using else_void.
>   * lto-streamer-in.c (lto_input_mode_table): Likewise.
>   * optabs-query.c (find_widening_optab_handler_and_mode): Likewise.
>   * stor-layout.c (bit_field_mode_iterator::next_mode): Likewise.
>   * internal-fn.c (expand_mul_overflow): Update use of
>   GET_MODE_2XWIDER_MODE.
>   * omp-low.c (omp_clause_aligned_alignment): Likewise.
>   * tree-ssa-math-opts.c (convert_mult_to_widen): Update use of
>   GET_MODE_WIDER_MODE.
>   (convert_plusminus_to_widen): Likewise.
>   * tree-switch-conversion.c (array_value_type): Likewise.
>   * var-tracking.c (emit_note_insn_var_location): Likewise.
>   * tree-vrp.c (simplify_float_conversion_using_ranges): Likewise.
>   Return false inside rather than outside the loop if no wider mode
>   exists
>   * optabs.c (expand_binop): Update use of GET_MODE_WIDER_MODE
>   and GET_MODE_2XWIDER_MODE
>   (can_compare_p): Use else_void.
>   * gdbhooks.py (OptMachineModePrinter): New class.
>   (build_pretty_printer): Use it for opt_mode.
> 
> gcc/ada/
>   * gcc-interface/decl.c (validate_size): Update use of
>   GET_MODE_WIDER_MODE, forcing a wider mode to exist.
I'm not a big fan of the API here, particularly using operator* to
handle asserting the mode exists.  I'd prefer to just use a member
function rather than overloading operator*.

What's the rationale behind using operator* to imply the assertion?

THe changes themsleves look fine, it's really just a question of the API
we present.

jeff

[06/77] Make GET_MODE_WIDER return an opt_mode

[Richard Sandiford]

GET_MODE_WIDER previously returned VOIDmode if no wider mode existed.
That would cause problems with stricter mode classes, since VOIDmode
isn't for example a valid scalar integer or floating-point mode.
This patch instead makes it return a new opt_mode class, which
holds either a T or nothing.

2017-07-13  Richard Sandiford  
Alan Hayward  
David Sherwood  

gcc/
* coretypes.h (opt_mode): New class.
* machmode.h (opt_mode): Likewise.
(opt_mode::else_void): New function.
(opt_mode::operator *): Likewise.
(opt_mode::exists): Likewise.
(GET_MODE_WIDER_MODE): Turn into a function and return an opt_mode.
(GET_MODE_2XWIDER_MODE): Likewise.
(mode_iterator::get_wider): Update accordingly.
(mode_iterator::get_2xwider): Likewise.
(mode_iterator::get_known_wider): Likewise, turning into a template.
* combine.c (make_extraction): Update use of GET_MODE_WIDER_MODE,
forcing a wider mode to exist.
* config/cr16/cr16.h (LONG_REG_P): Likewise.
* rtlanal.c (init_num_sign_bit_copies_in_rep): Likewise.
* config/c6x/c6x.c (c6x_rtx_costs): Update use of
GET_MODE_2XWIDER_MODE, forcing a wider mode to exist.
* lower-subreg.c (init_lower_subreg): Likewise.
* optabs-libfuncs.c (init_sync_libfuncs_1): Likewise, but not
on the final iteration.
* config/i386/i386.c (ix86_expand_set_or_movmem): Check whether
a wider mode exists before asking for a move pattern.
(get_mode_wider_vector): Update use of GET_MODE_WIDER_MODE,
forcing a wider mode to exist.
(expand_vselect_vconcat): Update use of GET_MODE_2XWIDER_MODE,
returning false if no such mode exists.
* config/ia64/ia64.c (expand_vselect_vconcat): Likewise.
* config/mips/mips.c (mips_expand_vselect_vconcat): Likewise.
* expmed.c (init_expmed_one_mode): Update use of GET_MODE_WIDER_MODE.
Avoid checking for a MODE_INT if we already know the mode is not a
SCALAR_INT_MODE_P.
(extract_high_half): Update use of GET_MODE_WIDER_MODE,
forcing a wider mode to exist.
(expmed_mult_highpart_optab): Likewise.
(expmed_mult_highpart): Likewise.
* expr.c (expand_expr_real_2): Update use of GET_MODE_WIDER_MODE,
using else_void.
* lto-streamer-in.c (lto_input_mode_table): Likewise.
* optabs-query.c (find_widening_optab_handler_and_mode): Likewise.
* stor-layout.c (bit_field_mode_iterator::next_mode): Likewise.
* internal-fn.c (expand_mul_overflow): Update use of
GET_MODE_2XWIDER_MODE.
* omp-low.c (omp_clause_aligned_alignment): Likewise.
* tree-ssa-math-opts.c (convert_mult_to_widen): Update use of
GET_MODE_WIDER_MODE.
(convert_plusminus_to_widen): Likewise.
* tree-switch-conversion.c (array_value_type): Likewise.
* var-tracking.c (emit_note_insn_var_location): Likewise.
* tree-vrp.c (simplify_float_conversion_using_ranges): Likewise.
Return false inside rather than outside the loop if no wider mode
exists
* optabs.c (expand_binop): Update use of GET_MODE_WIDER_MODE
and GET_MODE_2XWIDER_MODE
(can_compare_p): Use else_void.
* gdbhooks.py (OptMachineModePrinter): New class.
(build_pretty_printer): Use it for opt_mode.

gcc/ada/
* gcc-interface/decl.c (validate_size): Update use of
GET_MODE_WIDER_MODE, forcing a wider mode to exist.

Index: gcc/coretypes.h
===
--- gcc/coretypes.h 2017-07-02 10:05:20.995639436 +0100
+++ gcc/coretypes.h 2017-07-13 09:18:22.924279021 +0100
@@ -55,6 +55,7 @@ typedef const struct simple_bitmap_def *
 struct rtx_def;
 typedef struct rtx_def *rtx;
 typedef const struct rtx_def *const_rtx;
+template class opt_mode;
 
 /* Subclasses of rtx_def, using indentation to show the class
hierarchy, along with the relevant invariant.
Index: gcc/machmode.h
===
--- gcc/machmode.h  2017-07-13 09:18:21.533429040 +0100
+++ gcc/machmode.h  2017-07-13 09:18:22.930278376 +0100
@@ -221,6 +221,72 @@ #define CLASS_HAS_WIDER_MODES_P(CLASS)
 #define POINTER_BOUNDS_MODE_P(MODE)  \
   (GET_MODE_CLASS (MODE) == MODE_POINTER_BOUNDS)
 
+/* An optional T (i.e. a T or nothing), where T is some form of mode class.
+   operator * gives the T value.  */
+template
+class opt_mode
+{
+public:
+  enum from_int { dummy = MAX_MACHINE_MODE };
+
+  ALWAYS_INLINE opt_mode () : m_mode (E_VOIDmode) {}
+  ALWAYS_INLINE opt_mode (const T ) : m_mode (m) {}
+  ALWAYS_INLINE opt_mode (from_int m) : m_mode (machine_mode (m)) {}
+
+  machine_mode else_void () const;
+  T operator * () const;
+
+  bool exists () const;
+  template bool