Handle CALL_INSN_FUNCTION_USAGE clobbers in regcprop.c

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

On Fri, Jan 09, 2015 at 11:35:41AM +0100, Tom de Vries wrote:
> 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.

To avoid the duplication, wouldn't it be better to add

static void
kill_clobbered_values (rtx_insn *insn, struct value_data *vd)
{
  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);
	}
    }
}
function (with appropriate function comment) and use it in both places?

Otherwise LGTM.

	Jakub