The actual first failure of the build on ARM is not in the Rakudo setting
(which the previous message addresses), but on the very first thing that
NQP tries to build, using a bootstrap .mvm file.
ie, the built MoarVM is bust.
This turns out to be because P6Opaque was ignoring alignment constraints when
mapping struct members. On x86_64 Win32 and Linux (and other) this didn't
matter - all 3 types that P6Opaque deals with are all 64 bit.
(Or possibly 2 - is anything using a MVMnum64 attribute?)
On x86 Linux it also didn't matter, because the alignment of MVMint64 in
structures is only 4 bytes, so no padding is ever needed.
On ARM, MVMint64 needs to be 8 byte aligned in structures. Hence things blew
up quite spectacularly, very early.
With these 2 and the previous 2 patches, ARM almost builds.
Curiously, this seems to fix known problems on 32 bit Win32.
So I think that it's worth someone checking this on Windows (32 bit and 64
bit) and if it doesn't break anything that currently works, applying it.
Nicholas Clark
>From d57ceba5b4064846980b944943cea4d2f4b54bbc Mon Sep 17 00:00:00 2001
From: Nicholas Clark <n...@ccl4.org>
Date: Sun, 4 May 2014 14:45:41 +0200
Subject: [PATCH 3/5] Move the assignment to
repr_data->attribute_offsets[cur_slot] later in compose()
We need it later to deal better with alignment for platform portability.
Splitting it into a distinct commit makes it easier to show that this part
of the change introduces no regressions.
---
src/6model/reprs/P6opaque.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/6model/reprs/P6opaque.c b/src/6model/reprs/P6opaque.c
index bfff91a..bc7ffc6 100644
--- a/src/6model/reprs/P6opaque.c
+++ b/src/6model/reprs/P6opaque.c
@@ -689,8 +689,6 @@ static void compose(MVMThreadContext *tc, MVMSTable *st, MVMObject *info_hash) {
if (MVM_is_null(tc, name_obj))
MVM_exception_throw_adhoc(tc, "P6opaque: missing attribute name");
- /* Attribute will live at the current position in the object. */
- repr_data->attribute_offsets[cur_slot] = cur_alloc_addr;
if (REPR(name_obj)->ID == MVM_REPR_ID_MVMString) {
MVM_ASSIGN_REF(tc, &(st->header), name_map->names[i], (MVMString *)name_obj);
}
@@ -765,6 +763,9 @@ static void compose(MVMThreadContext *tc, MVMSTable *st, MVMObject *info_hash) {
}
}
+ /* Attribute will live at the current position in the object. */
+ repr_data->attribute_offsets[cur_slot] = cur_alloc_addr;
+
/* Handle object attributes, which need marking and may have auto-viv needs. */
if (!inlined) {
repr_data->gc_obj_mark_offsets[cur_obj_attr] = cur_alloc_addr;
--
1.8.4.2
>From b54d708ea1ee49ad307c9f0455466cd7ff04e630 Mon Sep 17 00:00:00 2001
From: Nicholas Clark <n...@ccl4.org>
Date: Sun, 4 May 2014 21:31:04 +0200
Subject: [PATCH 4/5] P6opaque must handle C structure member alignment, just
like CStruct does.
In particular, on some platforms MVMint64 has 8 byte alignment, which means
padding may well be required to correctly map to how the C compiler lays out
structs.
---
src/6model/reprs/P6opaque.c | 33 +++++++++++++++++++++++++++------
1 file changed, 27 insertions(+), 6 deletions(-)
diff --git a/src/6model/reprs/P6opaque.c b/src/6model/reprs/P6opaque.c
index bc7ffc6..64f08cc 100644
--- a/src/6model/reprs/P6opaque.c
+++ b/src/6model/reprs/P6opaque.c
@@ -576,7 +576,7 @@ static void compose(MVMThreadContext *tc, MVMSTable *st, MVMObject *info_hash) {
MVMint64 mro_pos, mro_count, num_parents, total_attrs, num_attrs,
cur_slot, cur_type, cur_alloc_addr, cur_obj_attr,
cur_init_slot, cur_mark_slot, cur_cleanup_slot, cur_unbox_slot,
- unboxed_type, bits, i;
+ unboxed_type, i;
MVMObject *info;
MVMStringConsts str_consts = tc->instance->str_consts;
@@ -684,6 +684,8 @@ static void compose(MVMThreadContext *tc, MVMSTable *st, MVMObject *info_hash) {
MVMint64 is_box_target = REPR(attr_info)->ass_funcs.exists_key(tc,
STABLE(attr_info), attr_info, OBJECT_BODY(attr_info), (MVMObject *)str_box_target);
MVMint8 inlined = 0;
+ MVMuint32 bits;
+ MVMuint32 align;
/* Ensure we have a name. */
if (MVM_is_null(tc, name_obj))
@@ -700,6 +702,7 @@ static void compose(MVMThreadContext *tc, MVMSTable *st, MVMObject *info_hash) {
/* Consider the type. */
unboxed_type = MVM_STORAGE_SPEC_BP_NONE;
bits = sizeof(MVMObject *) * 8;
+ align = ALIGNOF(void *);
if (!MVM_is_null(tc, type)) {
/* Get the storage spec of the type and see what it wants. */
MVMStorageSpec spec = REPR(type)->get_storage_spec(tc, STABLE(type));
@@ -707,6 +710,7 @@ static void compose(MVMThreadContext *tc, MVMSTable *st, MVMObject *info_hash) {
/* Yes, it's something we'll flatten. */
unboxed_type = spec.boxed_primitive;
bits = spec.bits;
+ align = spec.align;
MVM_ASSIGN_REF(tc, &(st->header), repr_data->flattened_stables[cur_slot], STABLE(type));
inlined = 1;
@@ -763,6 +767,13 @@ static void compose(MVMThreadContext *tc, MVMSTable *st, MVMObject *info_hash) {
}
}
+ /* C structure needs careful alignment. If cur_alloc_addr is not
+ * aligned to align bytes (cur_alloc_addr % align), make sure it is
+ * before we add the next element. */
+ if (cur_alloc_addr % align) {
+ cur_alloc_addr += align - cur_alloc_addr % align;
+ }
+
/* Attribute will live at the current position in the object. */
repr_data->attribute_offsets[cur_slot] = cur_alloc_addr;
@@ -826,15 +837,19 @@ static void deserialize_stable_size(MVMThreadContext *tc, MVMSTable *st, MVMSeri
/* To calculate size, we need number of attributes and to know about
* anything flattend in. */
MVMint64 num_attributes = reader->read_varint(tc, reader);
- MVMuint32 cur_offset = 0;
+ MVMuint32 cur_offset = sizeof(MVMP6opaque);
MVMint64 i;
for (i = 0; i < num_attributes; i++) {
if (reader->read_varint(tc, reader)) {
MVMSTable *st = reader->read_stable_ref(tc, reader);
MVMStorageSpec ss = st->REPR->get_storage_spec(tc, st);
- if (ss.inlineable)
+ if (ss.inlineable) {
/* TODO: Review if/when we get sub-byte things. */
+ if (cur_offset % ss.align) {
+ cur_offset += ss.align - cur_offset % ss.align;
+ }
cur_offset += ss.bits / 8;
+ }
else
cur_offset += sizeof(MVMObject *);
}
@@ -843,7 +858,7 @@ static void deserialize_stable_size(MVMThreadContext *tc, MVMSTable *st, MVMSeri
}
}
- st->size = sizeof(MVMP6opaque) + cur_offset;
+ st->size = cur_offset;
}
/* Serializes the REPR data. */
@@ -1017,7 +1032,7 @@ static void deserialize_repr_data(MVMThreadContext *tc, MVMSTable *st, MVMSerial
else {
/* Store position. */
MVMSTable *cur_st = repr_data->flattened_stables[i];
- repr_data->attribute_offsets[i] = cur_offset;
+ MVMStorageSpec spec = cur_st->REPR->get_storage_spec(tc, cur_st);
/* Set up flags for initialization and GC. */
if (cur_st->REPR->initialize)
@@ -1027,8 +1042,14 @@ static void deserialize_repr_data(MVMThreadContext *tc, MVMSTable *st, MVMSerial
if (cur_st->REPR->gc_cleanup)
repr_data->gc_cleanup_slots[cur_gc_cleanup_slot++] = i;
+ if (cur_offset % spec.align) {
+ cur_offset += spec.align - cur_offset % spec.align;
+ }
+
+ repr_data->attribute_offsets[i] = cur_offset;
+
/* Increment by size reported by representation. */
- cur_offset += cur_st->REPR->get_storage_spec(tc, cur_st).bits / 8;
+ cur_offset += spec.bits / 8;
}
}
repr_data->initialize_slots[cur_initialize_slot] = -1;
--
1.8.4.2
