On Wed, Dec 11, 2013 at 3:51 PM, Jeff Law <l...@redhat.com> wrote:
> On 12/11/13 02:51, Richard Biener wrote:
>>
>>
>> First of all phiopt runs unconditionally for -On with n > 0 but the
>> conversion
>> is clearly not suitable for non-speed optimizations. Thus I'd guard it
>> with at least !optimize_size && optimize >= 2. As you are targeting
>> a worse transformation done by if-conversion you may want to
>> add || (((flag_tree_loop_vectorize || cfun->has_force_vect_loops)
>> && flag_tree_loop_if_convert != 0)
>> || flag_tree_loop_if_convert == 1
>> || flag_tree_loop_if_convert_stores == 1)
>> (ugh).
>
> That's a hell of a condition to guard a transformation. But, yea, I agree.
>
>
>>> +
>>> + /* If inversion is needed, first try to invert the test since
>>> + that's cheapest. */
>>> + if (invert)
>>> + {
>>> + enum tree_code new_code
>>> + = invert_tree_comparison (cond_code,
>>> + HONOR_NANS (TYPE_MODE (TREE_TYPE
>>> (rhs))));
>>
>>
>> That looks wrong - you want to look at HONOR_NANS on the mode
>> of one of the comparison operands, not of the actual value you want
>> to negate (it's integer and thus never has NaNs).
>
> Bah. That was supposed to be HONOR_SIGNED_ZEROS. Which as far as I can
> tell is a property of the value being tested.
No, it's
invert_tree_comparison (enum tree_code code, bool honor_nans)
so indeed HONOR_NANS. And yes, on a conditional argument
(it can be a FP comparison but a integer negate).
Richard.
> So it seems to me it should be
>
> HONOR_SIGNED_ZEROS (TYPE_MODE (TREE_TYPE (one of the conditional's
> arguments)))
>
> much like is done in abs_replacement. With the difference we want to look
> at the comparison (which may have different arguments than the PHI we're
> converting) and that we can still apply the optimization, just in a slightly
> different way.
>
> jeff
>
