diff mbox series

Don't run ident tests on powerpc-darwin

Message ID 337A44FA-78DE-4CA1-9AF1-4A287B2DF023@sandoe.co.uk
State New
Headers show
Series Don't run ident tests on powerpc-darwin | expand

Commit Message

Iain Sandoe Dec. 23, 2018, 11:16 a.m. UTC 
Hi

The c-c++-common tests fail (or XPASS depending on which) on Darwin
because it doesn't currently emit .ident marker. 

For powerpc darwin (and, I think, AIX - hence copying David), there’s no
.ident support in the assembler, and we need to skip the tests.

In this case, I suggest that it’s not worth having a target-supports entry.
This is because what a target chooses to emit for -fident is controlled by
a target hook and therefore there’s no guarantee that a target-supports
that triggers off “ .ident” in the asm would be generically useful.

OTOH, if the testsuite maintainers prefer a support .. it can be done ;-)

OK for trunk?
Iain

gcc/testsuite/

	* c-c++-common/ident-0a.c: Skip for powerpc-darwin.
	* c-c++-common/ident-0b.c: Likewise.
	* c-c++-common/ident-1a.c: Likewise.
	* c-c++-common/ident-1b.c: Likewise.
	* c-c++-common/ident-2a.c: Likewise.
	* c-c++-common/ident-2b.c: Likewise.

Comments

Jeff Law Dec. 23, 2018, 4:25 p.m. UTC | #1 
On 12/23/18 4:16 AM, Iain Sandoe wrote:
> Hi
> 
> The c-c++-common tests fail (or XPASS depending on which) on Darwin
> because it doesn't currently emit .ident marker. 
> 
> For powerpc darwin (and, I think, AIX - hence copying David), there’s no
> .ident support in the assembler, and we need to skip the tests.
> 
> In this case, I suggest that it’s not worth having a target-supports entry.
> This is because what a target chooses to emit for -fident is controlled by
> a target hook and therefore there’s no guarantee that a target-supports
> that triggers off “ .ident” in the asm would be generically useful.
> 
> OTOH, if the testsuite maintainers prefer a support .. it can be done ;-)
> 
> OK for trunk?
> Iain
> 
> gcc/testsuite/
> 
> 	* c-c++-common/ident-0a.c: Skip for powerpc-darwin.
> 	* c-c++-common/ident-0b.c: Likewise.
> 	* c-c++-common/ident-1a.c: Likewise.
> 	* c-c++-common/ident-1b.c: Likewise.
> 	* c-c++-common/ident-2a.c: Likewise.
> 	* c-c++-common/ident-2b.c: Likewise.
OK
jeff
David Edelsohn Dec. 26, 2018, 4:07 p.m. UTC | #2 
On Sun, Dec 23, 2018 at 6:16 AM Iain Sandoe <iain@sandoe.co.uk> wrote:
>
> Hi
>
> The c-c++-common tests fail (or XPASS depending on which) on Darwin
> because it doesn't currently emit .ident marker.
>
> For powerpc darwin (and, I think, AIX - hence copying David), there’s no
> .ident support in the assembler, and we need to skip the tests.
>
> In this case, I suggest that it’s not worth having a target-supports entry.
> This is because what a target chooses to emit for -fident is controlled by
> a target hook and therefore there’s no guarantee that a target-supports
> that triggers off “ .ident” in the asm would be generically useful.
>
> OTOH, if the testsuite maintainers prefer a support .. it can be done ;-)
>
> OK for trunk?
> Iain
>
> gcc/testsuite/
>
>         * c-c++-common/ident-0a.c: Skip for powerpc-darwin.
>         * c-c++-common/ident-0b.c: Likewise.
>         * c-c++-common/ident-1a.c: Likewise.
>         * c-c++-common/ident-1b.c: Likewise.
>         * c-c++-common/ident-2a.c: Likewise.
>         * c-c++-common/ident-2b.c: Likewise.

AIX Assembler doesn't support "ident", but GCC doesn't emit an "ident"
string on AIX.  The failure on AIX is the assembler output doesn't
match the expected result, but it doesn't fail to assemble due to an
unrecognized pseudo-op.

Does GCC generate an ident pseudo-op on Darwin?  Maybe because of
common code inherited by Darwin that defines TARGET_ASM_OUTPUT_IDENT.

I had been testing an expected fail for those tests on AIX, e.g.,

Index: ident-2b.c
===================================================================
--- ident-2b.c  (revision 267421)
+++ ident-2b.c  (working copy)
@@ -5,4 +5,4 @@
 /* { dg-skip-if "no assembler .ident support" { powerpc*-*-darwin* } } */
 int ident;

-/* { dg-final { scan-assembler-times "GCC: " 1 } } */
+/* { dg-final { scan-assembler-times "GCC: " 1 { xfail
powerpc-ibm-aix* } } } */

