Posix x86_64 Calling conventions fixes #9013

SSoulaimane · 2018-11-25T20:42:02Z

Make DMD adhere better to the platform native calling conventions on x86_64 posix targets

dlang-bot · 2018-11-25T20:42:03Z

Thanks for your pull request and interest in making D better, @SSoulaimane! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
I have provided a detailed rationale explaining my changes
New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + dmd#9013"

SSoulaimane · 2018-11-25T20:47:29Z

All test cases not related to this PR pass. Two tests fail testargtypes.d and testabi.d which I'm working on now. I did hand checks though. This isn't ready for merge yet but it's ready for a review.

README.md

src/dmd/backend/ty.d

src/dmd/e2ir.d

src/dmd/globals.d

thewilsonator · 2018-11-26T02:09:55Z

Thanks! This will need to be squashed significantly before merge, but for now if you could squash the typo and minor mixup commits that would be great.

jacob-carlborg · 2018-11-26T12:52:28Z

What exactly is this doing? Changing DMD to not use its own calling conventions?

thewilsonator · 2018-11-26T13:15:04Z

this is for extern(C[++]), the current posix x64 ABI is broken, this fixes it.

e: Incidentally this is a requisite for mapping T[] to C++.

src/dmd/globals.d

src/dmd/target.d

SSoulaimane · 2018-11-27T00:27:14Z

Thanks! This will need to be squashed significantly before merge, but for now if you could squash the typo and minor mixup commits that would be great.

Brilliant idea. I'm going trough this now It feels like doing surgery or writing a resume but it's a good thing to do.

thewilsonator · 2018-11-27T02:14:42Z

This will also need to enumerate the bugs that it fixes in one of the commit messages in order to close them, take a look at some the other bug fix PRs for that.

src/dmd/argtypes.d

src/dmd/argtypes_sysv_x64.d

jacob-carlborg · 2018-11-27T10:43:26Z

All new public symbols should have Ddoc comments.

SSoulaimane · 2018-11-27T17:23:13Z

Thank you @jacob-carlborg and @n8sh for you reviews about the new toArgTypes code. But I just pulled that code from this PR #8837 by @kinke whatever he decides there I'll just follow. these reviews should go to that PR. I don't know how to make this pull request dependent on that one here on github (if that's even possible?).

SSoulaimane · 2018-11-27T17:28:36Z

This will also need to enumerate the bugs that it fixes in one of the commit messages in order to close them, take a look at some the other bug fix PRs for that.

Sure. I'll do that right after setting up some test cases first.

SSoulaimane · 2018-11-27T18:14:21Z

Individual commits do make sense now after some reordering and squashing. The more significant commits are the older ones while the recent ones are mostly small fixes (that still make sense).

WalterBright · 2018-11-28T08:48:45Z

This is extremely difficult to review because it does way too much. It should be done incementally in smaller steps.

thewilsonator · 2018-11-28T10:00:17Z

This is extremely difficult to review because it does way too much.

@WalterBright This is going to get reviewed by me irrespectively. I would recommend review-by-commit here.

It should be done incementally in smaller steps.

While nice in principle, @SSoulaimane focus on getting all the tests to pass first. I'll deal with the fission of this for merge and review when the time comes. The restructuring you did of those commits is a good start. Thanks again for fixing these nasty bugs!

kinke · 2018-11-29T21:14:25Z

Two tests fail testargtypes.d and testabi.d

Fixing testargtypes.d is trivial, just get rid of all version (DigitalMars) blocks. These are cases where the old toArgTypes() for 64-bit targets diverges from the new toArgTypes_sysv_x64() one.

… which is a SIMD vector

Other parts (mainly isReturnOnStack) don't yet match the windows 64 ABI.

allow a valid case where stackused == 1 the fpu stack may be preloaded in case the function call is the second operand to some binary operator example: `5 + fun()` ST(0) would be holding the constant 5 prior to executing funccall()

runnable/complex.d:test7581() breaks on assert(stackused == 0); This caused return values int ST0 not to be popped off the fpu stack when only the flags are needed (ex: comparison to zero).

…rray. Array operations will require moving the SIMD vector out of the SIMD register first.

DMD complains about global variable access from asm code

…argets" Just for CI's sake This reverts commit b4297b2.

When an if statement gets inlined into a comma expression and none of the if or else branches have a return statement then one of them may get removed by the optimizer which causes a type loss on the else branch. Q: Why does this matter? A: If the type is lost then the wrong registers will be allocated

…h mixed registers Use two new type flags for SROA `mTYxmmgpr` and `mTYgprxmm` Since I figured I don't really need the full type of the original aggregate preserved through optimization, but knowing which combination of registers to use is enough.

This was causing an ice in the backend

SSoulaimane · 2019-07-20T05:18:31Z

Rebooted at #10200.

12345swordy · 2019-07-20T14:38:35Z

Why is this pr still open then?

SSoulaimane requested review from ibuclaw, MartinNowak and wilzbach as code owners November 25, 2018 20:42

thewilsonator reviewed Nov 26, 2018

View reviewed changes

README.md Outdated Show resolved Hide resolved

thewilsonator reviewed Nov 26, 2018

View reviewed changes

src/dmd/backend/ty.d Outdated Show resolved Hide resolved

thewilsonator reviewed Nov 26, 2018

View reviewed changes

src/dmd/e2ir.d Outdated Show resolved Hide resolved

thewilsonator reviewed Nov 26, 2018

View reviewed changes

src/dmd/globals.d Outdated Show resolved Hide resolved

ibuclaw reviewed Nov 26, 2018

View reviewed changes

src/dmd/globals.d Outdated Show resolved Hide resolved

ibuclaw reviewed Nov 26, 2018

View reviewed changes

src/dmd/target.d Outdated Show resolved Hide resolved

n8sh reviewed Nov 27, 2018

View reviewed changes

src/dmd/argtypes.d Outdated Show resolved Hide resolved