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.

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

Reply via email to