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

[Richard Sandiford <richard.sandiford@linaro.org>]

Jeff Law <law@redhat.com> writes:
> On 08/29/2017 09:01 AM, Richard Sandiford wrote:
>> Jeff Law <law@redhat.com> 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