[SH][committed] PR 53988 - Fix wrong code

[SH][committed] PR 53988 - Fix wrong code

[Oleg Endo]

[-- Attachment #1: Type: text/plain, Size: 989 bytes --]

Hi,

The attached patch fixes a wrong-code issue which was the result of the
initial fix for PR 53988.
Tested with
make -k check RUNTESTFLAGS="--target_board=sh-sim
\{-m2/-ml,-m2/-mb,-m2a/-mb,-m4/-ml,-m4/-mb,-m4a/-ml,-m4a/-mb}"
and no new failures.  Committed as 219623.  Backports to 4.8 and 4.9
will follow later.

Cheers,
Oleg

gcc/ChangeLog:
	PR target/53988
	* config/sh/sh-protos.h (sh_find_set_of_reg): Add option to ignore
	reg-reg copies.
	(sh_extending_set_of_reg): New struct.
	(sh_find_extending_set_of_reg, sh_split_tst_subregs,
	sh_remove_reg_dead_or_unused_notes): New Declarations.
	* config/sh/sh.c (sh_remove_reg_dead_or_unused_notes,
	sh_find_extending_set_of_reg, sh_split_tst_subregs,
	sh_extending_set_of_reg::use_as_extended_reg): New functions.
	* config/sh/sh.md (*tst<mode>_t_zero): Rename to *tst<mode>_t_subregs,
	convert to insn_and_split and use new function sh_split_tst_subregs.

gcc/testsuite/ChangeLog:
	PR target/53988
	* gcc.target/sh/pr53988-1.c: New.

[-- Attachment #2: sh_pr53988_2.patch --]
[-- Type: text/x-patch, Size: 13717 bytes --]

Index: gcc/testsuite/gcc.target/sh/pr53988-1.c
===================================================================
--- gcc/testsuite/gcc.target/sh/pr53988-1.c	(revision 0)
+++ gcc/testsuite/gcc.target/sh/pr53988-1.c	(revision 0)
@@ -0,0 +1,66 @@
+/* Check that sign/zero extensions are emitted where needed when the
+   tst Rm,Rn instruction is used.  */
+/* { dg-do compile }  */
+/* { dg-options "-O1" }  */
+/* { dg-skip-if "" { "sh*-*-*" } { "-m5*"} { "" } }  */
+/* { dg-final { scan-assembler-times "tst\tr" 8 } }  */
+/* { dg-final { scan-assembler-times "mov.b" 4 } }  */
+/* { dg-final { scan-assembler-times "mov.w" 4 } }  */
+/* { dg-final { scan-assembler-times "extu.b" 4 } }  */
+/* { dg-final { scan-assembler-times "extu.w" 2 } }  */
+
+int
+test_00 (char* x, char* y)
+{
+  /* 2x mov.b (sign extending)  */
+  return *x & *y ? -40 : 60;
+}
+
+int
+test_01 (short* x, short* y)
+{
+  /* 2x mov.w (sign extending)  */
+  return *x & *y ? -40 : 60;
+}
+
+int
+test_02 (char x, char y)
+{
+  /* 1x extu.b  */
+  return x & y ? -40 : 60;
+}
+
+int
+test_03 (short x, short y)
+{
+  /* 1x extu.w  */
+  return x & y ? -40 : 60;
+}
+
+int
+test_04 (char* x, unsigned char y)
+{
+  /* 1x mov.b, 1x extu.b  */
+  return *x & y ? -40 : 60;
+}
+
+int
+test_05 (short* x, unsigned char y)
+{
+  /* 1x mov.w, 1x extu.b  */
+  return *x & y ? -40 : 60;
+}
+
+int
+test_06 (short x, short* y, int z, int w)
+{
+  /* 1x mov.w, 1x extu.w  */
+  return x & y[0] ? z : w;
+}
+
+int
+test_07 (char x, char* y, int z, int w)
+{
+  /* 1x mov.b, 1x extu.b  */
+  return x & y[0] ? z : w;
+}
Index: gcc/config/sh/sh.md
===================================================================
--- gcc/config/sh/sh.md	(revision 219622)
+++ gcc/config/sh/sh.md	(working copy)
@@ -666,30 +666,40 @@
   [(set_attr "type" "mt_group")])
 
 ;; This pattern might be risky because it also tests the upper bits and not
-;; only the subreg.  However, it seems that combine will get to this only
-;; when testing sign/zero extended values.  In this case the extended upper
-;; bits do not matter.
-(define_insn "*tst<mode>_t_zero"
+;; only the subreg.  We have to check whether the operands have been sign
+;; or zero extended.  In the worst case, a zero extension has to be inserted
+;; to mask out the unwanted bits.
+(define_insn_and_split "*tst<mode>_t_subregs"
   [(set (reg:SI T_REG)
 	(eq:SI
 	  (subreg:QIHI
-	    (and:SI (match_operand:SI 0 "arith_reg_operand" "%r")
-		    (match_operand:SI 1 "arith_reg_operand" "r")) <lowpart_le>)
+	    (and:SI (match_operand:SI 0 "arith_reg_operand")
+		    (match_operand:SI 1 "arith_reg_operand")) <lowpart_le>)
 	  (const_int 0)))]
-  "TARGET_SH1 && TARGET_LITTLE_ENDIAN"
-  "tst	%0,%1"
-  [(set_attr "type" "mt_group")])
+  "TARGET_SH1 && TARGET_LITTLE_ENDIAN && can_create_pseudo_p ()"
+  "#"
+  "&& 1"
+  [(const_int 0)]
+{
+  sh_split_tst_subregs (curr_insn, <MODE>mode, <lowpart_le>, operands);
+  DONE;
+})
 
