[Bug rtl-optimization/43491] Unnecessary temporary for global register variable

[Bug rtl-optimization/43491] Unnecessary temporary for global register variable

[amker.cheng at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43491

amker.cheng <amker.cheng at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |amker.cheng at gmail dot
                   |                            |com

--- Comment #2 from amker.cheng <amker.cheng at gmail dot com> 2011-11-23 05:50:51 UTC ---
Noticed that pass 097t.copyprop4 propagates reg.0_12 to statement Y in
following dump:
-------------------------------------
<bb 2>:
  reg.0_12 = reg;
  D.4705_13 = MEM[(unsigned int *)reg.0_12 + 8B];   <-----statement Z
  if (D.4705_13 != 0)
    goto <bb 3>;
  else
    goto <bb 6>;

<bb 3>:

<bb 4>:
  c ();
  reg.0_1 = reg.0_12;                                                 
<-----statement X
  D.4705_3 = MEM[(unsigned int *)reg.0_1 + 8B]; <-----statement Y
  if (D.4705_3 != 0)
    goto <bb 5>;
  else
    goto <bb 6>;

<bb 5>:
  goto <bb 4>;

<bb 6>:
  return;
-------------------------------------
to be:
  reg.0_1 = reg.0_12;                                                  
<-----statement X
  D.4705_3 = MEM[(unsigned int *)reg.0_12 + 8B]; <-----statement Y

So, should it propagates reg directly? Could this be done on ssa?

Also I found 
1) there are similar cases on redundant copy or load constant, for example,
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44025
2) some of these cases are generated after expanding into rtl;
3) redundant copy might be handled in IRA, but redundant load const might be
more difficult.

How about extending regcprop.c pass into a global pass?

[Bug rtl-optimization/43491] Unnecessary temporary for global register variable

