Re: Question about IPA and ordering of terms

[Richard Biener <richard dot guenther at gmail dot com>]

On June 28, 2017 6:10:27 PM GMT+02:00, Tamar Christina <Tamar.Christina@arm.com> wrote:
>Hi All,
>
>I noticed something weird with IPA and I'm wondering if it's a bug or
>not.
>
>Given these two functions
>
>double
>check_d_pos (double x, double y)
>{
>  return x * __builtin_copysign (1.0, y);
>}
>
>double
>check_d_neg (double x, double y)
>{
>  return __builtin_copysign (-1.0, y) * x;
>}
>
>and current trunk I get as expected the following GIMPLE tree
>
>check_d_pos (double x, double y)
>{
>  doubleD.31 D.3134;
>
>  _1 = copysignD.107 (1.0e+0, yD.3126);
>  D.3134 = xD.3125 * _1;
>  return D.3134;
>}
>
>
>check_d_neg (double x, double y)
>{
>  doubleD.31 D.3136;
>
>  _1 = copysignD.107 (1.0e+0, yD.3130);
>  D.3136 = xD.3129 * _1;
>  return D.3136;
>}
>
>GIMPLE seems to have the invariant that all variables appear before
>anything else? so it's correctly gimplified the functions in a manner
>that makes them the same.
>
>however by the time it gets to expand something odd has happend:
>
>fix_loop_structure: fixing up loops for function
>check_d_pos (doubleD.31 xD.3125, doubleD.31 yD.3126)
>{
>  doubleD.31 x_3(D) = xD.3125;
>  doubleD.31 y_2(D) = yD.3126;
>  doubleD.31 _1;
>  doubleD.31 _4;
>
>;;   basic block 2, loop depth 0, freq 10000, maybe hot
>;;    prev block 0, next block 1, flags: (NEW, REACHABLE, VISITED)
>;;    pred:       ENTRY [100.0%]  (FALLTHRU,EXECUTABLE)
>  _1 = copysignD.107 (1.0e+0, y_2(D));
>  _4 = _1 * x_3(D);
>  # VUSE <.MEM_5(D)>
>  return _4;
>;;    succ:       EXIT [100.0%]  (EXECUTABLE)
>
>}
>
>check_d_neg (doubleD.31 xD.3129, doubleD.31 yD.3130)
>{
>  doubleD.31 x_1(D) = xD.3129;
>  doubleD.31 y_2(D) = yD.3130;
>  doubleD.31 D.3153;
>  doubleD.31 retval.2D.3143;
>  doubleD.31 _5;
>  doubleD.31 _6;
>
>;;   basic block 2, loop depth 0, freq 10000, maybe hot
>;;    prev block 0, next block 1, flags: (NEW, REACHABLE)
>;;    pred:       ENTRY [100.0%]  (FALLTHRU)
>  _5 = copysignD.107 (1.0e+0, y_2(D));
>  _6 = x_1(D) * _5;
>  _7 = _6;
>  retval.2_3 = _7;
>  # VUSE <.MEM_4(D)>
>  return retval.2_3;
>;;    succ:       EXIT [100.0%] 
>
>}
>
>noticed it has swapped the order of the terms to the *. This
>re-ordering seems to make it miss optimizations in expr.c e.g.
>expand_expr_real_2
>which I think still rely on the invariant GIMPLE first introduced.
>
>What's happening is that IPA decides that the functions are similar
>enough and replaces the body of one of the functions with a call to the
>other:
>
>check_d_neg (double x, double y)
>{
>  double retval.2;
>
>  <bb 2> [100.00%] [count: INV]:
>  retval.2_3 = check_d_pos (x_1(D), y_2(D)); [tail call]
>  return retval.2_3;
>
>}
>
>check_d_pos (double x, double y)
>{
>  double _1;
>  double _4;
>
>  <bb 2> [100.00%] [count: INV]:
>  _1 = __builtin_copysign (1.0e+0, y_2(D));
>  _4 = _1 * x_3(D);
>  return _4;
>
>}
>
>It then later decides to undo this and so generates a different order.
>Question is, is this unexpected or should optimizations in expand be
>checking for associativity?

It's expected.  Once fully in SSA the canonical operand order is lower SSA name versions first.

Richard.

>Thanks,
>Tamar