Reverse parameter order of xopCmp to match extern(C) ABI

[ibuclaw]

Required to support dlang/druntime#2025 (and tests will fail until it is pulled also).

[dlang-bot]

Thanks for your pull request, @ibuclaw!

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.

[ibuclaw]

Hmm that's interesting, all tests pass.

I'll have to dig up the original patch which reversed these arguments. Because something should have broke.

[ibuclaw]

IIRC it was the sort property, which may mean that some test was removed that validates this particular call works.

[ibuclaw]

Yep, test was removed (test 5854) in #6827.

[ibuclaw]

Re-added test which validates the call to compiler-generated __xopCmp().

Now it should fail unless the druntime PR is pulled - however will check this against gdc first.

[kinke]

So this is a rebase of #5232.

[ibuclaw]

@kinke - right, I missed that. I wrote this independently, but will have a look at yours.

[kinke]

It appears to be exactly the same minus a few comments of mine and plus your test. ;)

IIRC, this approach doesn't work for LDC on 32-bit x86. A method extern(D) int opCmp([ref T this,] ref T rhs) and a static function extern(C) int opCmp(ref T this, ref T rhs) aren't equivalent ABI-wise - the method's this would be passed in EAX, while the static function expects both pointers to be passed on the stack. See #5232 (comment).

The real problem is that a function pointer is used to invoke either an extern(D) method or a static function. That's why my last suggestion was using a front-end-generated static function in any case for the TypeInfo's function pointer, even if opCmp([ref] const T rhs) does take its argument by reference (=> trampoline).

[ibuclaw]

IIRC, this approach doesn't work for LDC on 32-bit x86.

Right, but you would have the same problem calling a nested function from a delegate, then using that same delegate to call a method also.

As I said in the other PR, I've chosen to ignore supporting thiscall for the moment until a satisfactory solution comes up.

[kinke]

As this isn't about thiscall but rather about the D ABI on Win32, DMD should be affected by this too and so shouldn't work with the druntime patch (at least on Win32, i.e., wherever extern(D) method and extern(C) function ABIs diverge).

[ibuclaw]

There is no special Windows ABI, unless you mean the non-standard optlink calling convention that DMD unfavourably implements, which would be better off being dropped altogether.

You shouldn't support that in LDC either.

[kinke]

Don't know if it's Optlink specific, but it's D's official ABI for Win32 (https://dlang.org/spec/abi.html#function_calling_conventions) and as such also implemented by LDC (for all 32-bit x86 platforms, most likely because that's what inline assembly in druntime/Phobos expects...).

Anyway, I just wanted to emphasize that the revived changes most likely still don't work for DMD on Win32 as long as that ABI isn't ditched, and that something like the enforced static function would be required in the meantime.

[ibuclaw]

Don't know if it's Optlink specific,

The similarities are too uncanny for it to be a coincidence.

https://www.ibm.com/support/knowledgecenter/SSQ2R2_9.5.0/com.ibm.etools.cbl.win.doc/topics/rpsubw30.htm

https://dlang.org/spec/abi.html#function_calling_conventions

[wilzbach]

(rebased)

[wilzbach]

(rebased)

[WalterBright]

The similarities are too uncanny for it to be a coincidence.

I've never heard of IBM's "optlink" convention, and it has nothing to do with our Optlink linker. A linker does not define a calling convention anyway.

The win32 calling convention goes back to Microsoft's C++ compiler on win32. I tried to be compatible with it.

[WalterBright]

Hmm that's interesting, all tests pass.

Probably because the test cases do not have side effects in the arguments, so reversing the order of non-existent side effects will have no effect.

[Geod24]

@ibuclaw : Do you want to pursue this now that GDC is up to date with master ?

[Geod24]

Ping @ibuclaw

[ibuclaw]

Ping @ibuclaw

Well I still need to patch dmd to reverse the arguments. I assume ldc is the same too.

[ibuclaw]

Superseded by #13590