Fix x86_64 calling convention for functions with D linkage

[ljmf00]

Signed-off-by: Luís Ferreira contact@lsferreira.net

---

Related to dlang/dmd#13287 .

[dlang-bot]

Thanks for your pull request, @ljmf00!

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 run digger -- build "master + druntime#3779"

[ljmf00]

@RazvanN7 how can I fix this interdependency? There is probably a lot to fix tho, but one idea would be to build some manual runtime checks on the args registers to check correct calling convention and when the compiler PR is merged, remove them?

[RazvanN7]

@ljmf00 This is problematic. It seems that we need to merge these both at once. But this gives me some chills since we cannot test this properly using our CI infrastructure. I think that the path of least resistance would be to merge this one and then make sure that the dmd pr passes all of the tests. My reasoning is that the dmd pr has a bigger area of changes and therefore should be tested properly before merging. However, this means that the pipeline will be broken for quite some time.

Offtopic: Fingers crossed that maybe we will get the monorepo project going cc @PetarKirov .

[RazvanN7]

Hmm maybe we can temporarily disable the tests that use atomicCompareExchangeStrongNoResult and atomicFetchAdd ? Are there many?

[ljmf00]

@ljmf00 This is problematic. It seems that we need to merge these both at once. But this gives me some chills since we cannot test this properly using our CI infrastructure. I think that the path of least resistance would be to merge this one and then make sure that the dmd pr passes all of the tests. My reasoning is that the dmd pr has a bigger area of changes and therefore should be tested properly before merging. However, this means that the pipeline will be broken for quite some time.

Offtopic: Fingers crossed that maybe we will get the monorepo project going cc @PetarKirov .

I can try to test this as much as possible locally, although testsuite fails on master on my machine for some specific tests. I have to check them and try to fix it to avoid problems. Another problem we should care is that intrinsics are relying on this calling convention thing, some of which, directly mapped to opcodes, e.g. bts from core.bitops. The signature of bts function doesn't match with the intrinsic, making it binary incompatible. We could have a module called __intrinsics and remap args there to avoid this. I think it is a really bad decision mapping intrinsics directly to end user calls.

[dkorpel]

How about introducing a new version condition, say, version(D_reversedParametersAbi)? That also allows user code to transition and keep compatibility with multiple dmd versions.

[kinke]

how can I fix this interdependency?

Create same-named branches on the upstream repos. E.g., fix-call-conv branches on the DMD/druntime/Phobos repos. (Most) CI systems check out these branches then; Buildkite still needs dlang/ci#452.

[ljmf00]

how can I fix this interdependency?

Create same-named branches on the upstream repos. E.g., fix-call-conv branches on the DMD/druntime/Phobos repos. (Most) CI systems check out these branches then; Buildkite still needs dlang/ci#452.

That's a cool feature.

[ljmf00]

How about introducing a new version condition, say, version(D_reversedParametersAbi)? That also allows user code to transition and keep compatibility with multiple dmd versions.

I would say, a version specific to DMD. It was never a thing on the specification but rather a vendor-specific issue (actually LDC does the same thing), so it is weird to specify that.

[ljmf00]

Ultimately, this atomic ops should be intrinsics, but I'm not willing to implement those on the DMD backend.

[Geod24]

@ljmf00 : Now you can re-submit against DMD without worrying about inter-dependency :)