[PATCH] Handle CALL_INSN_FUNCTION_USAGE clobbers in regcprop.c

[Tom de Vries <Tom_deVries at mentor dot com>]

Jakub,

Attached patch handles CALL_INSN_FUNCTION_USAGE clobbers in 
copyprop_hardreg_forward_1.

Terry reported a cprop_hardreg misbehaviour here ( 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64154#c2 ), in the context of 
trying out -fipa-ra for thumb1. The -fipa-ra flag is currently disabled for 
thumb1, but due to an AFAIU unrelated issue.

The problem is that when cprop_hardreg processes a call_insn with a clobber in 
CALL_INSN_FUNCTION_USAGE, the clobber is not taken into account.

So for this call instruction, the 'clobber (reg:SI 12 ip)' is ignored:
...
(call_insn 141 292 142 13
 (parallel
  [
   (call (mem:SI
	  (symbol_ref:SI ("f2") [flags 0x3]  <function_decl  f2>)
	  [0 f2 S4 A32])
    (const_int 0 [0]))
   (use (const_int 0 [0]))
   (clobber (reg:SI 14 lr))
   ])
 vshift-3.c:119
 770 {*call_insn}
 (expr_list:REG_CALL_DECL (symbol_ref:SI ("f2") [flags 0x3]  <function_decl f2>)
  (expr_list:REG_EH_REGION (const_int 0 [0])
   (nil)))
 (expr_list (clobber (reg:SI 12 ip))
  (nil)))
...

This results in cprop_hardreg using register ip during the call, which is incorrect:
...
(insn (set (reg:SI 4 r4)
           (reg:SI 12 ip)))

(call_insn 141 ... )

(insn (set (reg:SI 0 r0)
           (reg:SI 12 ip)))
...

I have not been able to reproduce the failing code.  But I was able to observe 
that the clobber in CALL_INSN_FUNCTION_USAGE was ignored by 
copyprop_hardreg_forward_1. My understanding is that this is a latent bug in 
cprop_hardreg, uncovered by -fipa-ra.


Actually, there is code in copyprop_hardreg_forward_1 to handle clobbers in 
CALL_INSN_FUNCTION_USAGE, but that is part of the fix for PR57003, where we 
re-apply clobbers:
...
	  /* If SET was seen in CALL_INSN_FUNCTION_USAGE, and SET_SRC
	     of the SET isn't in regs_invalidated_by_call hard reg set,
	     but instead among CLOBBERs on the CALL_INSN, we could wrongly
	     assume the value in it is still live.  */
	  if (ksvd.ignore_set_reg)
	    {
	      note_stores (PATTERN (insn), kill_clobbered_value, vd);
	      for (exp = CALL_INSN_FUNCTION_USAGE (insn);
		   exp;
		   exp = XEXP (exp, 1))
		{
		  rtx x = XEXP (exp, 0);
		  if (GET_CODE (x) == CLOBBER)
		    kill_value (SET_DEST (x), vd);
		}
	    }
...
[ This code is related to a scenario where the called function returns one of 
it's arguments and is annotated as such, but that doesn't apply to this example. ]

However, the earlier application of the clobbers just handles the clobbers in 
PATTERN, not those in CALL_INSN_FUNCTION_USAGE:
...
      /* Within asms, a clobber cannot overlap inputs or outputs.
	 I wouldn't think this were true for regular insns, but
	 scan_rtx treats them like that...  */
      note_stores (PATTERN (insn), kill_clobbered_value, vd);
...

This patch basically adds the same CALL_INSN_FUNCTION_USAGE for loop after the 
earlier 'note_stores (PATTERN (insn), kill_clobbered_value, vd)' call.

Terry reported that the patch fixes the problem for him.

Bootstrapped and reg-tested on x86_64, no issues found.

OK for stage3 trunk?

Thanks,
- Tom

2015-01-09  Tom de Vries  <tom@codesourcery.com>

	PR rtl-optimization/64539
	* regcprop.c (copyprop_hardreg_forward_1): Handle clobbers in
	CALL_INSN_FUNCTION_USAGE.
---
 gcc/regcprop.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/gcc/regcprop.c b/gcc/regcprop.c
index 8c4f564..b42a4b7 100644
--- a/gcc/regcprop.c
+++ b/gcc/regcprop.c
@@ -801,6 +801,18 @@ copyprop_hardreg_forward_1 (basic_block bb, struct value_data *vd)
 	 I wouldn't think this were true for regular insns, but
 	 scan_rtx treats them like that...  */
       note_stores (PATTERN (insn), kill_clobbered_value, vd);
+      if (CALL_P (insn))
+	{
+	  rtx exp;
+	  for (exp = CALL_INSN_FUNCTION_USAGE (insn);
+	       exp;
+	       exp = XEXP (exp, 1))
+	    {
+	      rtx x = XEXP (exp, 0);
+	      if (GET_CODE (x) == CLOBBER)
+		kill_value (SET_DEST (x), vd);
+	    }
+	}
 
       /* Kill all auto-incremented values.  */
       /* ??? REG_INC is useless, since stack pushes aren't done that way.  */
-- 
1.9.1