Re: [SH] Introduce treg_set_expr

[Oleg Endo <oleg dot endo at t-online dot de>]

On Sun, 2015-01-18 at 13:25 +0100, Oleg Endo wrote:
> On Sat, 2015-01-17 at 22:40 +0900, Kaz Kojima wrote:
> > Oleg Endo <oleg.endo@t-online.de> wrote:
> > > Kaz, could you please test the patch on your sh4-linux setup and report
> > > your findings?  Even though it's a bit late, I'd like to get this in for
> > > GCC 5, if it doesn't break too many things.
> > 
> > Looks wrong code problem for tls and atomic constructs.
> > Here is the result of compare_tests for unpatched/patched
> > sh4-unknown-linux-gnu compilers:
> > 
> > New tests that FAIL:
> > 
> > ...
> 
> Thanks.  Doesn't look so bad actually.  I've expected worse.  These are
> the two problems:
> 
> 1) sh_find_extending_set_of_reg (introduced earlier as part of PR 53988)
> hits atomic insns, which in fact do a indirect sign extending mem load.
> The cmpeqsi_t splitter for const_int 0 then tries to use the value as
> sign extended, which wrongly converts an atomic into a simple mem load.
> 
> The easy solution is not to report 'sign extended' in
> sh_find_extending_set_of_reg for mems that are buried in UNSPEC/UNSPECV
> insns.  This also revealed a problem of inconsistent return values of
> sh_find_set_of_reg.  This should be fixed regardless of the
> treg_set_expr stuff first.  I'll create separate patch for that.

The attached patch fixes this issue.
Tested with 
make -k check RUNTESTFLAGS="--target_board=sh-sim
\{-m2/-ml,-m2/-mb,-m2a/-mb,-m4/-ml,-m4/-mb,-m4a/-ml,-m4a/-mb}"
Committed as r219864.

Cheers,
Oleg

Index: gcc/config/sh/sh.c
===================================================================
--- gcc/config/sh/sh.c	(revision 219863)
+++ gcc/config/sh/sh.c	(working copy)
@@ -13738,22 +13738,15 @@
 /* Return true if the specified insn contains any UNSPECs or
    UNSPEC_VOLATILEs.  */
 static bool
-sh_unspec_insn_p (rtx_insn* insn)
+sh_unspec_insn_p (rtx x)
 {
-  bool result = false;
+  subrtx_iterator::array_type array;
+  FOR_EACH_SUBRTX (i, array, x, ALL)
+    if (*i != NULL
+	&& (GET_CODE (*i) == UNSPEC || GET_CODE (*i) == UNSPEC_VOLATILE))
+      return true;
 
-  struct note_uses_func
-  {
-    static void
-    func (rtx* x, void* data)
-    {
-      if (GET_CODE (*x) == UNSPEC || GET_CODE (*x) == UNSPEC_VOLATILE)
-	*(static_cast<bool*> (data)) = true;
-    }
-  };
-
-  note_uses (&PATTERN (insn), note_uses_func::func, &result);
-  return result;
+  return false;
 }
 
 /* Return true if the register operands of the specified insn are modified
@@ -13770,7 +13763,8 @@
 
   subrtx_iterator::array_type array;
   FOR_EACH_SUBRTX (i, array, SET_SRC (s), ALL)
-    if ((REG_P (*i) || SUBREG_P (*i)) && reg_set_between_p (*i, from, to))
+    if (*i != NULL &&
+	((REG_P (*i) || SUBREG_P (*i)) && reg_set_between_p (*i, from, to)))
       return true;
 
   return false;
@@ -13927,7 +13921,7 @@
 	  || GET_CODE (result.set_src) == ZERO_EXTEND)
 	{
 	  if (dump_file)
-	    fprintf (dump_file, "sh_find_szexnteded_reg: reg %d is "
+	    fprintf (dump_file, "sh_find_extending_set_of_reg: reg %d is "
 				"explicitly sign/zero extended in insn %d\n",
 				REGNO (reg), INSN_UID (result.insn));
 	  result.from_mode = GET_MODE (XEXP (result.set_src, 0));
@@ -13935,7 +13929,8 @@
 	}
       else if (MEM_P (result.set_src)
 	       && (GET_MODE (result.set_src) == QImode
-		   || GET_MODE (result.set_src) == HImode))
+		   || GET_MODE (result.set_src) == HImode)
+	       && !sh_unspec_insn_p (result.insn))
 	{
 	  /* On SH QIHImode memory loads always sign extend.  However, in
 	     some cases where it seems that the higher bits are not
Index: gcc/config/sh/sh-protos.h
===================================================================
--- gcc/config/sh/sh-protos.h	(revision 219863)
+++ gcc/config/sh/sh-protos.h	(working copy)
@@ -192,11 +192,13 @@
   if (!REG_P (reg) || insn == NULL_RTX)
     return result;
 
+  rtx_insn* previnsn = insn;
+
   for (result.insn = stepfunc (insn); result.insn != NULL_RTX;
-       result.insn = stepfunc (result.insn))
+       previnsn = result.insn, result.insn = stepfunc (result.insn))
     {
       if (BARRIER_P (result.insn))
-	return result;
+	break;
       if (!NONJUMP_INSN_P (result.insn))
 	continue;
       if (reg_set_p (reg, result.insn))
@@ -204,7 +206,7 @@
 	  result.set_rtx = set_of (reg, result.insn);
 
 	  if (result.set_rtx == NULL_RTX || GET_CODE (result.set_rtx) != SET)
-	    return result;
+	    break;
 
 	  result.set_src = XEXP (result.set_rtx, 1);
 
@@ -220,10 +222,19 @@
 	      continue;
 	    }
 
-	  return result;
+	  break;
 	}
     }
 
+  /* If the loop above stopped at the first insn in the list,
+     result.insn will be null.  Use the insn from the previous iteration
+     in this case.  */
+  if (result.insn == NULL)
+    result.insn = previnsn;
+
+  if (result.set_src != NULL)
+    gcc_assert (result.insn != NULL && result.set_rtx != NULL);
+
   return result;
 }