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.e...@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;
}
