Strive for an ABI-agnostic handling of TypeInfo_Struct.xopEquals and xopCmp

[kinke]

While trying to use a non-reversed parameter order for extern (D) in LDC on x86_64, I stumbled upon 2 ugly places assuming a reversed parameter order:

1. druntime's TypeInfo_Struct.compare() (see Render TypeInfo_Struct.xopCmp ABI-agnostic wrt. parameter order druntime#1417)
2. DMD front-end when generating the static __xopEquals/__xopCmp functions

The problem is that a pointer to an extern(D) function taking 2 struct references is used to jump either to:

• an extern(D) method int S.opCmp(ref const S q) with LL signature int opCmp(S* this = p, S* q), or
• an extern(D) static function static int S.__xopCmp(ref const S p, ref const S q) with LL signature int __xopCmp(S* q, S* p) if reversing the args. This function is generated by the front-end in case the opCmp method doesn't take its single argument by reference.

For this to work, TypeInfo_Struct.compare() assumed reversed order for extern(D) and called the extern(D) function pointer TypeInfo_Struct.xopCmp with reversed args (q, p), to get the direct method call right (=> LL order (p, q)).
In the DMD front-end, the static int S.__opCmp(ref const S p, ref const S q) wrapper function needed to take this into account. So it also assumed a reversed arguments order for extern(D) and re-swapped p and q back again (=> return q.opCmp(p)).

When not reversing the args for extern (D) (as it should be on all non-Win32 platforms, according to the docs!), there's no need for any swapping at all.
I opted for an extern (C) approach for function pointer and static function, as that is lowered to a LL signature identical to the method's, as long as the this pointer is passed before the first explicit arg, another (but not new) assumption. I'm not sure whether that's free of any undesired effects though, e.g., for exception handling; name mangling seems to be fine with LDC.

At least these 2 places don't have to assume anything about the parameter order of extern (D) anymore.

[ibuclaw]

I think this fixes a gdc regression where upstream did a p/q parameter swap (I undid the change).

[kinke]

@ibuclaw: Yes, exactly - #3670 fixing or rather working around bugzilla #5854.

[yebblies]

Can't we generate a non-static member function instead and avoid this mess?

[kinke]

@yebblies: That would be one part of a (cleaner) alternate solution. The second part still involves druntime's TypeInfo_Struct.compare() invoking one or the other D method (opCmp or your proposed __xopCmp method) via the TypeInfo_Struct.xopCmp D function pointer, which is still messy:

Hypothetical D method: int S.foo(ref S a, ref S b), LL signature when reversing the args: int foo(S* this, S* b, S* a)
=> D function pointer with identical LL signature: int foo(ref S a, ref S b, ref S this)
=> D function pointer when NOT reversing the args: int foo(ref S this, ref S a, ref S b)
So the this pointer is either the first or last arg, depending on whether the params are reversed for extern(D) or not.

[dnadlinger]

@kinke: ABIs might also pass the this pointer in a particular special way, right?

[kinke]

Possibly, we've just had a somewhat related issue in combination with struct-return (hidden sret pointer argument to the pre-allocated result). For the SysV ABI, sret comes before this, while MSVC++ always passes this before sret. In this case with a returned int, it luckily doesn't matter, but yeah, invoking a method via a function pointer really is a hack.

[kinke]

ABIs might also pass the this pointer in a particular special way, right?

On 32-bit Windows, MSVC++ passes this in the ECX register (__thiscall convention), and so does LDC for extern(C++), while extern(D) puts it in EAX. For a static function (__xopCmp()), extern(D) would put the last arg (i.e., the q pointer) in EAX and pass p on the stack, while non-extern(D) wouldn't use any regs at all... :/

[ibuclaw]

@kinke - I gave up on using methods in D codegen a while ago now. All extern(D) functions must abide by the same calling convention because of delegates.

[kinke]

@ibuclaw So do you always pass this as last arg ABI-wise for extern(D)? That would eliminate this problem for LDC, which currently passes it as first arg...

[ibuclaw]

@kinke From a top-level view, the this pointer is always the first argument in the parameter list on my side. I don't care what the ABI is, as it doesn't change the logic. Because of this the order that parameters are accepted by xopEquals and xopCmp should be the same on all platforms with GDC, regardless of the order they are actually passed.