-(define_insn "*tst<mode>_t_zero"
+(define_insn_and_split "*tst<mode>_t_subregs"
   [(set (reg:SI T_REG)
 	(eq:SI
 	  (subreg:QIHI
-	    (and:SI (match_operand:SI 0 "arith_reg_operand" "%r")
-		    (match_operand:SI 1 "arith_reg_operand" "r")) <lowpart_be>)
+	    (and:SI (match_operand:SI 0 "arith_reg_operand")
+		    (match_operand:SI 1 "arith_reg_operand")) <lowpart_be>)
 	  (const_int 0)))]
-  "TARGET_SH1 && TARGET_BIG_ENDIAN"
-  "tst	%0,%1"
-  [(set_attr "type" "mt_group")])
+  "TARGET_SH1 && TARGET_BIG_ENDIAN && can_create_pseudo_p ()"
+  "#"
+  "&& 1"
+  [(const_int 0)]
+{
+  sh_split_tst_subregs (curr_insn, <MODE>mode, <lowpart_be>, operands);
+  DONE;
+})
 
 ;; Extract LSB, negate and store in T bit.
 (define_insn "tstsi_t_and_not"
Index: gcc/config/sh/sh-protos.h
===================================================================
--- gcc/config/sh/sh-protos.h	(revision 219622)
+++ gcc/config/sh/sh-protos.h	(working copy)
@@ -181,7 +181,8 @@
    'prev_nonnote_insn_bb'.  When the insn is found, try to extract the rtx
    of the reg set.  */
 template <typename F> inline set_of_reg
-sh_find_set_of_reg (rtx reg, rtx_insn* insn, F stepfunc)
+sh_find_set_of_reg (rtx reg, rtx_insn* insn, F stepfunc,
+		    bool ignore_reg_reg_copies = false)
 {
   set_of_reg result;
   result.insn = insn;
@@ -206,6 +207,19 @@
 	    return result;
 
 	  result.set_src = XEXP (result.set_rtx, 1);
+
+	  if (ignore_reg_reg_copies && REG_P (result.set_src))
+	    {
+	      reg = result.set_src;
+	      continue;
+	    }
+	  if (ignore_reg_reg_copies && SUBREG_P (result.set_src)
+	      && REG_P (SUBREG_REG (result.set_src)))
+	    {
+	      reg = SUBREG_REG (result.set_src);
+	      continue;
+	    }
+
 	  return result;
 	}
     }
@@ -213,10 +227,50 @@
   return result;
 }
 
+/* Result value of sh_find_extending_set_of_reg.  */
+struct sh_extending_set_of_reg : public set_of_reg
+{
+  /* The mode the set is extending from (QImode or HImode), or VOIDmode if
+     this is not a zero/sign extending set.  */
+  machine_mode from_mode;
+
+  /* ZERO_EXTEND, SIGN_EXTEND or UNKNOWN.  */
+  rtx_code ext_code;
+
+  sh_extending_set_of_reg (rtx_insn* i)
+  {
+    insn = i;
+    set_rtx = NULL;
+    set_src = NULL;
+    from_mode = VOIDmode;
+    ext_code = UNKNOWN;
+  }
+
+  sh_extending_set_of_reg (const set_of_reg& rhs)
+  {
+    *((set_of_reg*)this) = rhs;
+    from_mode = VOIDmode;
+    ext_code = UNKNOWN;
+  }
+
+  /* Returns the reg rtx of the sign or zero extending result, that can be
+     safely used at the specified insn in SImode.  If the set source is an
+     implicitly sign extending mem load, the mem load is converted into an
+     explicitly sign extending mem load.  */
+  rtx use_as_extended_reg (rtx_insn* use_at_insn) const;
+};
+
+extern sh_extending_set_of_reg sh_find_extending_set_of_reg (rtx reg,
+							     rtx_insn* insn);
+
 extern bool sh_is_logical_t_store_expr (rtx op, rtx_insn* insn);
 extern rtx sh_try_omit_signzero_extend (rtx extended_op, rtx_insn* insn);
 extern bool sh_split_movrt_negc_to_movt_xor (rtx_insn* curr_insn,
 					     rtx operands[]);
