Re: [PATCH] [SH] Fix target/48596
Oleg Endo wrote:
> The attached patch moves it as suggested to gcc.c-torture/compile.
> Briefly tested by running the gcc.c-torture/compile set on sh-him
> -m4a-single -ml.
You forgot to remove two dg-* lines:
> +/* { dg-do compile } */
> +/* { dg-options "-O1" } */
unneeded for this gcc.c-torture/compile test. Looks OK
with that change. FYI, I've tested it on i686-pc-linux-gnu
with no problem.
Regards,
kaz
Re: [PATCH] [SH] Fix target/48596
On Tue, 2012-03-06 at 08:24 +0900, Kaz Kojima wrote:
> Oleg Endo wrote:
> > I'd like to add the test case from the PR to the testsuite.
> >
> > Tested with
> > make check-gcc RUNTESTFLAGS="sh.exp=pr48596.c --target_board=sh-sim
> > \{-m2/-ml,-m2/-mb,-m2a-single/-mb,
> > -m4-single/-ml,-m4-single/-mb,-m4a-single/-ml,-m4a-single/-mb}"
> >
> > OK?
>
> A gcc.c-torture/compile test is better, isn't it?
>
I just noticed that I've accidentally added the pr48596.c to
gcc.target/sh in another commit. I'm sorry about that.
The attached patch moves it as suggested to gcc.c-torture/compile.
Briefly tested by running the gcc.c-torture/compile set on sh-him
-m4a-single -ml.
testsuite/ChangeLog:
PR target/48596
* gcc.target/sh/pr48596.c: Move accidentally added new test
case to ...
* gcc.c-torture/compile/pr48596.c: ... here.
Index: gcc/testsuite/gcc.target/sh/pr48596.c
===
--- gcc/testsuite/gcc.target/sh/pr48596.c (revision 185191)
+++ gcc/testsuite/gcc.target/sh/pr48596.c (working copy)
@@ -1,31 +0,0 @@
-/* Check that the following code compiles without errors. */
-/* { dg-do compile { target "sh*-*-*" } } */
-/* { dg-options "-O1" } */
-
-enum { nrrdCenterUnknown, nrrdCenterNode, nrrdCenterCell, nrrdCenterLast };
-typedef struct { int size; int center; } NrrdAxis;
-typedef struct { int dim; NrrdAxis axis[10]; } Nrrd;
-typedef struct { } NrrdKernel;
-typedef struct { const NrrdKernel *kernel[10]; int samples[10]; } Info;
-
-void
-foo (Nrrd *nout, Nrrd *nin, const NrrdKernel *kernel, const double *parm,
- const int *samples, const double *scalings)
-{
- Info *info;
- int d, p, np, center;
- for (d=0; ddim; d++)
-{
- info->kernel[d] = kernel;
- if (samples)
- info->samples[d] = samples[d];
- else
- {
- center = _nrrdCenter(nin->axis[d].center);
- if (nrrdCenterCell == center)
- info->samples[d] = nin->axis[d].size*scalings[d];
- else
- info->samples[d] = (nin->axis[d].size - 1)*scalings[d] + 1;
- }
-}
-}
Index: gcc/testsuite/gcc.c-torture/compile/pr48596.c
===
--- gcc/testsuite/gcc.c-torture/compile/pr48596.c (revision 0)
+++ gcc/testsuite/gcc.c-torture/compile/pr48596.c (revision 0)
@@ -0,0 +1,31 @@
+/* PR target/48596 */
+/* { dg-do compile } */
+/* { dg-options "-O1" } */
+
+enum { nrrdCenterUnknown, nrrdCenterNode, nrrdCenterCell, nrrdCenterLast };
+typedef struct { int size; int center; } NrrdAxis;
+typedef struct { int dim; NrrdAxis axis[10]; } Nrrd;
+typedef struct { } NrrdKernel;
+typedef struct { const NrrdKernel *kernel[10]; int samples[10]; } Info;
+
+void
+foo (Nrrd *nout, Nrrd *nin, const NrrdKernel *kernel, const double *parm,
+ const int *samples, const double *scalings)
+{
+ Info *info;
+ int d, p, np, center;
+ for (d=0; ddim; d++)
+{
+ info->kernel[d] = kernel;
+ if (samples)
+ info->samples[d] = samples[d];
+ else
+ {
+ center = _nrrdCenter(nin->axis[d].center);
+ if (nrrdCenterCell == center)
+ info->samples[d] = nin->axis[d].size*scalings[d];
+ else
+ info->samples[d] = (nin->axis[d].size - 1)*scalings[d] + 1;
+ }
+}
+}
Re: [PATCH] [SH] Fix target/48596
Oleg Endo wrote:
> I'd like to add the test case from the PR to the testsuite.
>
> Tested with
> make check-gcc RUNTESTFLAGS="sh.exp=pr48596.c --target_board=sh-sim
> \{-m2/-ml,-m2/-mb,-m2a-single/-mb,
> -m4-single/-ml,-m4-single/-mb,-m4a-single/-ml,-m4a-single/-mb}"
>
> OK?
A gcc.c-torture/compile test is better, isn't it?
Regards,
kaz
Re: [PATCH] [SH] Fix target/48596
On Thu, 2012-03-01 at 13:11 +0900, Kaz Kojima wrote:
> Hi,
>
> The attached patch is to avoid PR target/48596 which is a 4.7
> regression on SH.
> [...]
I'd like to add the test case from the PR to the testsuite.
Tested with
make check-gcc RUNTESTFLAGS="sh.exp=pr48596.c --target_board=sh-sim
\{-m2/-ml,-m2/-mb,-m2a-single/-mb,
-m4-single/-ml,-m4-single/-mb,-m4a-single/-ml,-m4a-single/-mb}"
OK?
Cheers,
Oleg
testsuite/ChangeLog:
PR target/48596
* gcc.target/sh/pr48596.c: New.
Index: gcc/testsuite/gcc.target/sh/pr48596.c
===
--- gcc/testsuite/gcc.target/sh/pr48596.c (revision 0)
+++ gcc/testsuite/gcc.target/sh/pr48596.c (revision 0)
@@ -0,0 +1,31 @@
+/* Check that the following code compiles without errors. */
+/* { dg-do compile { target "sh*-*-*" } } */
+/* { dg-options "-O1" } */
+
+enum { nrrdCenterUnknown, nrrdCenterNode, nrrdCenterCell, nrrdCenterLast };
+typedef struct { int size; int center; } NrrdAxis;
+typedef struct { int dim; NrrdAxis axis[10]; } Nrrd;
+typedef struct { } NrrdKernel;
+typedef struct { const NrrdKernel *kernel[10]; int samples[10]; } Info;
+
+void
+foo (Nrrd *nout, Nrrd *nin, const NrrdKernel *kernel, const double *parm,
+ const int *samples, const double *scalings)
+{
+ Info *info;
+ int d, p, np, center;
+ for (d=0; ddim; d++)
+{
+ info->kernel[d] = kernel;
+ if (samples)
+ info->samples[d] = samples[d];
+ else
+ {
+ center = _nrrdCenter(nin->axis[d].center);
+ if (nrrdCenterCell == center)
+ info->samples[d] = nin->axis[d].size*scalings[d];
+ else
+ info->samples[d] = (nin->axis[d].size - 1)*scalings[d] + 1;
+ }
+}
+}
Re: [PATCH] [SH] Fix target/48596
Oleg Endo wrote: > BTW, the patch also seems to fix PR 48806. Ah, I've confirmed it. Thanks for pointing it out. Regards, kaz
Re: [PATCH] [SH] Fix target/48596
On Thu, 2012-03-01 at 13:11 +0900, Kaz Kojima wrote:
> Hi,
>
> The attached patch is to avoid PR target/48596 which is a 4.7
> regression on SH. It seems that now IRA aggressively tried to
> use fp regs as the holder for memory addresses on this port.
> SH requires a special fpul register to move from the fp regs to
> the general regs which are legitimate for pointers and, in
> the problematic situation, fpul is already reserved for reload
> and the spill failure resulted for the reg equiv processing in RA.
> I guess that no other targets have such restrictions. Perhaps
> if there eas a direct way to notify IRA that some register classes
> will be too costly for the addresses, SH port will utilize it,
> though it looks to be invasive.
>
> The patch tries to work around the problem with increasing the move
> cost between fp and general registers for SImode. The usual tests
> are done successfully on sh4-unknown-linux-gnu with no new failures.
> A bit surprisingly, there are no size/performance regressions and
> a few code size improvements with CSiBE tests I've done.
> I'd like to hear the suggestions from the experts before applying
> this work around.
>
I was afraid that with this patch, in the following use case ...
int32_t float_as_int (float val)
{
union { float f; int32_t i; } u;
u.f = val;
return u.i;
}
... the value would somehow get pushed/popped through the stack.
Luckily this is not the case, it still goes through the fpul register :)
__Z12float_as_intf:
fldsfr5,fpul
rts
sts fpul,r0
(I've got a similar use case where it does go through the stack, but it
also happens without the patch...)
BTW, the patch also seems to fix PR 48806.
Cheers,
Oleg
Re: [PATCH] [SH] Fix target/48596
On Feb 29, 2012, at 8:11 PM, Kaz Kojima wrote: > The attached patch is to avoid PR target/48596 which is a 4.7 > regression on SH. It seems that now IRA aggressively tried to > use fp regs as the holder for memory addresses on this port. Gosh, I hope IRA is cleaned up, as other ports do have weird register restrictions... that don't want to get hit by the new IRA code. [ crosses fingers that when I update to 4.7, I won't hit this ]
