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: [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