+extern void sh_split_tst_subregs (rtx_insn* curr_insn,
+				  machine_mode subreg_mode, int subreg_offset,
+				  rtx operands[]);
+extern void sh_remove_reg_dead_or_unused_notes (rtx_insn* i, int regno);
 #endif /* RTX_CODE */
 
 extern void sh_cpu_cpp_builtins (cpp_reader* pfile);
Index: gcc/config/sh/sh.c
===================================================================
--- gcc/config/sh/sh.c	(revision 219622)
+++ gcc/config/sh/sh.c	(working copy)
@@ -13769,6 +13769,17 @@
   return false;
 }
 
+/* Given an insn and a reg number, remove reg dead or reg unused notes to
+   mark it as being used after the insn.  */
+void
+sh_remove_reg_dead_or_unused_notes (rtx_insn* i, int regno)
+{
+  if (rtx n = find_regno_note (i, REG_DEAD, regno))
+    remove_note (i, n);
+  if (rtx n = find_regno_note (i, REG_UNUSED, regno))
+    remove_note (i, n);
+}
+
 /* Given an op rtx and an insn, try to find out whether the result of the
    specified op consists only of logical operations on T bit stores.  */
 bool
@@ -13881,6 +13892,175 @@
     return false;
 }
 
+/* Given a reg and the current insn, see if the value of the reg originated
+   from a sign or zero extension and return the discovered information.  */
+sh_extending_set_of_reg
+sh_find_extending_set_of_reg (rtx reg, rtx_insn* curr_insn)
+{
+  if (reg == NULL)
+    return sh_extending_set_of_reg (curr_insn);
+
+  if (SUBREG_P (reg))
+    reg = SUBREG_REG (reg);
+
+  if (!REG_P (reg))
+    return sh_extending_set_of_reg (curr_insn);
+
+  /* FIXME: Also search the predecessor basic blocks.  It seems that checking
+     only the adjacent predecessor blocks would cover most of the cases.
+     Also try to look through the first extension that we hit.  There are some
+     cases, where a zero_extend is followed an (implicit) sign_extend, and it
+     fails to see the sign_extend.  */
+  sh_extending_set_of_reg result =
+	sh_find_set_of_reg (reg, curr_insn, prev_nonnote_insn_bb, true);
+
+  if (result.set_src != NULL)
+    {
+      if (GET_CODE (result.set_src) == SIGN_EXTEND
+	  || GET_CODE (result.set_src) == ZERO_EXTEND)
+	{
+	  if (dump_file)
+	    fprintf (dump_file, "sh_find_szexnteded_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));
+	  result.ext_code = GET_CODE (result.set_src);
+	}
+      else if (MEM_P (result.set_src)
+	       && (GET_MODE (result.set_src) == QImode
+		   || GET_MODE (result.set_src) == HImode))
+	{
+	  /* On SH QIHImode memory loads always sign extend.  However, in
+	     some cases where it seems that the higher bits are not
+	     interesting, the loads will not be expanded as sign extending
+	     insns, but as QIHImode loads into QIHImode regs.  We report that
+	     the reg has been sign extended by the mem load.  When it is used
+	     as such, we must convert the mem load into a sign extending insn,
+	     see also sh_extending_set_of_reg::use_as_extended_reg.  */
+	  if (dump_file)
+	    fprintf (dump_file, "sh_find_extending_set_of_reg: reg %d is "
+				"implicitly sign extended in insn %d\n",
+				REGNO (reg), INSN_UID (result.insn));
+	  result.from_mode = GET_MODE (result.set_src);
+	  result.ext_code = SIGN_EXTEND;
+	}
+    }
+
+  return result;
+}
+
+/* Given a reg that is known to be sign or zero extended at some insn,
+   take the appropriate measures so that the extended value can be used as
+   a reg at the specified insn and return the resulting reg rtx.  */
+rtx
+sh_extending_set_of_reg::use_as_extended_reg (rtx_insn* use_at_insn) const
+{
+  gcc_assert (insn != NULL && set_src != NULL && set_rtx != NULL);
+  gcc_assert (ext_code == SIGN_EXTEND || ext_code == ZERO_EXTEND);
+  gcc_assert (from_mode == QImode || from_mode == HImode);
+
+  if (MEM_P (set_src) && ext_code == SIGN_EXTEND)
+    {
+      if (dump_file)
+	fprintf (dump_file,
+		 "use_as_extended_reg: converting non-extending mem load in "
+		 "insn %d into sign-extending load\n", INSN_UID (insn));
+
+	rtx r = gen_reg_rtx (SImode);
+	rtx_insn* i0;
+	if (from_mode == QImode)
+	  i0 = emit_insn_after (gen_extendqisi2 (r, set_src), insn);
+	else if (from_mode == HImode)
+	  i0 = emit_insn_after (gen_extendhisi2 (r, set_src), insn);
+	else
+	  gcc_unreachable ();
+
+	emit_insn_after (
+		gen_move_insn (XEXP (set_rtx, 0),
+			       gen_lowpart (GET_MODE (set_src), r)), i0);
+	set_insn_deleted (insn);
+	return r;
+    }
+  else
+    {
+      rtx extension_dst = XEXP (set_rtx, 0);
+      if (modified_between_p (extension_dst, insn, use_at_insn))
+	{
+	  if (dump_file)
+	    fprintf (dump_file,
+		     "use_as_extended_reg: dest reg %d of extending insn %d is "
+		     "modified, inserting a reg-reg copy\n",
+		     REGNO (extension_dst), INSN_UID (insn));
+
+	  rtx r = gen_reg_rtx (SImode);
+	  emit_insn_after (gen_move_insn (r, extension_dst), insn);
+	  return r;
+	}
+      else
+	{
+	  sh_remove_reg_dead_or_unused_notes (insn, REGNO (extension_dst));
+	  return extension_dst;
+	}
+    }
+}
+
+/* Given the current insn, which is assumed to be the *tst<mode>_t_subregs insn,
+   perform the necessary checks on the operands and split it accordingly.  */
+void
+sh_split_tst_subregs (rtx_insn* curr_insn, machine_mode subreg_mode,
+		      int subreg_offset, rtx operands[])
+{
+  gcc_assert (subreg_mode == QImode || subreg_mode == HImode);
+
+  sh_extending_set_of_reg eop0 = sh_find_extending_set_of_reg (operands[0],
+							       curr_insn);
+  sh_extending_set_of_reg eop1 = sh_find_extending_set_of_reg (operands[1],
+							       curr_insn);
+
+  /* If one of the operands is known to be zero extended, that's already
+     sufficient to mask out the unwanted high bits.  */
+  if (eop0.ext_code == ZERO_EXTEND && eop0.from_mode == subreg_mode)
+    {
+      emit_insn (gen_tstsi_t (eop0.use_as_extended_reg (curr_insn),
+			      operands[1]));
+      return;
+    }
+  if (eop1.ext_code == ZERO_EXTEND && eop1.from_mode == subreg_mode)
+    {
+      emit_insn (gen_tstsi_t (operands[0],
+			      eop1.use_as_extended_reg (curr_insn)));
+      return;
+    }
+
+  /* None of the operands seem to be zero extended.
+     If both are sign extended it's OK, too.  */
+  if (eop0.ext_code == SIGN_EXTEND && eop1.ext_code == SIGN_EXTEND
+      && eop0.from_mode == subreg_mode && eop1.from_mode == subreg_mode)
+    {
+      emit_insn (gen_tstsi_t (eop0.use_as_extended_reg (curr_insn),
+			      eop1.use_as_extended_reg (curr_insn)));
+      return;
+    }
+
+  /* Otherwise we have to insert a zero extension on one of the operands to
+     mask out the unwanted high bits.
+     Prefer the operand that has no known extension.  */
+  if (eop0.ext_code != UNKNOWN && eop1.ext_code == UNKNOWN)
+    std::swap (operands[0], operands[1]);
+
+  rtx tmp0 = gen_reg_rtx (SImode);
+  rtx tmp1 = simplify_gen_subreg (subreg_mode, operands[0],
+				  GET_MODE (operands[0]), subreg_offset);
+  emit_insn (subreg_mode == QImode
+	     ? gen_zero_extendqisi2 (tmp0, tmp1)
+	     : gen_zero_extendhisi2 (tmp0, tmp1));
+  emit_insn (gen_tstsi_t (tmp0, operands[1]));
+}
+
+/*------------------------------------------------------------------------------
+  Mode switching support code.
+*/
+
 static void
 sh_emit_mode_set (int entity ATTRIBUTE_UNUSED, int mode,
 		  int prev_mode, HARD_REG_SET regs_live ATTRIBUTE_UNUSED)
@@ -13949,6 +14129,10 @@
   return ((TARGET_FPU_SINGLE != 0) ^ (n) ? FP_MODE_SINGLE : FP_MODE_DOUBLE);
 }
 
+/*------------------------------------------------------------------------------
+  Misc
+*/
+
 /* Return true if we use LRA instead of reload pass.  */
 static bool
 sh_lra_p (void)