[amker.cheng at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43491

--- Comment #3 from amker.cheng <amker.cheng at gmail dot com> 2011-11-24 09:24:37 UTC ---
(In reply to comment #1)

> 
> I'm thinking that this is perfectly normal thing to do, and that the redundant
> move is meant to disappear in a later pass.  My guess is that IRA is choosing
> not to assign the pseudo to r4, but I do not know why at the moment.

As dump in 191r.shed1:
------------------------------------------
(insn 5 7 6 2 (set (reg/f:SI 135 [ reg.0 ])
        (reg/v:SI 4 r4 [ reg ])) pr43491.c:16 709 {*thumb2_movsi_insn}
     (expr_list:REG_DEAD (reg/v:SI 4 r4 [ reg ])
        (nil)))

(insn 6 5 8 2 (set (reg:SI 137 [ MEM[(unsigned int *)reg.0_12 + 8B] ])
        (mem:SI (plus:SI (reg/f:SI 135 [ reg.0 ])
                (const_int 8 [0x8])) [2 MEM[(unsigned int *)reg.0_12 + 8B]+0 S4
A32])) pr43491.c:16 709 {*thumb2_movsi_insn}
     (nil))

(jump_insn 8 6 49 2 (parallel [
            (set (pc)
                (if_then_else (eq (reg:SI 137 [ MEM[(unsigned int *)reg.0_12 +
8B] ])
                        (const_int 0 [0]))
                    (label_ref:SI 22)
                    (pc)))
            (clobber (reg:CC 24 cc))
        ]) pr43491.c:16 747 {*thumb2_cbz}
     (expr_list:REG_DEAD (reg:SI 137 [ MEM[(unsigned int *)reg.0_12 + 8B] ])
        (expr_list:REG_UNUSED (reg:CC 24 cc)
            (expr_list:REG_BR_PROB (const_int 900 [0x384])
                (nil))))
 -> 22)

(code_label 49 8 48 3 4 "" [1 uses])

(note 48 49 16 3 [bb 3] NOTE_INSN_BASIC_BLOCK)

(note 16 48 14 3 NOTE_INSN_DELETED)

(call_insn 14 16 15 3 (parallel [
            (call (mem:SI (symbol_ref:SI ("c") [flags 0x41]  <function_decl
0xb76c3b80 c>) [0 c S4 A32])
                (const_int 0 [0]))
            (use (const_int 0 [0]))
            (clobber (reg:SI 14 lr))
        ]) pr43491.c:17 247 {*call_symbol}
     (nil)
    (nil))

(insn 15 14 17 3 (set (reg:SI 138 [ MEM[(unsigned int *)reg.0_12 + 8B] ])
        (mem:SI (plus:SI (reg/f:SI 135 [ reg.0 ])
                (const_int 8 [0x8])) [2 MEM[(unsigned int *)reg.0_12 + 8B]+0 S4
A32])) pr43491.c:16 709 {*thumb2_movsi_insn}
     (nil))
------------------------------------------
Since reg is manually declared in r4, function globalize_reg sets r4 in
fixed_reg_set/call_used_reg_set/call_fixed_reg_set. IRA then add r4 into
allocno(r135)'s conflict_hard_regs. That's why IRA not assigns the pseudo(r135)
to r4. I guess it's natural unless we can make IRA aware of constant register.

[Bug rtl-optimization/43491] Unnecessary temporary for global register variable

[amker.cheng at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43491

--- Comment #4 from amker.cheng <amker.cheng at gmail dot com> 2011-12-21 03:44:03 UTC ---
This bug is even worse on mips.

The cause is ssa-pre eliminates global register variable when it is the RHS of
single assign statment, while following passes do not handle the const/register
attributes of the variable.
It can be handled in tree-ssa-pre.c without hurting true redundancy elimination
on global register variables.

So could somebody change the tag from rtl-optimization to tree-optimization?

[Bug tree-optimization/43491] Unnecessary temporary for global register variable

[ubizjak at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43491

Uros Bizjak <ubizjak at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|rtl-optimization            |tree-optimization
      Known to fail|                            |

--- Comment #5 from Uros Bizjak <ubizjak at gmail dot com> 2011-12-21 07:54:15 UTC ---
(In reply to comment #4)

> So could somebody change the tag from rtl-optimization to tree-optimization?

Done.

[Bug tree-optimization/43491] Unnecessary temporary for global register variable

[jye2 at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43491

--- Comment #6 from jye2 at gcc dot gnu.org 2011-12-23 05:43:17 UTC ---
Author: jye2
Date: Fri Dec 23 05:43:09 2011
New Revision: 182650

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=182650
Log:
2011-12-22  Bin Cheng  <bin.cheng@arm.com>
    Richard Guenther  <rguenther@suse.de>

    PR tree-optimization/43491
    * tree-ssa-pre.c (eliminate): Don't replace global register
    variable when it is the RHS of a single assign.

    testsuite:
    * gcc.dg/tree-ssa/pr43491.c: New test.


Added:
    trunk/gcc/testsuite/gcc.dg/tree-ssa/pr43491.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-pre.c

[Bug tree-optimization/43491] Unnecessary temporary for global register variable

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43491

--- Comment #7 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-02-16 14:43:51 UTC ---
With tree hoisting we generate

<bb 2>:
  pretmp.5_19 = data_0;
  pretmp.5_20 = data_3;
  i_21 = pretmp.5_19 + pretmp.5_20;
  if (data_3(D) != 0)
    goto <bb 3>;
  else
    goto <bb 4>;

<bb 3>:

<bb 4>:
  # v_1 = PHI <v_7(D)(3), 2(2)>
  # i_2 = PHI <i_21(3), 5(2)>
  D.1719_14 = v_1 * i_21;
  D.1718_15 = i_2 * D.1719_14;
  return D.1718_15;

instead of

<bb 2>:
  if (data_3(D) != 0)
    goto <bb 4>;
  else
    goto <bb 3>;

<bb 3>:
  pretmp.5_19 = data_0;
  pretmp.5_21 = data_3;
  i_23 = pretmp.5_19 + pretmp.5_21;
  goto <bb 5>;

<bb 4>:
  data_0.0_4 = data_0;
  data_3.1_5 = data_3;
  i_6 = data_0.0_4 + data_3.1_5;

<bb 5>:
  # v_1 = PHI <v_7(D)(4), 2(3)>
  # i_2 = PHI <i_6(4), 5(3)>
  # i_24 = PHI <i_6(4), i_23(3)>
  D.1719_14 = v_1 * i_24;
  D.1718_15 = i_2 * D.1719_14;
  return D.1718_15;

}

I suppose that's good enough?  See that PRE still inserts loads from
register variables, not sure if you'd want to disallow that as well.

[Bug tree-optimization/43491] Unnecessary temporary for global register variable

[amker.cheng at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43491

--- Comment #8 from amker.cheng <amker.cheng at gmail dot com> 2012-02-17 03:55:24 UTC ---
(In reply to comment #7)
> With tree hoisting we generate
> 
> <bb 2>:
>   pretmp.5_19 = data_0;
>   pretmp.5_20 = data_3;
>   i_21 = pretmp.5_19 + pretmp.5_20;
>   if (data_3(D) != 0)
>     goto <bb 3>;
>   else
>     goto <bb 4>;
> 
> <bb 3>:
> 
> <bb 4>:
>   # v_1 = PHI <v_7(D)(3), 2(2)>
>   # i_2 = PHI <i_21(3), 5(2)>
>   D.1719_14 = v_1 * i_21;
>   D.1718_15 = i_2 * D.1719_14;
>   return D.1718_15;
> 
> instead of
> 
> <bb 2>:
>   if (data_3(D) != 0)
>     goto <bb 4>;
>   else
>     goto <bb 3>;
> 
> <bb 3>:
>   pretmp.5_19 = data_0;
>   pretmp.5_21 = data_3;
>   i_23 = pretmp.5_19 + pretmp.5_21;
>   goto <bb 5>;
> 
> <bb 4>:
>   data_0.0_4 = data_0;
>   data_3.1_5 = data_3;
>   i_6 = data_0.0_4 + data_3.1_5;
> 
> <bb 5>:
>   # v_1 = PHI <v_7(D)(4), 2(3)>
>   # i_2 = PHI <i_6(4), 5(3)>
>   # i_24 = PHI <i_6(4), i_23(3)>
>   D.1719_14 = v_1 * i_24;
>   D.1718_15 = i_2 * D.1719_14;
>   return D.1718_15;
> 
> }
> 
> I suppose that's good enough?  See that PRE still inserts loads from
> register variables, not sure if you'd want to disallow that as well.

I think the reason why gcc inserts loads from global register variable is gcc
treats loads/uses of such variable as memory references. If I am right, It
seems a ssa issue, rather than PRE.
As for the original bug, it is caused by loading const global register
variable, then using the loaded ssa var across function calls(this step by
pre), which introduces unnecessary register conflict. I guess the load itself
won't hurt, but not sure whether hoisting will(as pre had done before).

BTW, I did not get the hoisted code on trunk. Is it a patch your are working
on?

Thanks.

[Bug tree-optimization/43491] Unnecessary temporary for global register variable

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43491

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rguenth at gcc dot gnu.org

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
As said, the fix in comment #6 isn't really effective and I intend to basically
revert it.

But I'd like to have guidance on what transforms people think are ok for
global register vars - esp. what is "true redundancy elimination"?  Any
redundancy elimination can cause the extension of the lifetime of the
temporaries we create and thus increase register pressure.

Ideally we'd treat global register variables by rewriting them into SSA
form but avoiding overlapping life ranges.  At the moment we get those
extra "temporaries" by means of gimple restrictions which see global
register vars as memory.

Note that you can reliably prevent any "disturbing" transforms of global
register vars by declaring them volatile.

I suppose the real issue is that GCC inserts/moves sets of the global
register variable.  CSE across function calls could be easily inhibited
as well.