Re: [RFC-v2] amd64-windows: Fix funcall with by-pointer arguments

Joel Brobecker
2013-09-17T13:50:32Z

> Thanks for doing that. I haven't completely finished reviewing
> the patch yet, but I wanted to send a message so that you and
> everyone else knows that I am on it. The patch looked good, minus
> some trivial formatting issues which I will fix, but I noticed
> a problem with function returning arrays. I am not sure where
> it is coming from, whether from me incorrectly applying your
> change, or missing something else, or whether it's a regression.
>
> I ran out of time for today, but will try to pursue this further
> tomorrow.

Small update for today. This is the change I had to apply on top of
your patch in order to fix the problem:

- else if (amd64_windows_passed_by_integer_register (type))
- regnum = AMD64_RAX_REGNUM;
+ else if (len == 1 || len == 2 || len == 4 || len == 8)
+ {
+ /* All values of this size are returned via RAX. */
+ regnum = AMD64_RAX_REGNUM;
+ }

It fixes the case where we return a small array of integers.
The documentation I have said, at the time:

Return values that can fit into 64-bits are returned through RAX
(including __m64 types), except for __m128, __m128i, __m128d, floats,
and doubles, which are returned in XMM0. [...] User defined types to
be returned must be 1, 2, 4, 8, 16, 32, or 64 bits in length.

This all made me realize that merging our two patches creates
a bit of confusion about what is a pure rewrite, and what is an
attempt to fix what.

So, here is what I will do: I will clean my patch up, retest,
also remove all amd64-windows-specific hooks as initially said,
and then send you an updated patch containing your changes, with
my fixes. If you could please:
1. Re-test, making sure that I did not break anything back;
2. Re-submit each piece for each problem that you're solving,
with description provided with each patch.

This will make the review process easier for me.

Thank you!
--
Joel