Re: fix long-term -fgcse-sm bug

[Steven Bosscher <stevenb dot gcc at gmail dot com>]

On Tuesday 10 April 2007 04:16, Alexandre Oliva wrote:
> :ADDPATCH gcse-sm:
>
> The patch below fixes a very long-term bug in store motion.  We failed
> to look for stores inside PARALLELs when looking for kills.  Oops ;-)
>
> There's an x86-only testcase, but I suppose it might be possible to
> construct testcases for other platforms.
>
> Bootstrapped on x86_64-linux-gnu with -fgcse-sm (without it, the patch
> would be an obvious nop ;-)
>
> Ok to install?

Hi Alex,

First of all: Good catch!
But unfortunately, as rth already pointed out the loop you crafted
took a bit of effort to dig.

Can you see if the attached (untested) patch also works for you?

Thanks,

Gr.
Steven


	* gcse.c (store_killed_in_pat): New function split out from...
	(store_killed_in_insn): ...here.  Use the new function to look
	at SETs inside PARALLELs.

Index: gcse.c
===================================================================
*** gcse.c	(revision 123799)
--- gcse.c	(working copy)
*************** find_loads (rtx x, rtx store_pattern, in
*** 5909,5914 ****
--- 5909,5948 ----
    return ret;
  }
  
+ /* Return true if PAT kills the store in X.  PAT is a SET in the
+    pattern of an insn.
+    AFTER is true if we are checking the case when store X occurs
+    after the insn.  */
+ 
+ static bool
+ store_killed_in_pat (rtx x, rtx pat, int after)
+ {
+   rtx dest = SET_DEST (pat);
+  
+   if (GET_CODE (dest) == ZERO_EXTRACT)
+     dest = XEXP (dest, 0);
+ 
+   /* Check for memory stores to aliased objects.  */
+   if (MEM_P (dest)
+       && !expr_equiv_p (dest, x))
+     {
+       if (after)
+ 	{
+ 	  if (output_dependence (dest, x))
+ 	    return true;
+ 	}
+       else
+ 	{
+ 	  if (output_dependence (x, dest))
+ 	    return true;
+ 	}
+     }
+   if (find_loads (SET_SRC (pat), x, after))
+     return true;
+ 
+   return false;
+ }
+ 
  /* Check if INSN kills the store pattern X (is aliased with it).
     AFTER is true if we are checking the case when store X occurs
     after the insn.  Return true if it does.  */
*************** find_loads (rtx x, rtx store_pattern, in
*** 5916,5922 ****
  static bool
  store_killed_in_insn (rtx x, rtx x_regs, rtx insn, int after)
  {
!   rtx reg, base, note;
  
    if (!INSN_P (insn))
      return false;
--- 5950,5957 ----
  static bool
  store_killed_in_insn (rtx x, rtx x_regs, rtx insn, int after)
  {
!   rtx reg, base, note, pat;
!   int i;
  
    if (!INSN_P (insn))
      return false;
*************** store_killed_in_insn (rtx x, rtx x_regs,
*** 5943,5973 ****
        return false;
      }
  
!   if (GET_CODE (PATTERN (insn)) == SET)
      {
!       rtx pat = PATTERN (insn);
!       rtx dest = SET_DEST (pat);
! 
!       if (GET_CODE (dest) == ZERO_EXTRACT)
! 	dest = XEXP (dest, 0);
! 
!       /* Check for memory stores to aliased objects.  */
!       if (MEM_P (dest)
! 	  && !expr_equiv_p (dest, x))
  	{
! 	  if (after)
! 	    {
! 	      if (output_dependence (dest, x))
! 		return true;
! 	    }
! 	  else
! 	    {
! 	      if (output_dependence (x, dest))
! 		return true;
! 	    }
  	}
-       if (find_loads (SET_SRC (pat), x, after))
- 	return true;
      }
    else if (find_loads (PATTERN (insn), x, after))
      return true;
--- 5978,5996 ----
        return false;
      }
  
!   pat = PATTERN (insn);
!   if (GET_CODE (pat) == SET
!       && store_killed_in_pat (x, pat, after))
!     return true;
!   else if (GET_CODE (pat) == PARALLEL)
      {
!       for (i = 0; i < XVECLEN (pat, 0); i++)
  	{
! 	  rtx set = XVECEXP (pat, 0, i);
! 	  if (GET_CODE (set) == SET
! 	      && store_killed_in_pat (x, set, after))
! 	    return true;
  	}
      }
    else if (find_loads (PATTERN (insn), x, after))
      return true;