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