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

Don't reverse parameters order for extern(D) #3873

Merged
merged 1 commit into from Mar 8, 2022

Conversation

kinke
Copy link
Member

@kinke kinke commented Nov 14, 2021

No description provided.

@kinke
Copy link
Member Author

kinke commented Jan 16, 2022

I must have missed some 32-bit x86 bits, some 32-bit Linux multilib tests fail. Edit: fixed.

On Linux/Mac x64, things are interesting - the entire testsuite passes, but a compiler compiled by itself fails to compile the runtime libs, complaining about forward references wrt. templates.

@kinke
Copy link
Member Author

kinke commented Jan 31, 2022

but a compiler compiled by itself fails to compile the runtime libs

Turns out the frontend itself depends on some peculiar opEquals order weirdness: https://issues.dlang.org/show_bug.cgi?id=22717


@p0nce: How badly would this breaking ABI change affect your intel-intrinsics project? Accessing parameters of extern(D) naked asm functions would need work, because the formal parameters aren't reversed anymore, see e.g. the Phobos diff.

@p0nce
Copy link
Contributor

p0nce commented Feb 1, 2022

@kinke well I don't use naked anywhere.
Still a tiny bit annoying that extern(D) changes but no interoperable protocol really relies on that fortunately.
I think it should be transparent and for intel-intrinsics nothing will break.

@kinke
Copy link
Member Author

kinke commented Feb 3, 2022

Thanks @p0nce. - @JohanEngelen: Any potential problems for Weka?

Still a tiny bit annoying that extern(D) changes

This 'just' brings the compiler in line with the spec (and LDC more compatible with GDC) - at least for non-32-bit-x86 targets (I don't reverse the params on 32-bit x86 anymore either, and now accordingly optionally pass the first param in a GP register, not the last one). I've wanted to rectify this for years and the topic keeps popping up every few months in the forum, so I don't really want to wait much longer (or even on DMD to follow suit).

@p0nce
Copy link
Contributor

p0nce commented Feb 3, 2022

void setFPUControlState(ushort dummy, ushort newState) nothrow @nogc
{
    asm nothrow @nogc
    {
        fclex;
        fldcw newState;
    }
}

Will that sort of inline assembly keep working?

@kinke
Copy link
Member Author

kinke commented Feb 4, 2022

Yes, it only affects code that assumed params to end up in particular registers or stack slots. No problem when regularly accessing by symbol.

@JohanEngelen
Copy link
Member

JohanEngelen commented Mar 1, 2022

Thanks @p0nce. - @JohanEngelen: Any potential problems for Weka?

I don't expect problems: I've converted all assembly to LLVM IR or intrinsics (AArch compatibility). Or it is an extern(C) function.

The build process seems to depend on libdparse, and that has assembly in it that appears to be affected:
https://github.com/dlang-community/libdparse/blob/b7729008d30a84c92f548293f38294eaa3eb46c0/src/dparse/lexer.d#L2216-L2228

@kinke
Copy link
Member Author

kinke commented Mar 1, 2022

Great Johan, thx for testing. Yes, libdparse would need some apparently pretty trivial adaptations (and the asm should then work on x86 in general, not just for Posix x64, and with GDC too).

@kinke
Copy link
Member Author

kinke commented Mar 8, 2022

I have tested this for Symmetry, no problems so far, testsuite is green.

@kinke kinke merged commit 0fd5c6e into ldc-developers:master Mar 8, 2022
@kinke kinke deleted the no_reverse branch March 8, 2022 16:03
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.

None yet

3 participants