On Thu, Oct 5, 2017 at 4:41 PM, Laurent Thevenoux
<laurent.theven...@inria.fr> wrote:
> Hello,
>
> This patch improves the some_nonzerop(tree t) function from tree-complex.c 
> file (the function is only used there).
>
> This function returns true if a tree as a parameter is not the constant 0 (or 
> +0.0 only for reals when !flag_signed_zeros ). The former result is then used 
> to determine if some simplifications are possible for complex expansions 
> (addition, multiplication, and division).
>
> Unfortunately, if the tree is a real constant, the function always return 
> true, even for +0.0 because of the explicit test on flag_signed_zeros (so if 
> your system enables signed zeros you cannot benefit from those 
> simplifications). To avoid this behavior and allow complex expansion 
> simplifications, I propose the following patch, which test for the sign of 
> the real constant 0.0 instead of checking the flag.

But

+  if (TREE_CODE (t) == REAL_CST)
+    {
+      if (real_identical (&TREE_REAL_CST (t), &dconst0))
+       zerop = !real_isneg (&TREE_REAL_CST (t));
+    }

shouldn't you do

   zerop = !flag_signed_zeros || !real_isneg (&TREE_REAL_CST (t));

?

Also doesn't this miscalculate A - B for {ar, 0.} - {br, 0.} given the
simplification simply
returns bi?

So I'm not convinced this handling is correct.

> This first fix reveals a bug (thanks to 
> c-c++-common/torture/complex-sign-add.c ) in the simplification section of 
> expand_complex_addition (also fixed in the patch).

Your patch looks backward and the existing code looks ok.

@@ -937,7 +940,7 @@ expand_complex_addition (gimple_stmt_iterator
*gsi, tree inner_type,

     case PAIR (VARYING, ONLY_REAL):
       rr = gimplify_build2 (gsi, code, inner_type, ar, br);
-      ri = ai;
+      ri = bi;
       break;

down we have

    case PAIR (ONLY_REAL, VARYING):
      if (code == MINUS_EXPR)
        goto general;
      rr = gimplify_build2 (gsi, code, inner_type, ar, br);
      ri = bi;
      break;

which for sure would need to change as well then.  But for your
changed code we know
bi is zero (but ai may not).

Richard.

> The patch has passed bootstrap and testing on x86_64-pc-linux-gnu .
>
> Best regards,
> Laurent Thévenoux

Reply via email to