Thanks, David
Iain Sandoe Dec. 26, 2018, 4:18 p.m. UTC | #3 
> On 26 Dec 2018, at 16:07, David Edelsohn <dje.gcc@gmail.com> wrote:
> 
> On Sun, Dec 23, 2018 at 6:16 AM Iain Sandoe <iain@sandoe.co.uk> wrote:
>> 
>> Hi
>> 
>> The c-c++-common tests fail (or XPASS depending on which) on Darwin
>> because it doesn't currently emit .ident marker.
>> 
>> For powerpc darwin (and, I think, AIX - hence copying David), there’s no
>> .ident support in the assembler, and we need to skip the tests.
>> 
>> In this case, I suggest that it’s not worth having a target-supports entry.
>> This is because what a target chooses to emit for -fident is controlled by
>> a target hook and therefore there’s no guarantee that a target-supports
>> that triggers off “ .ident” in the asm would be generically useful.
>> 
>> OTOH, if the testsuite maintainers prefer a support .. it can be done ;-)
>> 
>> OK for trunk?
>> Iain
>> 
>> gcc/testsuite/
>> 
>>        * c-c++-common/ident-0a.c: Skip for powerpc-darwin.
>>        * c-c++-common/ident-0b.c: Likewise.
>>        * c-c++-common/ident-1a.c: Likewise.
>>        * c-c++-common/ident-1b.c: Likewise.
>>        * c-c++-common/ident-2a.c: Likewise.
>>        * c-c++-common/ident-2b.c: Likewise.
> 
> AIX Assembler doesn't support "ident", but GCC doesn't emit an "ident"
> string on AIX.  The failure on AIX is the assembler output doesn't
> match the expected result, but it doesn't fail to assemble due to an
> unrecognized pseudo-op.

Right that’s the cause of the XPASSes and FAILs  respectively.
> 
> Does GCC generate an ident pseudo-op on Darwin?  
> Maybe because of
> common code inherited by Darwin that defines TARGET_ASM_OUTPUT_IDENT.

no that is disabled there too.

Darwin was not trying to emit the .ident (TARGET_ASM_OUTPUT_IDENT was undefined).

That leads to the effect noted above.  Since the assembler doesn’t support .ident we can’t fix the tests by adding TARGET_ASM_OUTPUT_IDENT, so I’ve skipped them.  

If, one day, we fix the assembler to accept .ident (or use a more modern one that already does) then those skip lines can be removed.

> I had been testing an expected fail for those tests on AIX, e.g.,

Ah, that might be an alternate scheme, indeed.  However, for now (on Darwin) I just settled for the simple one.

Iain

> 
> Index: ident-2b.c
> ===================================================================
> --- ident-2b.c  (revision 267421)
> +++ ident-2b.c  (working copy)
> @@ -5,4 +5,4 @@
> /* { dg-skip-if "no assembler .ident support" { powerpc*-*-darwin* } } */
> int ident;
> 
> -/* { dg-final { scan-assembler-times "GCC: " 1 } } */
> +/* { dg-final { scan-assembler-times "GCC: " 1 { xfail
> powerpc-ibm-aix* } } } */
> 
> Thanks, David
Rainer Orth Jan. 2, 2019, 1:39 p.m. UTC | #4 
Hi Iain,

> The c-c++-common tests fail (or XPASS depending on which) on Darwin
> because it doesn't currently emit .ident marker. 
>
> For powerpc darwin (and, I think, AIX - hence copying David), there’s no
> .ident support in the assembler, and we need to skip the tests.
>
> In this case, I suggest that it’s not worth having a target-supports entry.
> This is because what a target chooses to emit for -fident is controlled by
> a target hook and therefore there’s no guarantee that a target-supports
> that triggers off “ .ident” in the asm would be generically useful.

however, if you look at the three implementations not using
default_asm_output_ident_directive (cris, microblaze, and pdp11), cris
calls the default if it does anything and the pdp11 string also includes
.ident.  Only microblaze desn't.

> OTOH, if the testsuite maintainers prefer a support .. it can be done ;-)

It's on the brink, I'd say.  The effective-keyword form is shorter and
more descriptive and can be extended to more targets way more easily.
Checking 2018-12 testresults, I find failures of the ident-*.c tests on
hppa2.0w-hp-hpux11.11 and powerpc-ibm-aix7.2.0.0.  If those were to be
handled as well, I'd strongly prefer the effective-keyword variant
rather than adding more and more targets to the individual tests.

	Rainer
Mike Stump Jan. 3, 2019, 10:16 p.m. UTC | #5 
On Jan 2, 2019, at 5:39 AM, Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> wrote:
> 
> It's on the brink, I'd say.  The effective-keyword form is shorter and
> more descriptive and can be extended to more targets way more easily.
> Checking 2018-12 testresults, I find failures of the ident-*.c tests on
> hppa2.0w-hp-hpux11.11 and powerpc-ibm-aix7.2.0.0.  If those were to be
> handled as well, I'd strongly prefer the effective-keyword variant
> rather than adding more and more targets to the individual tests.

