Re: [gofrontend-dev] Re: [PATCH 7/9] Gccgo port to s390[x] -- part I
On Thu, Oct 30, 2014 at 08:20:20AM -0700, Ian Taylor wrote: /* { dg-skip-if not supported for target { ! s390*-*-* x86_64-*-* } } */ +/* { dg-skip-if not supported for target { ! lp64 } } */ Hm, but then the test will be skipped on s390 (31 bit) too where it should work. The current solution is hopefully good enough for now, but maybe boolean operators in the selector can help to improve it; I'll check that, but give me some time for more important issues first. Ciao Dominik ^_^ ^_^ -- Dominik Vogt IBM Germany
Re: [gofrontend-dev] Re: [PATCH 7/9] Gccgo port to s390[x] -- part I
On Thu, Oct 30, 2014 at 07:51:45AM -0700, Ian Taylor wrote: On Thu, Oct 30, 2014 at 12:25 AM, Dominik Vogt v...@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 ^_^ ^_^ -- Dominik Vogt IBM Germany
Re: [gofrontend-dev] Re: [PATCH 7/9] Gccgo port to s390[x] -- part I
On Fri, Oct 31, 2014 at 2:11 AM, Dominik Vogt v...@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 v...@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
Re: [gofrontend-dev] Re: [PATCH 7/9] Gccgo port to s390[x] -- part I
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. Ciao Dominik ^_^ ^_^ -- Dominik Vogt IBM Germany From fc92faa8532e3ea0ac3b4c8b18eb6b0a3ee862dc Mon Sep 17 00:00:00 2001 From: Dominik Vogt v...@linux.vnet.ibm.com Date: Thu, 30 Oct 2014 07:50:18 +0100 Subject: [PATCH] Xfail -fdump-go-spec tests for all platforms except s390[x] and x86_64. --- gcc/testsuite/gcc.misc-tests/godump-1.c | 1 + 1 file changed, 1 insertion(+) diff --git a/gcc/testsuite/gcc.misc-tests/godump-1.c b/gcc/testsuite/gcc.misc-tests/godump-1.c index f339cc9..91b8637 100644 --- a/gcc/testsuite/gcc.misc-tests/godump-1.c +++ b/gcc/testsuite/gcc.misc-tests/godump-1.c @@ -2,6 +2,7 @@ /* { dg-options -c -fdump-go-spec=godump-1.out } */ /* { dg-do compile } */ +/* { dg-xfail-if not supported for target { ! s390*-*-* x86_64-*-* } } */ #include stdint.h -- 1.8.4.2
Re: [gofrontend-dev] Re: [PATCH 7/9] Gccgo port to s390[x] -- part I
On Thu, Oct 30, 2014 at 12:25 AM, Dominik Vogt v...@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 v...@linux.vnet.ibm.com * gcc.misc-tests/godump-1.c: Skip -fdump-go-spec tests for all platforms except s390[x] and x86_64. 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
Re: [gofrontend-dev] Re: [PATCH 7/9] Gccgo port to s390[x] -- part I
Hello! 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. 2014-10-30 Dominik Vogt v...@linux.vnet.ibm.com * gcc.misc-tests/godump-1.c: Skip -fdump-go-spec tests for all platforms except s390[x] and x86_64. /* { dg-options -c -fdump-go-spec=godump-1.out } */ /* { dg-do compile } */ +/* { dg-skip-if not supported for target { ! s390*-*-* x86_64-*-* } } */ x86_64 also needs lp64, otherwise -m32 will defeat this condition. Uros.
Re: [gofrontend-dev] Re: [PATCH 7/9] Gccgo port to s390[x] -- part I
On Thu, Oct 30, 2014 at 8:05 AM, Uros Bizjak ubiz...@gmail.com wrote: /* { dg-options -c -fdump-go-spec=godump-1.out } */ /* { dg-do compile } */ +/* { dg-skip-if not supported for target { ! s390*-*-* x86_64-*-* } } */ x86_64 also needs lp64, otherwise -m32 will defeat this condition. Thanks. I committed this patch. Ian 2014-10-30 Ian Lance Taylor i...@google.com * gcc.misc-tests/godump-1.c: Skip if ! lp64. Index: gcc.misc-tests/godump-1.c === --- gcc.misc-tests/godump-1.c (revision 216936) +++ gcc.misc-tests/godump-1.c (working copy) @@ -3,6 +3,7 @@ /* { dg-options -c -fdump-go-spec=godump-1.out } */ /* { dg-do compile } */ /* { dg-skip-if not supported for target { ! s390*-*-* x86_64-*-* } } */ +/* { dg-skip-if not supported for target { ! lp64 } } */ #include stdint.h
Re: [gofrontend-dev] Re: [PATCH 7/9] Gccgo port to s390[x] -- part I
On Thu, 30 Oct 2014, Dominik Vogt wrote: 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 You should never do something in a test for x86_64 and not i?86, because they cover exactly the same set of targets (if only LP64 x86 / x86_64 is relevant, use { { i?86-*-* x86_64-*-* } lp64 }). -- Joseph S. Myers jos...@codesourcery.com
Re: [gofrontend-dev] [PATCH 7/9] Gccgo port to s390[x] -- part I
On Tue, Oct 28, 2014 at 10:30:08AM -0700, Ian Taylor wrote: On Tue, Oct 28, 2014 at 7:31 AM, Dominik Vogt v...@linux.vnet.ibm.com wrote: The attached patch contains all the discussed changes. I made a few formatting changes. I patched the test to work on x86, by making the char types accept either int8 or uint8, and making the long double tests accept any floating point size. Approved and applied as attached. Great, thanks! By the way, the changes I made to this patch do not interfer with patch #8 (complex type support for -fdump-go-spec) in any way - it should still apply without conflict. Ciao Dominik ^_^ ^_^ -- Dominik Vogt IBM Germany
Re: [PATCH 7/9] Gccgo port to s390[x] -- part I
I'm getting these test failures on m68k-linux: FAIL: gcc.misc-tests/godump-1.c scan-file (?n)^type _tsbf_anon_pad1 struct { c uint[0-9]*; }$ FAIL: gcc.misc-tests/godump-1.c scan-file (?n)^type _tsbf_anon_pad5 struct { c uint[0-9]*; }$ FAIL: gcc.misc-tests/godump-1.c scan-file (?n)^type _tsbf_pad16_1 struct { Godump_0_pad \\[1\\]byte; c uint[0-9]*; Godump_1_align \\[0\\]int[0-9]*; }$ FAIL: gcc.misc-tests/godump-1.c scan-file (?n)^type _tsbf_pad16_2 struct { Godump_0_pad \\[2\\]byte; c uint[0-9]*; Godump_1_align \\[0\\]int[0-9]*; }$ FAIL: gcc.misc-tests/godump-1.c scan-file (?n)^type _tsbf_pad32_1 struct { Godump_0_pad \\[1\\]byte; c uint[0-9]*; Godump_1_align \\[0\\]int[0-9]*; }$ FAIL: gcc.misc-tests/godump-1.c scan-file (?n)^type _tsbf_pad32_2 struct { Godump_0_pad \\[4\\]byte; c uint[0-9]*; Godump_1_align \\[0\\]int[0-9]*; }$ FAIL: gcc.misc-tests/godump-1.c scan-file (?n)^type _tsbf_pad64_1 struct { Godump_0_pad \\[1\\]byte; c uint[0-9]*; Godump_1_align \\[0\\]int[0-9]*; }$ FAIL: gcc.misc-tests/godump-1.c scan-file (?n)^type _tsbf_pad64_2 struct { Godump_0_pad \\[8\\]byte; c uint[0-9]*; Godump_1_align \\[0\\]int[0-9]*; }$ FAIL: gcc.misc-tests/godump-1.c scan-file (?n)^type _tsbf_18b struct { Godump_0_pad \\[3\\]byte; }$ FAIL: gcc.misc-tests/godump-1.c scan-file (?n)^type _tsbf_gaps struct { bf1 uint[0-9]*; c uint[0-9]*; bf2 uint[0-9]*; Godump_0_pad \\[2\\]byte; s uint[0-9]*; Godump_1_align \\[0\\]int[0-9]*; }$ FAIL: gcc.misc-tests/godump-1.c scan-file (?n)^type _tu1 struct { c \\[8\\]byte; Godump_0_align \\[0\\]uint64; }$ FAIL: gcc.misc-tests/godump-1.c scan-file (?n)^type _tu2 struct { l \\[8\\]byte; Godump_0_align \\[0\\]uint64; }$ FAIL: gcc.misc-tests/godump-1.c scan-file (?n)^type _tu3 struct { l \\[24\\]byte; Godump_0_align \\[0\\]uint64; }$ FAIL: gcc.misc-tests/godump-1.c scan-file (?n)^type _tsu_anon struct { Godump_0 struct { c \\[8\\]byte; Godump_1_align \\[0\\]uint64; }; }$ FAIL: gcc.misc-tests/godump-1.c scan-file (?n)^type _tu_size struct { bf \\[8\\]byte; Godump_0_align \\[0\\]uint64; }$ FAIL: gcc.misc-tests/godump-1.c scan-file (?n)^type _tu3_size struct { b \\[8\\]byte; Godump_0_align \\[0\\]uint64; }$ FAIL: gcc.misc-tests/godump-1.c scan-file (?n)^var _sbf_anon_pad1 struct { c uint[0-9]*; }$ FAIL: gcc.misc-tests/godump-1.c scan-file (?n)^var _sbf_anon_pad5 struct { c uint[0-9]*; }$ FAIL: gcc.misc-tests/godump-1.c scan-file (?n)^var _sbf_pad16_1 struct { Godump_0_pad \\[1\\]byte; c uint[0-9]*; Godump_1_align \\[0\\]int[0-9]*; }$ FAIL: gcc.misc-tests/godump-1.c scan-file (?n)^var _sbf_pad16_2 struct { Godump_0_pad \\[2\\]byte; c uint[0-9]*; Godump_1_align \\[0\\]int[0-9]*; }$ FAIL: gcc.misc-tests/godump-1.c scan-file (?n)^var _sbf_pad32_1 struct { Godump_0_pad \\[1\\]byte; c uint[0-9]*; Godump_1_align \\[0\\]int[0-9]*; }$ FAIL: gcc.misc-tests/godump-1.c scan-file (?n)^var _sbf_pad32_2 struct { Godump_0_pad \\[4\\]byte; c uint[0-9]*; Godump_1_align \\[0\\]int[0-9]*; }$ FAIL: gcc.misc-tests/godump-1.c scan-file (?n)^var _sbf_pad64_1 struct { Godump_0_pad \\[1\\]byte; c uint[0-9]*; Godump_1_align \\[0\\]int[0-9]*; }$ FAIL: gcc.misc-tests/godump-1.c scan-file (?n)^var _sbf_pad64_2 struct { Godump_0_pad \\[8\\]byte; c uint[0-9]*; Godump_1_align \\[0\\]int[0-9]*; }$ FAIL: gcc.misc-tests/godump-1.c scan-file (?n)^var _sbf_18b struct { Godump_0_pad \\[3\\]byte; }$ FAIL: gcc.misc-tests/godump-1.c scan-file (?n)^var _sbf_gaps struct { bf1 uint[0-9]*; c uint[0-9]*; bf2 uint[0-9]*; Godump_0_pad \\[2\\]byte; s uint[0-9]*; Godump_1_align \\[0\\]int[0-9]*; }$ FAIL: gcc.misc-tests/godump-1.c scan-file (?n)^var _u1 struct { c \\[8\\]byte; Godump_0_align \\[0\\]uint64; }$ FAIL: gcc.misc-tests/godump-1.c scan-file (?n)^var _u2 struct { l \\[8\\]byte; Godump_0_align \\[0\\]uint64; }$ FAIL: gcc.misc-tests/godump-1.c scan-file (?n)^var _u3 struct { l \\[24\\]byte; Godump_0_align \\[0\\]uint64; }$ FAIL: gcc.misc-tests/godump-1.c scan-file (?n)^var _su_anon struct { Godump_0 struct { c \\[8\\]byte; Godump_1_align \\[0\\]uint64; }; }$ FAIL: gcc.misc-tests/godump-1.c scan-file (?n)^var _u_size struct { bf \\[8\\]byte; Godump_0_align \\[0\\]uint64; }$ FAIL: gcc.misc-tests/godump-1.c scan-file (?n)^var _u3_size struct { b \\[8\\]byte; Godump_0_align \\[0\\]uint64; }$ Andreas. -- Andreas Schwab, SUSE Labs, sch...@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 And now for something completely different.
Re: [gofrontend-dev] Re: [PATCH 7/9] Gccgo port to s390[x] -- part I
On Wed, Oct 29, 2014 at 9:38 AM, Andreas Schwab sch...@suse.de wrote: I'm getting these test failures on m68k-linux: Can you send the file BUILDDIR/gcc/testsuite/gcc/godump-1.out? Ian
Re: [gofrontend-dev] Re: [PATCH 7/9] Gccgo port to s390[x] -- part I
// unknowndefine __SIZE_TYPE__ unsigned int // unknowndefine __PTRDIFF_TYPE__ int // unknowndefine __WCHAR_TYPE__ long int // unknowndefine __WINT_TYPE__ unsigned int // unknowndefine __INTMAX_TYPE__ long long int // unknowndefine __UINTMAX_TYPE__ long long unsigned int // unknowndefine __CHAR16_TYPE__ short unsigned int // unknowndefine __CHAR32_TYPE__ unsigned int // unknowndefine __SIG_ATOMIC_TYPE__ int // unknowndefine __INT8_TYPE__ signed char // unknowndefine __INT16_TYPE__ short int // unknowndefine __INT32_TYPE__ int // unknowndefine __INT64_TYPE__ long long int // unknowndefine __UINT8_TYPE__ unsigned char // unknowndefine __UINT16_TYPE__ short unsigned int // unknowndefine __UINT32_TYPE__ unsigned int // unknowndefine __UINT64_TYPE__ long long unsigned int // unknowndefine __INT_LEAST8_TYPE__ signed char // unknowndefine __INT_LEAST16_TYPE__ short int // unknowndefine __INT_LEAST32_TYPE__ int // unknowndefine __INT_LEAST64_TYPE__ long long int // unknowndefine __UINT_LEAST8_TYPE__ unsigned char // unknowndefine __UINT_LEAST16_TYPE__ short unsigned int // unknowndefine __UINT_LEAST32_TYPE__ unsigned int // unknowndefine __UINT_LEAST64_TYPE__ long long unsigned int // unknowndefine __INT_FAST8_TYPE__ signed char // unknowndefine __INT_FAST16_TYPE__ int // unknowndefine __INT_FAST32_TYPE__ int // unknowndefine __INT_FAST64_TYPE__ long long int // unknowndefine __UINT_FAST8_TYPE__ unsigned char // unknowndefine __UINT_FAST16_TYPE__ unsigned int // unknowndefine __UINT_FAST32_TYPE__ unsigned int // unknowndefine __UINT_FAST64_TYPE__ long long unsigned int // unknowndefine __INTPTR_TYPE__ int // unknowndefine __UINTPTR_TYPE__ unsigned int // unknowndefine __DBL_MAX__ ((double)1.1) // unknowndefine __DBL_MIN__ ((double)1.1) // unknowndefine __DBL_EPSILON__ ((double)1.1) // unknowndefine __DBL_DENORM_MIN__ ((double)1.1) // unknowndefine __REGISTER_PREFIX__ % // unknowndefine __LEAF , __leaf__ // unknowndefine __LEAF_ATTR __attribute__ ((__leaf__)) // unknowndefine __THROW __attribute__ ((__nothrow__ __LEAF)) // unknowndefine __THROWNL __attribute__ ((__nothrow__)) // unknowndefine __ptr_t void * // unknowndefine __long_double_t long double // unknowndefine __fortify_function __extern_always_inline __attribute_artificial__ // unknowndefine __flexarr [] // unknowndefine __attribute_malloc__ __attribute__ ((__malloc__)) // unknowndefine __attribute_pure__ __attribute__ ((__pure__)) // unknowndefine __attribute_const__ __attribute__ ((__const__)) // unknowndefine __attribute_used__ __attribute__ ((__used__)) // unknowndefine __attribute_noinline__ __attribute__ ((__noinline__)) // unknowndefine __attribute_deprecated__ __attribute__ ((__deprecated__)) // unknowndefine __attribute_warn_unused_result__ __attribute__ ((__warn_unused_result__)) // unknowndefine __always_inline __inline __attribute__ ((__always_inline__)) // unknowndefine __attribute_artificial__ __attribute__ ((__artificial__)) // unknowndefine __extern_inline extern __inline __attribute__ ((__gnu_inline__)) // unknowndefine __extern_always_inline extern __always_inline __attribute__ ((__gnu_inline__)) // unknowndefine __restrict_arr __restrict // unknowndefine INT64_MIN (-__INT64_C(9223372036854775807)-1) // unknowndefine INT64_MAX (__INT64_C(9223372036854775807)) // unknowndefine UINT64_MAX (__UINT64_C(18446744073709551615)) // unknowndefine INT_LEAST64_MIN (-__INT64_C(9223372036854775807)-1) // unknowndefine INT_LEAST64_MAX (__INT64_C(9223372036854775807)) // unknowndefine UINT_LEAST64_MAX (__UINT64_C(18446744073709551615)) // unknowndefine INT_FAST64_MIN (-__INT64_C(9223372036854775807)-1) // unknowndefine INT_FAST64_MAX (__INT64_C(9223372036854775807)) // unknowndefine UINT_FAST64_MAX (__UINT64_C(18446744073709551615)) // unknowndefine INTMAX_MIN (-__INT64_C(9223372036854775807)-1) // unknowndefine INTMAX_MAX (__INT64_C(9223372036854775807)) // unknowndefine UINTMAX_MAX (__UINT64_C(18446744073709551615)) type _int8_t int8 type _int16_t int16 type _int32_t int32 type _int64_t int64 type _uint8_t uint8 type _uint16_t uint16 type _uint32_t uint32 type _uint64_t uint64 type _int_least8_t int8 type _int_least16_t int16 type _int_least32_t int32 type _int_least64_t int64 type _uint_least8_t uint8 type _uint_least16_t uint16 type _uint_least32_t uint32 type _uint_least64_t uint64 type _int_fast8_t int8 type _int_fast16_t int32 type _int_fast32_t int32 type _int_fast64_t int64 type _uint_fast8_t uint8 type _uint_fast16_t uint32 type _uint_fast32_t uint32 type _uint_fast64_t uint64 type _intptr_t int32 type _uintptr_t uint32 type _intmax_t int64 type _uintmax_t uint64 type _c_t int8 type _s_t int16 type _i_t int32 type _l_t int32 type _ll_t int64 type _uc_t uint8 type _us_t uint16 type _ui_t uint32 type _ul_t uint32 type _ull_t uint64 type _sc_t int8 type _ss_t int16 type _si_t int32 type _sl_t int32 type _sll_t int64 type _i8_t int8 type _i16_t int16 type _i32_t int32 type _i64_t int64 type _ui8_t uint8 type _iu16_t uint16 type _iu32_t uint32
Re: [gofrontend-dev] Re: [PATCH 7/9] Gccgo port to s390[x] -- part I
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. I'll let Dominik decide how he wants to handle this. We could disable the test on m68k or make it more accepting. There may be problems on other architectures as well. Ian
Re: [gofrontend-dev] [PATCH 7/9] Gccgo port to s390[x] -- part I
On Thu, Oct 16, 2014 at 04:45:03PM -0700, Ian Lance Taylor wrote: On Tue, Sep 9, 2014 at 6:02 AM, Dominik Vogt v...@linux.vnet.ibm.com wrote: +case 8: + return (is_unsigned) ? uint8 : int8; No need to parenthesize is_unsigned here and in the following lines. That's just my way of coding. I always put conditions into parentheses, but I don't care whether they are removed here. +#if 1 /*!!!todo: identifier may not be unique???*/ +#endif I'm not sure what the #if 1 is about. Just put a FIXME comment--see other examples in the code base. That's an accident. + static unsigned int art_i = 0; + if (is_nested == false) +art_i = 0; Don't use a static variable. Instead, pass down an int*. It can be NULL for the case where !is_nested. Done. (Also, in code like this, write !is_nested rather than is_nested == false). Okay. + layout_type (type); Is this really necessary? It seems so. Without it, -fdump-go-specs triggers a core dump when processing the s390x headers. A file like this generates the core: typedef struct S T; + decl_align_unit = precision_to_units (DECL_ALIGN (field)); You can just use DECL_ALIGN_UNIT. + type_align_unit = precision_to_units (TYPE_ALIGN (type)); You can just use TYPE_ALIGN_UNIT. Okay. + snprintf (buf, sizeof buf, %u, sz_units); This doesn't look right--shouldn't it be sz_units / TYPE_ALIGN_UNITS (type) ? Using sz_units only looks right if go_get_uinttype_for_precision returns byte. The generated size is correct because sz_units is always in bytes, and the code generates a *byte* array. However, the alignment may be wrong. Given a C union union { uint8_t c; uint64_t ll; } u; with size 8 and alignment 8, the generated Go struct is _u struct { c [8]byte; } The alignment of this struct is 1 and the size is 8. I've added an alignment field at the end of the struct like this: _u struct { c [8]byte; Godump_n_align [0]uint64; } and added more tests. + ( !htab_find_slot (container-type_hash, IDENTIFIER_POINTER (id), Don't use these leading spaces after parens, just let emacs format your code even if looks awkward. Okay. -- The attached patch contains all the discussed changes. Ciao Dominik ^_^ ^_^ -- Dominik Vogt IBM Germany From 5e1556d67febf91e066953a7c795f7855b8f841e Mon Sep 17 00:00:00 2001 From: Dominik Vogt v...@linux.vnet.ibm.com Date: Fri, 5 Sep 2014 07:30:56 +0100 Subject: [PATCH] godump: Support bitfields and unions in go_format_type. 1) Fix handling of arrays with zero elements. 2) Support bitfields and unions in go_format_type. Bitfields are replaced by byte arrays with synthetic names that cover the space occupied by the bitfield and any padding after it. Unions are represented by a struct with a single field that has the name of the first field of the original record (or Godump_n if the first field has no name). This field is a byte array with the size of the largest field of the original union. If alignment of the union does not match the size of the byte array, an array of integers with the alignment of the union and zero elements is added at the end of the record to enforce proper alignment (name Godump_n_align). Placement of the fields of a record is derived using the internal placement algorithm in stor.layout.c. In some cases byte arrays of proper size are inserted into records to get the alignment right. Also, if the representation of a record in Go does not have the proper alignment after all fields have been converted to Go, an array of integers with the alignment of the recordn and zero elements is added at the end of the record (name Godump_n_align). When translating a VAR_DECL, a type that is aliased with typedef (or a struct or union) is used literally, if possible, and not resolved to builtin types. Packed records are (still) not supported. 3) Add godump testsuite. --- gcc/godump.c| 361 +--- gcc/testsuite/gcc.misc-tests/godump-1.c | 482 gcc/testsuite/gcc.misc-tests/godump.exp | 36 +++ 3 files changed, 785 insertions(+), 94 deletions(-) create mode 100644 gcc/testsuite/gcc.misc-tests/godump-1.c create mode 100644 gcc/testsuite/gcc.misc-tests/godump.exp diff --git a/gcc/godump.c b/gcc/godump.c index 7566f4d..a124291 100644 --- a/gcc/godump.c +++ b/gcc/godump.c @@ -37,6 +37,8 @@ along with GCC; see the file COPYING3. If not see #include obstack.h #include debug.h #include wide-int-print.h +#include stor-layout.h +#include defaults.h /* We dump this information from the debug hooks. This gives us a stable and maintainable API to hook into. In order to work @@ -73,6 +75,14 @@ struct macro_hash_value char *value; }; +/* Returns the number of units necessary to represent an integer with the given + PRECISION (in bits). */ +static inline unsigned int +precision_to_units (unsigned int precision) +{ + return
Re: [gofrontend-dev] [PATCH 7/9] Gccgo port to s390[x] -- part I
On Tue, Oct 28, 2014 at 7:31 AM, Dominik Vogt v...@linux.vnet.ibm.com wrote: The attached patch contains all the discussed changes. I made a few formatting changes. I patched the test to work on x86, by making the char types accept either int8 or uint8, and making the long double tests accept any floating point size. Approved and applied as attached. Thanks. Ian gcc/: 2014-10-28 Dominik Vogt v...@linux.vnet.ibm.com * godump.c (precision_to_units): New helper function. (go_append_artificial_name): Ditto. (go_append_decl_name): Ditto. (go_append_bitfield): Ditto. (go_get_uinttype_for_precision): Ditto. (go_append_padding): Ditto. (go_force_record_alignment): Ditto. (go_format_type): Represent unions with an array of uints of the size of the alignment in go. This fixes the 'random' size of the union's representation using just the first field. (go_format_type): Add argument that indicates whether a record is nested (used for generation of artificial go names). (go_output_fndecl): Adapt to new go_format_type signature. (go_output_typedef): Ditto. (go_output_var): Ditto. (go_output_var): Prefer to output type as alias (typedef). (go_format_type): Bitfields in records are simulated as arrays of bytes in go. * godump.c (go_format_type): Fix handling of arrays with zero elements. gcc/testsuite/: 2014-10-28 Dominik Vogt v...@linux.vnet.ibm.com * gcc.misc-tests/godump.exp: New. * gcc.misc-tests/godump-1.c: New. Index: godump.c === --- godump.c(revision 216766) +++ godump.c(working copy) @@ -37,6 +37,8 @@ along with GCC; see the file COPYING3. #include obstack.h #include debug.h #include wide-int-print.h +#include stor-layout.h +#include defaults.h /* We dump this information from the debug hooks. This gives us a stable and maintainable API to hook into. In order to work @@ -73,6 +75,15 @@ struct macro_hash_value char *value; }; +/* Returns the number of units necessary to represent an integer with the given + PRECISION (in bits). */ + +static inline unsigned int +precision_to_units (unsigned int precision) +{ + return (precision + BITS_PER_UNIT - 1) / BITS_PER_UNIT; +} + /* Calculate the hash value for an entry in the macro hash table. */ static hashval_t @@ -552,19 +563,132 @@ go_append_string (struct obstack *ob, tr obstack_grow (ob, IDENTIFIER_POINTER (id), IDENTIFIER_LENGTH (id)); } +/* Given an integer PRECISION in bits, returns a constant string that is the + matching go int or uint type (depending on the IS_UNSIGNED flag). Returns a + NULL pointer if there is no matching go type. */ + +static const char * +go_get_uinttype_for_precision (unsigned int precision, bool is_unsigned) +{ + switch (precision) +{ +case 8: + return is_unsigned ? uint8 : int8; +case 16: + return is_unsigned ? uint16 : int16; +case 32: + return is_unsigned ? uint32 : int32; +case 64: + return is_unsigned ? uint64 : int64; +default: + return NULL; +} +} + +/* Append an artificial variable name with the suffix _INDEX to OB. Returns + INDEX + 1. */ + +static unsigned int +go_append_artificial_name (struct obstack *ob, unsigned int index) +{ + char buf[100]; + + /* FIXME: identifier may not be unique. */ + obstack_grow (ob, Godump_, 7); + snprintf (buf, sizeof buf, %u, index); + obstack_grow (ob, buf, strlen (buf)); + + return index + 1; +} + +/* Append the variable name from DECL to OB. If the name is in the + KEYWORD_HASH, prepend an '_'. */ + +static void +go_append_decl_name (struct obstack *ob, tree decl, htab_t keyword_hash) +{ + const char *var_name; + void **slot; + + /* Start variable name with an underscore if a keyword. */ + var_name = IDENTIFIER_POINTER (DECL_NAME (decl)); + slot = htab_find_slot (keyword_hash, var_name, NO_INSERT); + if (slot != NULL) +obstack_1grow (ob, '_'); + go_append_string (ob, DECL_NAME (decl)); +} + +/* Appends a byte array with the necessary number of elements and the name + Godump_INDEX_pad to pad from FROM_OFFSET to TO_OFFSET to OB assuming that + the next field is automatically aligned to ALIGN_UNITS. Returns INDEX + 1, + or INDEX if no padding had to be appended. The resulting offset where the + next field is allocated is returned through RET_OFFSET. */ + +static unsigned int +go_append_padding (struct obstack *ob, unsigned int from_offset, + unsigned int to_offset, unsigned int align_units, + unsigned int index, unsigned int *ret_offset) +{ + if (from_offset % align_units 0) +from_offset += align_units - (from_offset % align_units); + gcc_assert (to_offset = from_offset); + if (to_offset from_offset) +{ + char buf[100]; + + index = go_append_artificial_name
Re: [gofrontend-dev] [PATCH 7/9] Gccgo port to s390[x] -- part I
On Tue, Sep 9, 2014 at 6:02 AM, Dominik Vogt v...@linux.vnet.ibm.com wrote: This patch extends the -fdump-go-spec option to handle bitfields and unions and fixes handlinx of zero length arrays. All of this is necessary for s390[x] since the system headers use these features. Please check the commit comment for a detailed description of the patch. gcc/ChangeLog - 2014-09-05 Dominik Vogt v...@linux.vnet.ibm.com * godump.c (precision_to_units): New helper function. (go_append_artificial_name): Ditto. (go_append_decl_name): Ditto. (go_append_bitfield): Ditto. (go_get_uinttype_for_precision): Ditto. (go_append_padding): Ditto. (go_force_record_alignment): Ditto. (go_format_type): Represent unions with an array of uints of the size of the alignment in go. This fixes the 'random' size of the union's representation using just the first field. (go_format_type): Add argument that indicates whether a record is nested (used for generation of artificial go names). (go_output_fndecl): Adapt to new go_format_type signature. (go_output_typedef): Ditto. (go_output_var): Ditto. (go_output_var): Prefer to output type as alias (typedef). (go_format_type): Bitfields in records are simulated as arrays of bytes in go. 2014-09-05 Dominik Vogt v...@linux.vnet.ibm.com * godump.c (go_format_type): Fix handling of arrays with zero elements. gcc/testsuite/ChangeLog --- 2014-09-05 Dominik Vogt v...@linux.vnet.ibm.com * gcc.misc-tests/godump-1.c: Add tests for bitfields and unions. 2014-09-05 Dominik Vogt v...@linux.vnet.ibm.com * gcc.misc-tests/godump.exp: New. * gcc.misc-tests/godump-1.c: New. +case 8: + return (is_unsigned) ? uint8 : int8; No need to parenthesize is_unsigned here and in the following lines. +#if 1 /*!!!todo: identifier may not be unique???*/ +#endif I'm not sure what the #if 1 is about. Just put a FIXME comment--see other examples in the code base. + static unsigned int art_i = 0; + if (is_nested == false) +art_i = 0; Don't use a static variable. Instead, pass down an int*. It can be NULL for the case where !is_nested. (Also, in code like this, write !is_nested rather than is_nested == false). + layout_type (type); Is this really necessary? At this point the types should be laid out already. + decl_align_unit = precision_to_units (DECL_ALIGN (field)); You can just use DECL_ALIGN_UNIT. + type_align_unit = precision_to_units (TYPE_ALIGN (type)); You can just use TYPE_ALIGN_UNIT. + snprintf (buf, sizeof buf, %u, sz_units); This doesn't look right--shouldn't it be sz_units / TYPE_ALIGN_UNITS (type) ? Using sz_units only looks right if go_get_uinttype_for_precision returns byte. + ( !htab_find_slot (container-type_hash, IDENTIFIER_POINTER (id), Don't use these leading spaces after parens, just let emacs format your code even if looks awkward. Thanks. Ian
[PATCH 7/9] Gccgo port to s390[x] -- part I
This patch extends the -fdump-go-spec option to handle bitfields and unions and fixes handlinx of zero length arrays. All of this is necessary for s390[x] since the system headers use these features. Please check the commit comment for a detailed description of the patch. gcc/ChangeLog - 2014-09-05 Dominik Vogt v...@linux.vnet.ibm.com * godump.c (precision_to_units): New helper function. (go_append_artificial_name): Ditto. (go_append_decl_name): Ditto. (go_append_bitfield): Ditto. (go_get_uinttype_for_precision): Ditto. (go_append_padding): Ditto. (go_force_record_alignment): Ditto. (go_format_type): Represent unions with an array of uints of the size of the alignment in go. This fixes the 'random' size of the union's representation using just the first field. (go_format_type): Add argument that indicates whether a record is nested (used for generation of artificial go names). (go_output_fndecl): Adapt to new go_format_type signature. (go_output_typedef): Ditto. (go_output_var): Ditto. (go_output_var): Prefer to output type as alias (typedef). (go_format_type): Bitfields in records are simulated as arrays of bytes in go. 2014-09-05 Dominik Vogt v...@linux.vnet.ibm.com * godump.c (go_format_type): Fix handling of arrays with zero elements. gcc/testsuite/ChangeLog --- 2014-09-05 Dominik Vogt v...@linux.vnet.ibm.com * gcc.misc-tests/godump-1.c: Add tests for bitfields and unions. 2014-09-05 Dominik Vogt v...@linux.vnet.ibm.com * gcc.misc-tests/godump.exp: New. * gcc.misc-tests/godump-1.c: New. Ciao Dominik ^_^ ^_^ -- Dominik Vogt IBM Germany From 01b400c36daba0ef3f96fd9d98d9becbc5b15080 Mon Sep 17 00:00:00 2001 From: Dominik Vogt v...@linux.vnet.ibm.com Date: Fri, 5 Sep 2014 07:30:56 +0100 Subject: [PATCH 7/9] godump: Support bitfields and unions in go_format_type. 1) Fix handling of arrays with zero elements. 2) Support bitfields and unions in go_format_type. Bitfields are replaced by byte arrays with synthetic names that cover the space occupied by the bitfield and any padding after it. Unions are represented by a struct with a single field that has the name of the first field of the original record (or Godump_n if the first field has no name). This field is a byte array with the size of the largest field of the original union. If alignment of the union does not match the size of the byte array, an array of integers with the alignment of the union and zero elements is added at the end of the record to enforce proper alignment (name Godump_n_align). Placement of the fields of a record is derived using the internal placement algorithm in stor.layout.c. In some cases byte arrays of proper size are inserted into records to get the alignment right. Also, if the representation of a record in Go does not have the proper alignment after all fields have been converted to Go, an array of integers with the alignment of the recordn and zero elements is added at the end of the record (name Godump_n_align). When translating a VAR_DECL, a type that is aliased with typedef (or a struct or union) is used literally, if possible, and not resolved to builtin types. Packed records are (still) not supported. 3) Add godump testsuite. --- gcc/godump.c| 356 +--- gcc/testsuite/gcc.misc-tests/godump-1.c | 472 gcc/testsuite/gcc.misc-tests/godump.exp | 36 +++ 3 files changed, 770 insertions(+), 94 deletions(-) create mode 100644 gcc/testsuite/gcc.misc-tests/godump-1.c create mode 100644 gcc/testsuite/gcc.misc-tests/godump.exp diff --git a/gcc/godump.c b/gcc/godump.c index 7566f4d..4f3e69d 100644 --- a/gcc/godump.c +++ b/gcc/godump.c @@ -37,6 +37,8 @@ along with GCC; see the file COPYING3. If not see #include obstack.h #include debug.h #include wide-int-print.h +#include stor-layout.h +#include defaults.h /* We dump this information from the debug hooks. This gives us a stable and maintainable API to hook into. In order to work @@ -73,6 +75,14 @@ struct macro_hash_value char *value; }; +/* Returns the number of units necessary to represent an integer with the given + PRECISION (in bits). */ +static inline unsigned int +precision_to_units (unsigned int precision) +{ + return (precision + BITS_PER_UNIT - 1) / BITS_PER_UNIT; +} + /* Calculate the hash value for an entry in the macro hash table. */ static hashval_t @@ -552,19 +562,129 @@ go_append_string (struct obstack *ob, tree id) obstack_grow (ob, IDENTIFIER_POINTER (id), IDENTIFIER_LENGTH (id)); } +/* Given an integer PRECISION in bits, returns a constant string that is the + matching go int or uint type (depending on the IS_UNSIGNED flag). Returns a + NULL pointer if there is no matching go type. */ +static const