[committed,testsuite] Fix fp-int-convert-timode-1.c testism.
diff mbox series

Message ID 20191121141105.GA20322@arm.com
State New
Headers show
Series
  • [committed,testsuite] Fix fp-int-convert-timode-1.c testism.
Related show

Commit Message

Tamar Christina Nov. 21, 2019, 2:11 p.m. UTC 
Hi All,

The test fp-int-convert-timode-1.c uses FE_TONEAREST without
actually checking if the target has defined it.

Like the rest of the tests I now add a check to see if the target
has actually implemented it.

This fixed Arm newlib target failures.

Regtested on aarch64-none-elf and aarch64_be-none-elf and no issues.

Committed under the GCC obvious rules.

gcc/testsuite/ChangeLog:

2019-11-21  Tamar Christina  <tamar.christina@arm.com>

	* gcc.dg/torture/fp-int-convert-timode-1.c: Add check for FE_TONEAREST.

--

Comments

Joseph Myers Nov. 21, 2019, 2:39 p.m. UTC | #1 
On Thu, 21 Nov 2019, Tamar Christina wrote:

> Hi All,
> 
> The test fp-int-convert-timode-1.c uses FE_TONEAREST without
> actually checking if the target has defined it.
> 
> Like the rest of the tests I now add a check to see if the target
> has actually implemented it.

FE_TONEAREST is the default rounding mode, so it would be better just to 
remove the fesetround call and <fenv.h> include and fenv effective-target 
and -frounding-math option and let this test run for all configurations 
with int128 support.
Tamar Christina Nov. 21, 2019, 6:29 p.m. UTC | #2 
Hi Joseph,

> FE_TONEAREST is the default rounding mode, so it would be better just to
> remove the fesetround call and <fenv.h> include and fenv effective-target
> and -frounding-math option and let this test run for all configurations with
> int128 support.
> 

Hmm right, doing makes the execution of the test fail.
I'll investigate the failures and send a patch to remove the fenv together.

Or do you want me to send them separately?

Thanks,
Tamar

> --
> Joseph S. Myers
> joseph@codesourcery.com
Joseph Myers Nov. 21, 2019, 10:13 p.m. UTC | #3 
On Thu, 21 Nov 2019, Tamar Christina wrote:

> Hi Joseph,
> 
> > FE_TONEAREST is the default rounding mode, so it would be better just to
> > remove the fesetround call and <fenv.h> include and fenv effective-target
> > and -frounding-math option and let this test run for all configurations with
> > int128 support.
> > 
> 
> Hmm right, doing makes the execution of the test fail.
> I'll investigate the failures and send a patch to remove the fenv together.
> 
> Or do you want me to send them separately?

I think it's best to fix the test now not to have the #ifdef, then if you 
have execution failures those can be addressed separately.  (If you want 
to avoid the test FAILing before then, an XFAIL with a comment referencing 
an open bug in Bugzilla would be appropriate, not #ifdef that makes the 
test spuriously PASS.)
Tamar Christina Nov. 22, 2019, 10:06 a.m. UTC | #4 
Hi Joseph,

> > Or do you want me to send them separately?
> 
> I think it's best to fix the test now not to have the #ifdef, then if you 
> have execution failures those can be addressed separately.  (If you want 
> to avoid the test FAILing before then, an XFAIL with a comment referencing 
> an open bug in Bugzilla would be appropriate, not #ifdef that makes the 
> test spuriously PASS.)
> 

Fair enough, found the issue and it wasn't with the test. I've attached the
new patch.

Ok for trunk?

Thanks,
Tamar

> -- 
> Joseph S. Myers
> joseph@codesourcery.com

--
Joseph Myers Nov. 22, 2019, 9:27 p.m. UTC | #5 
On Fri, 22 Nov 2019, Tamar Christina wrote:

> Hi Joseph,
> 
> > > Or do you want me to send them separately?
> > 
> > I think it's best to fix the test now not to have the #ifdef, then if you 
> > have execution failures those can be addressed separately.  (If you want 
> > to avoid the test FAILing before then, an XFAIL with a comment referencing 
> > an open bug in Bugzilla would be appropriate, not #ifdef that makes the 
> > test spuriously PASS.)
> > 
> 
> Fair enough, found the issue and it wasn't with the test. I've attached the
> new patch.
> 
> Ok for trunk?

OK.

Patch
diff mbox series 

diff --git a/gcc/testsuite/gcc.dg/torture/fp-int-convert-timode-1.c b/gcc/testsuite/gcc.dg/torture/fp-int-convert-timode-1.c
index 0c524a8c4782b6197bc0247a7f66340ca7d9579c..bf7f3cedb294cc834437593dae3507005f0f6b56 100644
--- a/gcc/testsuite/gcc.dg/torture/fp-int-convert-timode-1.c
+++ b/gcc/testsuite/gcc.dg/torture/fp-int-convert-timode-1.c
@@ -11,6 +11,7 @@ 
 int
 main (void)
 {
+#ifdef FE_TONEAREST
   volatile unsigned long long h = 0x8000000000000000LL;
   volatile unsigned long long l = 0xdLL;
   volatile unsigned __int128 u128 = (((unsigned __int128) h) << 64) | l;
@@ -22,5 +23,6 @@  main (void)
   double ds = s128;
   if (ds != -0x1p+127)
     abort ();
+#endif
   exit (0);
 }