[Mono-devel-list] More 64 bit changes

[Bernie Solomon]

Thanks for the feedback. Yes my stack size changes were wrong. And
I misunderstood __alignof__ - I thought it would give mininum aligment
not recommended alignment. However it seems to me that the type_size
changes are right as they are used to layout native structs and
those will use __alignof__ alignment for fields. Indeed I think (though may
be wrong and have misunderstood things) that alignment is wrong in
the current code for doubles in structs for pinvoke even on X86 - my
testing seems to indicate doubles are aligned on 8 byte boundaries in
structs on Windows with default packing - though stack arguments
are on 4 byte boundaries.

Given my (yet to be released) HP trampoline code doesn't use
the stack size routine at all I don't think it needs changing for this
after all.

I've also been improving the tests to remove some assumptions about
sizes, how pinvoke works (HP does not return a 4 byte struct
the same way as a 4 byte int), endian-ness and so on. libtest.c also now
depends on glib because of g_malloc so I've changed Makefile.am
for that so glib.h can be included so the return type of g_malloc
is known to be a pointer. I've changed some of the returned values
in a couple of cases to help distinguish failures (just helps debugging).

I have added a struct with a double and long in it to
check pinvoke layout really does work in these cases (this
is in pinvoke2.cs).

diffs for tests attached.

If I'm right then I think the attached marshal.c
diffs are nearer being correct (note there is another fix for
bool conversion as well).

I think that's it for changes - I'm actually on holiday for
a bit now so no more messages from me for a bit!

Bernie Solomon

----- Original Message ----- 
From: "Paolo Molaro" <lupus at ximian.com>
To: <mono-devel-list at lists.ximian.com>
Sent: Thursday, July 03, 2003 11:41 AM
Subject: Re: [Mono-devel-list] More 64 bit changes


> On 07/02/03 Bernie Solomon wrote:
> > The rounding up to just 4 byte boundaries in mono_type_native_stack_size
for
> > value types doesn't look general enough to me (as commented) though I
> > haven't fallen over it as yet.
>
> Yes, that code is broken, but your patch is not correct, because it
> could cause int64 and doubles to be aligned on 8 byte boundaries,
> which is incorrect on x86. so you need something like:
>
> #ifdef __x86__ /* add other archs as appropriate */
> #define DOUBLE_ALIGNMENT 4
> #define INT64_ALIGNMENT 4
> #else
> #define DOUBLE_ALIGNMENT 8
> #define INT64_ALIGNMENT 8
> #endif
>
> And then use INT64_ALIGNMENT or DOUBLE_ALIGNMENT in the function.
> The rounding up needs to be done using sizeof (gpointer) as you point
> out: since I guess that's what most of the platforms do.
> I didn't apply the marshal.c chunks of your patch.
>
> > mono_runtime_class_init checks for "initializing" and "initialized" but
if
> > it returns early it would leave the domain locked. I have unlocked it
but I
>
> Thanks for the fix.
>
> > actually think the second thread should actually wait for initialized to
> > become true shouldn't it?
>
> No: if initialized was set it can return, if it's not set but
> initializing is set it means it's a recursive call from the same thread
> that was already initalizing the type and we need to return to avoid a
> deadlock.
>
> > In mono_marshal_type_size I have changed the alignments of 8 byte values
so
> > they are naturally aligned on HP. Things crash without this. However is
this
> > right or should it be that code generates the same binary data on all
> > platforms which means more testing on whether unaligned access is
allowed in
> > code that manipulates data using the results of this routine.
>
> Well, there are interesting issues. For the normal code (and once the
> above issues with sizeof(gpointer) and alignement are fixed) there
> should be no issue, because the runtime should lay out all the fields as
> necessary for the platform. In some cases, though, the marshal code
> should insert unaligned. opcode prefixes so that the execution engine
> uses the appropriate code that doesn't trigger the unaligned fault
> handlers. I'm afraid, though, that that neither mint nor the jit use
> the unaligned prefix, yet, but as I said, if the rest of the code is
> fixed, you should not hit this case normally.
>
> lupus
>
> -- 
> -----------------------------------------------------------------
> lupus at debian.org                                     debian/rules
> lupus at ximian.com                             Monkeys do it better
> _______________________________________________
> Mono-devel-list mailing list
> Mono-devel-list at lists.ximian.com
> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: tests.diff
Type: application/octet-stream
Size: 11509 bytes
Desc: not available
Url : http://lists.ximian.com/pipermail/mono-devel-list/attachments/20030703/23bc74c8/attachment.obj 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: marshal.diffs
Type: application/octet-stream
Size: 2045 bytes
Desc: not available
Url : http://lists.ximian.com/pipermail/mono-devel-list/attachments/20030703/23bc74c8/attachment-0001.obj