diff mbox

[PR61605,1/2] Handle copy cycles in pass_cprop_hardreg

Message ID 543F8C56.3060601@mentor.com
State New
Headers show

Commit Message

Tom de Vries Oct. 16, 2014, 9:13 a.m. UTC
Eric,

this patch is the first half of the fix for PR61605.

The problem it addresses is the following: Consider this copy cycle (a = b; b = a):
...
(insn 2 18 3 2 (set (reg/v:SI 1 dx [orig:86 yD.1749 ] [86])
         (reg:SI 5 di [ yD.1749 ])) test.c:9 90 {*movsi_internal}
      (expr_list:REG_DEAD (reg:SI 5 di [ yD.1749 ])
         (nil)))
(note 3 2 6 2 NOTE_INSN_FUNCTION_BEG)
(insn 6 3 7 2 (set (reg:SI 5 di)
         (reg/v:SI 1 dx [orig:86 yD.1749 ] [86])) test.c:10 90 {*movsi_internal}
      (nil))
...

cprop_hardreg handles this currently in the following way:
- it processes the first copy, and sets up di as representant of dx.
- it then processes the second copy, and replaces the dx with di:
   ...
   (insn 6 3 7 2 (set (reg:SI 5 di)
	(reg:SI 5 di [orig:86 yD.1749 ] [86])) test.c:10 90 {*movsi_internal}
      (nil))
   ...
   turning it into a noop.

pass_fast_rtl_dce subsequently removes the noop.

However, while processing the second copy, it considers the set of di in insn 6 
as killing, and removes di as representant of dx. So a use of dx in a following 
insn is not replaced by di.

By running pass_cprop_hardreg once more after pass_fast_rtl_dce, we do manage to 
replace the use of dx in a following insn by di.

This patch achieves the same, without rerunning pass_cprop_hardreg. It ensures 
in copyprop_hardreg_forward_1 that the set of a dest by a noop is not considered 
killing.

Bootstrapped and reg-tested on x86_64.

OK for trunk?

Thanks,
- Tom

Comments

Jeff Law Oct. 16, 2014, 9:57 p.m. UTC | #1
On 10/16/14 03:13, Tom de Vries wrote:
> Eric,
>
> this patch is the first half of the fix for PR61605.
>
> The problem it addresses is the following: Consider this copy cycle (a =
> b; b = a):
> ...
> (insn 2 18 3 2 (set (reg/v:SI 1 dx [orig:86 yD.1749 ] [86])
>          (reg:SI 5 di [ yD.1749 ])) test.c:9 90 {*movsi_internal}
>       (expr_list:REG_DEAD (reg:SI 5 di [ yD.1749 ])
>          (nil)))
> (note 3 2 6 2 NOTE_INSN_FUNCTION_BEG)
> (insn 6 3 7 2 (set (reg:SI 5 di)
>          (reg/v:SI 1 dx [orig:86 yD.1749 ] [86])) test.c:10 90
> {*movsi_internal}
>       (nil))
> ...
>
> cprop_hardreg handles this currently in the following way:
> - it processes the first copy, and sets up di as representant of dx.
> - it then processes the second copy, and replaces the dx with di:
>    ...
>    (insn 6 3 7 2 (set (reg:SI 5 di)
>      (reg:SI 5 di [orig:86 yD.1749 ] [86])) test.c:10 90 {*movsi_internal}
>       (nil))
>    ...
>    turning it into a noop.
>
> pass_fast_rtl_dce subsequently removes the noop.
>
> However, while processing the second copy, it considers the set of di in
> insn 6 as killing, and removes di as representant of dx. So a use of dx
> in a following insn is not replaced by di.
>
> By running pass_cprop_hardreg once more after pass_fast_rtl_dce, we do
> manage to replace the use of dx in a following insn by di.
>
> This patch achieves the same, without rerunning pass_cprop_hardreg. It
> ensures in copyprop_hardreg_forward_1 that the set of a dest by a noop
> is not considered killing.
>
> Bootstrapped and reg-tested on x86_64.
>
> OK for trunk?
>
> Thanks,
> - Tom
>
> 0001-Don-t-regard-a-copy-with-identical-src-and-dest-as-k.patch
>
>
> 2014-10-13  Tom de Vries<tom@codesourcery.com>
>
> 	PR rtl-optimization/61605
> 	* regcprop.c (copyprop_hardreg_forward_1): Add copy_p and noop_p.  Don't
> 	notice stores for noops.  Don't regard noops as copies.
OK.
Jeff
diff mbox

Patch

2014-10-13  Tom de Vries  <tom@codesourcery.com>

	PR rtl-optimization/61605
	* regcprop.c (copyprop_hardreg_forward_1): Add copy_p and noop_p.  Don't
	notice stores for noops.  Don't regard noops as copies.

diff --git a/gcc/regcprop.c b/gcc/regcprop.c
index 3297721..c71de98 100644
--- a/gcc/regcprop.c
+++ b/gcc/regcprop.c
@@ -1032,12 +1032,21 @@  copyprop_hardreg_forward_1 (basic_block bb, struct value_data *vd)
 	    note_stores (PATTERN (insn), kill_clobbered_value, vd);
 	}
 
-      /* Notice stores.  */
-      note_stores (PATTERN (insn), kill_set_value, &ksvd);
+      bool copy_p = (set
+		     && REG_P (SET_DEST (set))
+		     && REG_P (SET_SRC (set)));
+      bool noop_p = (copy_p
+		     && rtx_equal_p (SET_DEST (set), SET_SRC (set)));
 
-      /* Notice copies.  */
-      if (set && REG_P (SET_DEST (set)) && REG_P (SET_SRC (set)))
-	copy_value (SET_DEST (set), SET_SRC (set), vd);
+      if (!noop_p)
+	{
+	  /* Notice stores.  */
+	  note_stores (PATTERN (insn), kill_set_value, &ksvd);
+
+	  /* Notice copies.  */
+	  if (copy_p)
+	    copy_value (SET_DEST (set), SET_SRC (set), vd);
+	}
 
       if (insn == BB_END (bb))
 	break;
-- 
1.9.1