It is just of minor annoyance that DMD seems to call xopEquals using one ABI, but accept the parameters as another.

[kinke]

DMD calls the TypeInfo_Struct.xopEquals function pointer of type extern(D) bool function(in void*, in void*) in TypeInfo_Struct.compare(lhs, rhs) by passing this = lhs as last arg, i.e., calling xopEquals(rhs, lhs). Is there a D definition of how methods are to be called via function pointers, i.e., at which position the this pointer is to be passed?

[kinke]

Maybe the cleanest solution would be to always create the static __xopCmp function, not only if opCmp takes its argument by value, and have the TypeInfo's xopCmp function pointer always point to that function. The static function can then safely be called via the function pointer, and there'd be no need for any swapping.
That'd lead to a bit more code and possibly some loss of performance if the opCmp call in __xopCmp isn't inlined.

[ibuclaw]

I think that we should stop covering for DMD's flaws, and just fix it at the source. :-)

[kinke]

just fix it at the source

I.e. by always pointing to the __xopCmp function?

[ibuclaw]

Out of curiosity.

1. Do only use thiscall for non-D methods?
2. Do you brew your own calling convention for D?

[kinke]

1. We use thiscall for extern(C++) methods only, and only for 32-bit MSVC targets. Only for MSVC++ compatibility which expects this in ECX.
2. For extern(D), we use multiple ABIs for the different target platforms. They're based on LLVM's default behaviour for the corresponding C calling convention and tweak it a bit where necessary or desired. For 32-bit x86, we employ DMD's Win32 ABI (reversing the args and putting either this, sret or the right-most param in EAX) - for all OS. For x86_64 and non-Windows, we use DMD's toArgTypes(), which used NOT to be fully System V compliant and probably still isn't. On Win64, the ABI is almost correct (dynamic arrays aren't passed by reference as they should).

But all of our extern(D) ABIs currently need to reverse the args (all except for the special ones this and sret) for non-variadic functions because of this issue here. I'd really like to change that. The goal is to render the extern(D) ABIs as close as possible to the corresponding C(++) ones, as stated in the docs.

[ibuclaw]

1. OK.
2. On the one hand I could see someone argue that LLVM is more powerful because you can control that from the frontend, but at the same time it's really horrible that you even have to think about what registers things should be passed in at all. I can only shake my head and chuckle there.

The fact that you tinker with order of arguments for ABI compatibility sake (compatible with what? DMD?) means that LDC is part of the problem that should be fixed.

[kinke]

The fact that you tinker with order of arguments for ABI compatibility sake (compatible with what? DMD?) means that LDC is part of the problem that should be fixed.

Once again I don't quite get what you mean. The problem isn't LDC, the problem is that DMD assumes reversed args in front-end and in druntime/Phobos. If I choose not to reverse the args for extern(D), regardless of which ABI/platform, arrays get sorted in reversed order due to this issue here in front-end + druntime. And of course there's some x86/x86_64 DMD inline assembly (mostly in Phobos iirc, e.g. std.math) which relies on that damn reversed order, even for Win64, although the docs state that reversing should only apply for Win32.
This is where DMD has been and keeps on violating its own ABI specs.

Imo, the approach with xopCmp always pointing to __xopCmp for the issue at hand is a viable and clean one.

[dnadlinger]

The fact that you tinker with order of arguments for ABI compatibility sake (compatible with what? DMD?) means that LDC is part of the problem that should be fixed.

This is a rather myopic statement. There exist large codebases with inline asm for x86/x86_64, and we'd break those in non-obvious ways if we just reversed the argument order compared to what DMD does. Of course you get a free pass on that by just not supporting DMD-style inline ASM at all, but that doesn't mean we (LDC) can just ignore the problem.

I think we all agree that the argument order ought to be changed to match C, but this is not the time and place to discuss that.

Regarding the issue here, I think we should make sure not to perpetuate any hacks and just fix it in a way that's actually guaranteed to work on all platforms (regardless of whether extern(C) and extern(D) is the same, up to reversing, or completely different).

[ibuclaw]

Rebased in #7561