Re: Fix 70278 (LRA split_regs followup patch)
On 22 March 2016 at 13:14, Bernd Schmidt wrote: > On 03/22/2016 10:24 AM, Christophe Lyon wrote: >> >> >> The ARM test isn't sufficiently protected against non-compliant >> configurations, >> and fails if GCC is configured for arm*linux-gnueabihf for instance >> (see >> http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/234342/report-build-info.html) >> >> The attached small patch fixes that by requiring arm_arch_v4t_multilib >> effective target. >> >> I used arm_arch_v4t_multilib instead of arm_arch_v4t because, as I >> reported a long time ago >> the later does not complain in some unsupported configuration because >> the sample effective >> target test does not contain actual code. In particular it's not >> sufficient to reject thumb-1 with >> hard-float. >> >> OK? > > > No objections from me, but I copied all this from the existing testcase > ftest-armv4t-thumb.c, so I'm puzzled why that one doesn't fail. > It's similar to why I tried to explain above: ftest-armv4t-thumb.c contains only preprocessor tests, no actual code. When the program contains code (or even a single global variable definition), the compiler complains that" Thumb-1 hard-float VFP ABI" is not implemented. A long time ago, I submitted a patch to add some code to the arm_arch_FUNC_ok effective target, but it was not accepted. Christophe. > > Bernd
Re: Fix 70278 (LRA split_regs followup patch)
On 03/22/2016 10:24 AM, Christophe Lyon wrote: The ARM test isn't sufficiently protected against non-compliant configurations, and fails if GCC is configured for arm*linux-gnueabihf for instance (see http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/234342/report-build-info.html) The attached small patch fixes that by requiring arm_arch_v4t_multilib effective target. I used arm_arch_v4t_multilib instead of arm_arch_v4t because, as I reported a long time ago the later does not complain in some unsupported configuration because the sample effective target test does not contain actual code. In particular it's not sufficient to reject thumb-1 with hard-float. OK? No objections from me, but I copied all this from the existing testcase ftest-armv4t-thumb.c, so I'm puzzled why that one doesn't fail. Bernd
Re: Fix 70278 (LRA split_regs followup patch)
On 18 March 2016 at 17:51, Jeff Law wrote:
> On 03/18/2016 06:25 AM, Bernd Schmidt wrote:
>>
>> This fixes an oversight in my previous patch here. I used biggest_mode
>> in the assumption that if the reg was used in the function, it would be
>> set to something other than VOIDmode, but that fails if we have a
>> multiword access - only the first hard reg gets its biggest_mode
>> assigned in that case.
>>
>> Bootstrapped and tested on x86_64-linux, ran (just) the new arm testcase
>> manually with arm-eabi. Ok?
>>
>> (The testcase seems to be from glibc. Do we keep the copyright notices
>> on the reduced form)?
>
> I don't recall specific guidance on including the copyright notice on a
> reduced/derived test.
>
> Given the actual copyright on the original code, ISTM the safest thing to do
> is keep the notice intact.
>
> A long long time ago I receive guidance from the FSF WRT what could be
> included in the testsuite -- unfortunately I didn't keep that message. I
> probably should have.
>
>>
>> Bernd
>>
>> 70278.diff
>>
>>
>> PR rtl-optimization/70278
>> * lra-constraints.c (split_reg): Handle the case where
>> biggest_mode is
>> VOIDmode.
>>
>> testsuite/
>> * gcc.dg/torture/pr70278.c: New test.
>> * gcc.target/arm/pr70278.c: New test.
>
The ARM test isn't sufficiently protected against non-compliant configurations,
and fails if GCC is configured for arm*linux-gnueabihf for instance
(see
http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/234342/report-build-info.html)
The attached small patch fixes that by requiring arm_arch_v4t_multilib
effective target.
I used arm_arch_v4t_multilib instead of arm_arch_v4t because, as I
reported a long time ago
the later does not complain in some unsupported configuration because
the sample effective
target test does not contain actual code. In particular it's not
sufficient to reject thumb-1 with
hard-float.
OK?
Thanks,
Christophe.
> OK.
> jeff
>
2016-03-22 Christophe Lyon
* gcc.target/arm/pr70278.c: Require arm_arch_v4t_multilib
effective target.
diff --git a/gcc/testsuite/gcc.target/arm/pr70278.c
b/gcc/testsuite/gcc.target/arm/pr70278.c
index c44c07b..889f626 100644
--- a/gcc/testsuite/gcc.target/arm/pr70278.c
+++ b/gcc/testsuite/gcc.target/arm/pr70278.c
@@ -2,6 +2,7 @@
/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-march=*" }
{ "-march=armv4t" } } */
/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-marm" } {
"" } } */
/* { dg-options "-mthumb" } */
+/* { dg-require-effective-target arm_arch_v4t_multilib } */
/* { dg-add-options arm_arch_v4t } */
/*
*
Fix 70278 (LRA split_regs followup patch)
This fixes an oversight in my previous patch here. I used biggest_mode
in the assumption that if the reg was used in the function, it would be
set to something other than VOIDmode, but that fails if we have a
multiword access - only the first hard reg gets its biggest_mode
assigned in that case.
Bootstrapped and tested on x86_64-linux, ran (just) the new arm testcase
manually with arm-eabi. Ok?
(The testcase seems to be from glibc. Do we keep the copyright notices
on the reduced form)?
Bernd
PR rtl-optimization/70278
* lra-constraints.c (split_reg): Handle the case where biggest_mode is
VOIDmode.
testsuite/
* gcc.dg/torture/pr70278.c: New test.
* gcc.target/arm/pr70278.c: New test.
Index: gcc/lra-constraints.c
===
--- gcc/lra-constraints.c (revision 234184)
+++ gcc/lra-constraints.c (working copy)
@@ -4982,7 +4982,12 @@ split_reg (bool before_p, int original_r
nregs = 1;
mode = lra_reg_info[hard_regno].biggest_mode;
machine_mode reg_rtx_mode = GET_MODE (regno_reg_rtx[hard_regno]);
- if (GET_MODE_SIZE (mode) > GET_MODE_SIZE (reg_rtx_mode))
+ /* A reg can have a biggest_mode of VOIDmode if it was only ever seen
+ as part of a multi-word register. In that case, or if the biggest
+ mode was larger than a register, just use the reg_rtx. Otherwise,
+ limit the size to that of the biggest access in the function. */
+ if (mode == VOIDmode
+ || GET_MODE_SIZE (mode) > GET_MODE_SIZE (reg_rtx_mode))
{
original_reg = regno_reg_rtx[hard_regno];
mode = reg_rtx_mode;
Index: gcc/testsuite/gcc.dg/torture/pr70278.c
===
--- gcc/testsuite/gcc.dg/torture/pr70278.c (revision 0)
+++ gcc/testsuite/gcc.dg/torture/pr70278.c (working copy)
@@ -0,0 +1,37 @@
+/* { dg-do compile } */
+/*
+ *
+ * Copyright (C) 1993 by Sun Microsystems, Inc. All rights reserved.
+ *
+ * Developed at SunPro, a Sun Microsystems, Inc. business.
+ * Permission to use, copy, modify, and distribute this
+ * software is freely granted, provided that this notice
+ * is preserved.
+ *
+ */
+typedef union
+{
+ double value;
+ struct
+ {
+unsigned int msw;
+ } parts;
+} ieee_double_shape_type;
+double __ieee754_hypot(double x, double y)
+{
+ double a=x,b=y,t1,t2,y1,y2,w;
+ int j,k,ha,hb;
+ do { ieee_double_shape_type gh_u; gh_u.value = (x); (ha) = gh_u.parts.msw; } while (0);;
+ if(hb > ha) {a=y;b=x;j=ha; ha=hb;hb=j;} else {a=x;b=y;}
+ if(ha > 0x5f30) {
+do { ieee_double_shape_type sh_u; sh_u.value = (a); sh_u.parts.msw = (ha); (a) = sh_u.value; } while (0);;
+ }
+ w = a-b;
+ if (w <= b)
+ {
+ t2 = a - t1;
+ w = t1*y1-(w*(-w)-(t1*y2+t2*b));
+ }
+ if(k!=0) {
+ } else return w;
+}
Index: gcc/testsuite/gcc.target/arm/pr70278.c
===
--- gcc/testsuite/gcc.target/arm/pr70278.c (revision 0)
+++ gcc/testsuite/gcc.target/arm/pr70278.c (working copy)
@@ -0,0 +1,41 @@
+/* { dg-do compile } */
+/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-march=*" } { "-march=armv4t" } } */
+/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-marm" } { "" } } */
+/* { dg-options "-mthumb" } */
+/* { dg-add-options arm_arch_v4t } */
+/*
+ *
+ * Copyright (C) 1993 by Sun Microsystems, Inc. All rights reserved.
+ *
+ * Developed at SunPro, a Sun Microsystems, Inc. business.
+ * Permission to use, copy, modify, and distribute this
+ * software is freely granted, provided that this notice
+ * is preserved.
+ *
+ */
+typedef union
+{
+ double value;
+ struct
+ {
+unsigned int msw;
+ } parts;
+} ieee_double_shape_type;
+double __ieee754_hypot(double x, double y)
+{
+ double a=x,b=y,t1,t2,y1,y2,w;
+ int j,k,ha,hb;
+ do { ieee_double_shape_type gh_u; gh_u.value = (x); (ha) = gh_u.parts.msw; } while (0);;
+ if(hb > ha) {a=y;b=x;j=ha; ha=hb;hb=j;} else {a=x;b=y;}
+ if(ha > 0x5f30) {
+do { ieee_double_shape_type sh_u; sh_u.value = (a); sh_u.parts.msw = (ha); (a) = sh_u.value; } while (0);;
+ }
+ w = a-b;
+ if (w <= b)
+ {
+ t2 = a - t1;
+ w = t1*y1-(w*(-w)-(t1*y2+t2*b));
+ }
+ if(k!=0) {
+ } else return w;
+}
Re: Fix 70278 (LRA split_regs followup patch)
On 03/18/2016 06:25 AM, Bernd Schmidt wrote: This fixes an oversight in my previous patch here. I used biggest_mode in the assumption that if the reg was used in the function, it would be set to something other than VOIDmode, but that fails if we have a multiword access - only the first hard reg gets its biggest_mode assigned in that case. Bootstrapped and tested on x86_64-linux, ran (just) the new arm testcase manually with arm-eabi. Ok? (The testcase seems to be from glibc. Do we keep the copyright notices on the reduced form)? I don't recall specific guidance on including the copyright notice on a reduced/derived test. Given the actual copyright on the original code, ISTM the safest thing to do is keep the notice intact. A long long time ago I receive guidance from the FSF WRT what could be included in the testsuite -- unfortunately I didn't keep that message. I probably should have. Bernd 70278.diff PR rtl-optimization/70278 * lra-constraints.c (split_reg): Handle the case where biggest_mode is VOIDmode. testsuite/ * gcc.dg/torture/pr70278.c: New test. * gcc.target/arm/pr70278.c: New test. OK. jeff
