diff mbox

[gofrontend-dev] Re: [PATCH 7/9] Gccgo port to s390[x] -- part I

Message ID CAOyqgcWnnmHKzxDTYN5_z8d8oUgy_aZW-GuvTr3SnoADvcZz_Q@mail.gmail.com
State New
Headers show

Commit Message

Ian Lance Taylor Oct. 30, 2014, 2:51 p.m. UTC 
On Thu, Oct 30, 2014 at 12:25 AM, Dominik Vogt <vogt@linux.vnet.ibm.com> wrote:
> On Wed, Oct 29, 2014 at 10:16:40AM -0700, Ian Taylor wrote:
>> Thanks.  Part of the problem is that the m68k max alignment is 16
>> bits, but the godump test expects it to be at least 64 bits.  This is
>> BIGGEST_ALIGNMENT in config/m68k/m68k.h.  Another part of the problem
>> seems to be that structs are sometimes aligned to 16 bits although
>> there is no obvious reason for that.  I'm not sure where that is
>> coming from.
>
> Hm, the test cases make assumptions about the Abi (structure
> padding and alignment) that are true on x86, x64_64 and s390[x].
> That may not be the case on other platforms and hence the tests
> fail.  Another candidate for test failures is the effect of
> bitfields (named or anonymous) on structure layout.
>
>> We could disable the test on m68k or make it more accepting.
>
> Since the point of some of the tests is to make sure that the Go
> structure layout matches the C layout, making the tests accept
> deviations seems to be rather pointless.  It's possible to add
> target selectors to all the "scan-file" lines, but that is a lot
> of work and will likely become unmaintainable very soon when more
> platforms need to be added.  Personally I cannot provide fixed
> tests for all the Abis either, so my suggestion is to "xfail" the
> test on all targets except s390[x] and x86_64 and leave it to the
> people who know the other platforms to decide whether the test
> should work there or how the test could be modified to work.
>
> See attached patch.

I don't mind skipping the test on other platforms, but xfail is not
correct.  When an xfail test passes, we get an XPASS error from the
testsuite.  We need dg-skip-if.  I committed this patch.

Ian


2014-10-30  Dominik Vogt  <vogt@linux.vnet.ibm.com>

	* gcc.misc-tests/godump-1.c: Skip -fdump-go-spec tests for all
	platforms except s390[x] and x86_64.

Comments

Dominik Vogt Oct. 31, 2014, 9:11 a.m. UTC | #1 
On Thu, Oct 30, 2014 at 07:51:45AM -0700, Ian Taylor wrote:
> On Thu, Oct 30, 2014 at 12:25 AM, Dominik Vogt <vogt@linux.vnet.ibm.com> wrote:
> > See attached patch.
> 
> I don't mind skipping the test on other platforms, but xfail is not
> correct.  When an xfail test passes, we get an XPASS error from the
> testsuite.  We need dg-skip-if.  I committed this patch.

That is exactly the reason why I chose dg-xfail-if:  To identify
the targets where the test works out of the box, because I think
there won't be many of them.

But anyway, we can leave it as it is for the moment and eventually
I'll get around cleaning that up.

Ciao

Dominik ^_^  ^_^
Ian Lance Taylor Oct. 31, 2014, 4:41 p.m. UTC | #2 
On Fri, Oct 31, 2014 at 2:11 AM, Dominik Vogt <vogt@linux.vnet.ibm.com> wrote:
> On Thu, Oct 30, 2014 at 07:51:45AM -0700, Ian Taylor wrote:
>> On Thu, Oct 30, 2014 at 12:25 AM, Dominik Vogt <vogt@linux.vnet.ibm.com> wrote:
>> > See attached patch.
>>
>> I don't mind skipping the test on other platforms, but xfail is not
>> correct.  When an xfail test passes, we get an XPASS error from the
>> testsuite.  We need dg-skip-if.  I committed this patch.
>
> That is exactly the reason why I chose dg-xfail-if:  To identify
> the targets where the test works out of the box, because I think
> there won't be many of them.

That would be fine if coupled with an immediate plan to test on all
systems.  What we don't want is an XPASS on some random system six
months from now that forces some poor GCC developer who knows nothing
at all about this code to figure out what is going on.


> But anyway, we can leave it as it is for the moment and eventually
> I'll get around cleaning that up.

OK.

Ian
diff mbox

Patch

Index: gcc.misc-tests/godump-1.c
===================================================================
--- gcc.misc-tests/godump-1.c	(revision 216840)
+++ gcc.misc-tests/godump-1.c	(working copy)
@@ -2,6 +2,7 @@ 
 
 /* { dg-options "-c -fdump-go-spec=godump-1.out" } */
 /* { dg-do compile } */
+/* { dg-skip-if "not supported for target" { ! "s390*-*-* x86_64-*-*" } } */
 
 #include <stdint.h>