Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement ABI stuff for -preview=in #3578

Merged
merged 1 commit into from
Jan 8, 2021
Merged

Conversation

kinke
Copy link
Member

@kinke kinke commented Oct 4, 2020

I.e., the ABI-specific decision for which parameter types to prefer passing a ref over a value.

The x86_64 and AArch64 ABIs are pretty clear - use a ref if the POD type cannot be passed in registers. For all others, use a simple default heuristic based on the type size - if larger than 2 machine words, use a ref. Note that this includes x87 real for the 32-bit x86 ABI.

@JohanEngelen
Copy link
Member

JohanEngelen commented Oct 4, 2020

I am very worried about the semantic meaning of -preview=in being unclear. If this PR only implements the ABI part, and does not change the semantic meaning of code, it's all good with me; but I'm worried the frontend implementation is incorrect and does not provide the correct guarantees when passing by ref (i.e. creating a temporary local copy if parameters alias).

My worry is that with this PR, we combine the decision that we must pass by reference to not violate D language semantics, with the decision that we may pass by ref as an optimization (without changing language semantics).

Edit: looking a bit closer at the PR, I see that my fear is not justified (all was behind preview=in before aswell).

@kinke
Copy link
Member Author

kinke commented Oct 4, 2020

Yeah, this just implements the ABI-specific part for LDC (well, at least the 3 major ABIs, but the 32-bit x86 ABI is IMO okay too this way). The upstream ABI part for DMD needs work - I've promised to implement it, so the frontend changes in here will probably be upstreamed more or less as-is.

@kinke
Copy link
Member Author

kinke commented Oct 4, 2020

i.e. creating a temporary local copy if parameters alias

This is hard to prove (could be aliased via global state as well), and explicit in + -preview=in shifts the burden to the user.

@thewilsonator
Copy link
Contributor

The upstream ABI part for DMD needs work - I've promised to implement it, so the frontend changes in here will probably be upstreamed more or less as-is.

Ping me when you make that PR.

kinke added a commit to kinke/dmd that referenced this pull request Oct 7, 2020
This is the promised follow-up on dlang#11000 and makes DMD exploit the
specifics of the few supported ABIs (Win64, SysV x86_64, 32-bit x86).

It also almost perfectly matches the proposed LDC implementation in
ldc-developers/ldc#3578 (just a minor divergence for Win64 and dynamic
arrays, but in that point the LDC and DMD Win64 ABI diverges in
general).
kinke added a commit to kinke/dmd that referenced this pull request Oct 7, 2020
This is the promised follow-up on dlang#11000 and makes DMD exploit the
specifics of the few supported ABIs (Win64, SysV x86_64, 32-bit x86).

It also almost perfectly matches the proposed LDC implementation in
ldc-developers/ldc#3578 (just a minor divergence for Win64 and dynamic
arrays, but in that point the LDC and DMD Win64 ABI diverges in
general).
kinke added a commit to kinke/dmd that referenced this pull request Oct 31, 2020
This is the promised follow-up on dlang#11000 and makes DMD exploit the
specifics of the few supported ABIs (Win64, SysV x86_64, 32-bit x86).

It also almost perfectly matches the proposed LDC implementation in
ldc-developers/ldc#3578 (just a minor divergence for Win64 and dynamic
arrays, but in that point the LDC and DMD Win64 ABI diverges in
general).
kinke added a commit to kinke/ldc that referenced this pull request Nov 7, 2020
I expect this to be slightly more performant than the previous behavior,
where a delegate was treated like a corresponding struct, passed via
hidden pointer and returned via sret.

The primary motivation is a smooth preparation for PR ldc-developers#3578 - in order
to allow people to experiment with `-preview=in` without recompiling
druntime and Phobos, `in` slices and delegates must not be passed by-ref
with `-preview=in` (see dlang/dmd#11828). This would have required a
special case for delegates on Win64, which is IMO better handled this
way.
kinke added a commit to kinke/ldc that referenced this pull request Nov 7, 2020
I expect this to be slightly more performant than the previous behavior,
where a delegate was treated like a corresponding struct, passed via
hidden pointer and returned via sret.

The primary motivation is a smooth preparation for PR ldc-developers#3578 - in order
to allow people to experiment with `-preview=in` without recompiling
druntime and Phobos, `in` slices and delegates must not be passed by-ref
with `-preview=in` (see dlang/dmd#11828). This would have required a
special case for delegates on Win64, which is IMO better handled this
way.
AsterMiha pushed a commit to AsterMiha/dmd that referenced this pull request Nov 9, 2020
This is the promised follow-up on dlang#11000 and makes DMD exploit the
specifics of the few supported ABIs (Win64, SysV x86_64, 32-bit x86).

It also almost perfectly matches the proposed LDC implementation in
ldc-developers/ldc#3578 (just a minor divergence for Win64 and dynamic
arrays, but in that point the LDC and DMD Win64 ABI diverges in
general).
kinke added a commit that referenced this pull request Nov 11, 2020
…ers (#3609)

I expect this to be slightly more performant than the previous behavior,
where a delegate was treated like a corresponding struct, passed via
hidden pointer and returned via sret.

The primary motivation is a smooth preparation for PR #3578 - in order
to allow people to experiment with `-preview=in` without recompiling
druntime and Phobos, `in` slices and delegates must not be passed by-ref
with `-preview=in` (see dlang/dmd#11828). This would have required a
special case for delegates on Win64, which is IMO better handled this
way.
kinke added a commit to ldc-developers/dmd-testsuite that referenced this pull request Nov 26, 2020
This is the promised follow-up on #11000 and makes DMD exploit the
specifics of the few supported ABIs (Win64, SysV x86_64, 32-bit x86).

It also almost perfectly matches the proposed LDC implementation in
ldc-developers/ldc#3578 (just a minor divergence for Win64 and dynamic
arrays, but in that point the LDC and DMD Win64 ABI diverges in
general).
@kinke kinke force-pushed the preview_in branch 2 times, most recently from a83a362 to 153c5f6 Compare January 8, 2021 14:06
I.e., the ABI-specific decision for which parameter types to prefer
passing a ref over a value.

The x86_64 and AArch64 ABIs are pretty clear - use a ref if the POD type
cannot be passed in registers. For all others, use a simple default
heuristic based on the type size - if larger than 2 machine words, use a
ref. Note that this includes x87 `real` for the 32-bit x86 ABI.
@kinke kinke merged commit bdc317e into ldc-developers:master Jan 8, 2021
@kinke kinke deleted the preview_in branch January 8, 2021 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants