From: Oleg Endo <oleg.endo@t-online.de>
To: gcc-patches <gcc-patches@gcc.gnu.org>
Subject: [SH][committed] PR 53988 - Fix wrong code
Date: Thu, 15 Jan 2015 00:38:00 -0000 [thread overview]
Message-ID: <1421279760.2473.272.camel@yam-132-YW-E178-FTW> (raw)
[-- 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)
reply other threads:[~2015-01-14 23:56 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1421279760.2473.272.camel@yam-132-YW-E178-FTW \
--to=oleg.endo@t-online.de \
--cc=gcc-patches@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).