For a completely different thought, we can compare to a fine grain feature database that dejagnu or the running program can probe and adjust for...

#ifdef __has_feature(directive_ident)
#endif

then, this is directly populated from the port's #define, and the duplication is gone.  I think I like this direction.  Let maintenance, faster port time, higher quality port, and testing less noisy in general.
diff mbox series

Patch

diff --git a/gcc/testsuite/c-c++-common/ident-0a.c b/gcc/testsuite/c-c++-common/ident-0a.c
index 900d206bad..13bc6284de 100644
--- a/gcc/testsuite/c-c++-common/ident-0a.c
+++ b/gcc/testsuite/c-c++-common/ident-0a.c
@@ -1,6 +1,7 @@ 
 /* PR testsuite/52665
  * Make sure scan-assembler-not turns off .ident  */
 /* { dg-do compile } */
+/* { dg-skip-if "no assembler .ident support" { powerpc*-*-darwin* } } */
 int i;
 
 /* { dg-final { scan-assembler-not "GCC: " } } */
diff --git a/gcc/testsuite/c-c++-common/ident-0b.c b/gcc/testsuite/c-c++-common/ident-0b.c
index e08126d2d2..1184f8ff1c 100644
--- a/gcc/testsuite/c-c++-common/ident-0b.c
+++ b/gcc/testsuite/c-c++-common/ident-0b.c
@@ -2,6 +2,7 @@ 
  * Make sure scan-assembler-not turns off .ident unless -fident in testcase */
 /* { dg-do compile } */
 /* { dg-options "-fident" } */
+/* { dg-skip-if "no assembler .ident support" { powerpc*-*-darwin* } } */
 int i;
 
 /* { dg-final { scan-assembler-not "GCC: " { xfail *-*-* } } } */
diff --git a/gcc/testsuite/c-c++-common/ident-1a.c b/gcc/testsuite/c-c++-common/ident-1a.c
index 867ee43fb2..b6003735ce 100644
--- a/gcc/testsuite/c-c++-common/ident-1a.c
+++ b/gcc/testsuite/c-c++-common/ident-1a.c
@@ -1,6 +1,7 @@ 
 /* PR testsuite/52665
  * Make sure scan-assembler turns off .ident  */
 /* { dg-do compile } */
+/* { dg-skip-if "no assembler .ident support" { powerpc*-*-darwin* } } */
 int i;
 
 /* { dg-final { scan-assembler "GCC: " { xfail *-*-* } } } */
diff --git a/gcc/testsuite/c-c++-common/ident-1b.c b/gcc/testsuite/c-c++-common/ident-1b.c
index 2431086d24..b0d88983f3 100644
--- a/gcc/testsuite/c-c++-common/ident-1b.c
+++ b/gcc/testsuite/c-c++-common/ident-1b.c
@@ -2,6 +2,7 @@ 
  * Make sure scan-assembler turns off .ident unless -fident in testcase */
 /* { dg-do compile } */
 /* { dg-options "-fident" } */
+/* { dg-skip-if "no assembler .ident support" { powerpc*-*-darwin* } } */
 int i;
 
 /* { dg-final { scan-assembler "GCC: " } } */
diff --git a/gcc/testsuite/c-c++-common/ident-2a.c b/gcc/testsuite/c-c++-common/ident-2a.c
index 131b867628..e9321c7292 100644
--- a/gcc/testsuite/c-c++-common/ident-2a.c
+++ b/gcc/testsuite/c-c++-common/ident-2a.c
@@ -1,6 +1,7 @@ 
 /* PR testsuite/52665
  * Make sure scan-assembler-times turns off .ident  */
 /* { dg-do compile } */
+/* { dg-skip-if "no assembler .ident support" { powerpc*-*-darwin* } } */
 int i;
 
 /* { dg-final { scan-assembler-times "GCC: " 0 } } */ /* internal test, keep -times 0 ! */
diff --git a/gcc/testsuite/c-c++-common/ident-2b.c b/gcc/testsuite/c-c++-common/ident-2b.c
index a21e25fcce..e057fac747 100644
--- a/gcc/testsuite/c-c++-common/ident-2b.c
+++ b/gcc/testsuite/c-c++-common/ident-2b.c
@@ -2,6 +2,7 @@ 
  * Make sure scan-assembler-times turns off .ident unless -fident in testcase */
 /* { dg-do compile } */
 /* { dg-options "-fident" } */
+/* { dg-skip-if "no assembler .ident support" { powerpc*-*-darwin* } } */
 int ident;
 
 /* { dg-final { scan-assembler-times "GCC: " 1 } } */