Patchwork Fix VIS3 assembler check and conditionalize testsuite on VIS3 support.

login
register
mail settings
Submitter David Miller
Date Oct. 11, 2011, 8:12 p.m.
Message ID <20111011.161235.1865115470975891476.davem@davemloft.net>
Download mbox | patch
Permalink /patch/119068/
State New
Headers show

Comments

David Miller - Oct. 11, 2011, 8:12 p.m.
From: Eric Botcazou <ebotcazou@adacore.com>
Date: Tue, 11 Oct 2011 21:57:18 +0200

>> I would expect that to spit out a warning.  Do I need to explicitly
>> add "-Wall", "-Wno-implicit" or similar?  Similar tests in i386.exp don't
>> seem to need this and that was what I used as my template.
> 
> -Wall does yield a warning:
> 
> vis.c: In function '_vis3_fpadd64':
> vis.c:4:3: warning: implicit declaration of function '__builtin_vis_fpadd64' 
> [-Wimplicit-function-declaration]

Cool, Eric could you quickly test the following?  This still leaves
the i386.exp case issue open, it stands to reason that something like
-Wall is needed for those tests too.
Eric Botcazou - Oct. 11, 2011, 9:08 p.m.
> Cool, Eric could you quickly test the following?  This still leaves
> the i386.exp case issue open, it stands to reason that something like
> -Wall is needed for those tests too.

I think that we should go the i386 way.  This works on i386 because the 
builtins are always available (when you pass the right options) and the 
assembler rejects the unknown instructions.  So in config/sparc/sparc.h:

#ifndef HAVE_AS_FMAF_HPC_VIS3
#define AS_NIAGARA3_FLAG "b"
#undef TARGET_FMAF
#define TARGET_FMAF 0
#undef TARGET_VIS3
#define TARGET_VIS3 0
#else
#define AS_NIAGARA3_FLAG "d"
#endif

we shouldn't force TARGET_FMAF and TARGET_VIS3 to 0.  The configure test would 
only be used to compute default options.
David Miller - Oct. 11, 2011, 10:01 p.m.
From: Eric Botcazou <ebotcazou@adacore.com>
Date: Tue, 11 Oct 2011 23:08:50 +0200

>> Cool, Eric could you quickly test the following?  This still leaves
>> the i386.exp case issue open, it stands to reason that something like
>> -Wall is needed for those tests too.
> 
> I think that we should go the i386 way.  This works on i386 because the 
> builtins are always available (when you pass the right options) and the 
> assembler rejects the unknown instructions.  So in config/sparc/sparc.h:
 ...
> we shouldn't force TARGET_FMAF and TARGET_VIS3 to 0.  The configure test would 
> only be used to compute default options.

I see, so we can test the code generation in the testsuite even if the
compiler was built against an assembler without support for the
instructions.

But in such a case, I'm unsure if I understand why i386.exp needs
these tests at all.  The presence of support for a particular i386
intrinsic is an implicit property of the gcc sources that these test
cases are a part of.

If the tests are properly added only once the code to support the i386
intrinsic is added as well, the checks seem superfluous.

Maybe I'm missing something obvious.
Eric Botcazou - Oct. 11, 2011, 10:33 p.m.
> I see, so we can test the code generation in the testsuite even if the
> compiler was built against an assembler without support for the
> instructions.

At least partially, yes.

> But in such a case, I'm unsure if I understand why i386.exp needs
> these tests at all.  The presence of support for a particular i386
> intrinsic is an implicit property of the gcc sources that these test
> cases are a part of.
>
> If the tests are properly added only once the code to support the i386
> intrinsic is added as well, the checks seem superfluous.

The check is an _object_ check, for example:

proc check_effective_target_sse4 { } {
    return [check_no_compiler_messages sse4.1 object {

so it checks that an object file can be produced.  You indeed don't need to 
invoke the check via the sse4.1 tag if you use:

/* { dg-do compile } */

in your tests, but you do need the sse4.1 tag if you use:

/* { dg-do assemble } */

or

/* { dg-do run } */

So the first category of tests will always be executed, whereas the latter two 
will only be executed if you have the binutils support.

Patch

diff --git a/gcc/testsuite/gcc.target/sparc/sparc.exp b/gcc/testsuite/gcc.target/sparc/sparc.exp
index 51c9c16..7d30dcc 100644
--- a/gcc/testsuite/gcc.target/sparc/sparc.exp
+++ b/gcc/testsuite/gcc.target/sparc/sparc.exp
@@ -32,7 +32,7 @@  proc check_effective_target_vis3 { } {
 	{
 	    return __builtin_vis_fpadd64 (__X, __Y);
 	}
-    } "-mcpu=niagara3 -mvis" ]
+    } "-mcpu=niagara3 -mvis -Wall" ]
 }
 
 # If a testcase doesn't have special options, use these.