fix Issue 21515 extern(C) and extern(C++) returns creal in wrong order

[ibuclaw]

@WalterBright this works when the extern(C) or extern(C++) function is external.
i.e:

extern(C++) creal cpp_twol();

extern(C) int main()
{
    auto a = cpp_twol();
    assert(a.re == 2 && a.im == 0);
    auto b = a;
    assert(b.re == 2 && b.im == 0);
    return 0;
}

But fails when the function is implemented in D.

extern(C++) creal cpp_twol() { return 2+0Li; }

extern(C) int main()
{
    auto a = cpp_twol();
    assert(a.re == 2 && a.im == 0);
    auto b = a;
    assert(b.re == 2 && b.im == 0);
    return 0;
}

Is this enough of a hint to do the fix proper?

[dlang-bot]

Thanks for your pull request, @ibuclaw!

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	21515	major	extern(C) and extern(C++) returns creal in wrong order

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 + dmd#12073"

[RazvanN7]

I am not capable of understanding whether the fix is correct, however, you should include the test so that we see whether it passes or not.

[ibuclaw]

I am not capable of understanding whether the fix is correct, however, you should include the test so that we see whether it passes or not.

It doesn't when it's implemented in D. I say this in my original post. ;-)

[RazvanN7]

Whatever it is fixing, it should have a test to not let reviewers guess what is the problem that is being fixed.

As I understand, this PR is fixing the code generation for the first example that you are providing. If that is the case, then please, include the test.

[ibuclaw]

I don't consider it to be fixing anything at all, because it occurs at a place that affects every read of a complex number. There has to be a better place to tell the dmd backend to read a complex number backwards when it is returned from a function.

This is why @WalterBright should have a look to see if the hint sparks any suggestions.

[WalterBright]

Looks ok to me. Add the test!

[ibuclaw]

Looks ok to me. Add the test!

Isn't there any way to swap the re/im values earlier in codegen? It only needs to be done for

1. Return statements in an extern C/C++ function.
2. Expressions that read a complex number returned by an extern C/C++ function.

[ibuclaw]

According to SYS V ABI for AMD64:

If the class is COMPLEX_X87, the real part of the value is returned in %st0 and the imaginary part in %st1.

And if I understand loadComplex correctly, it always stores the values in the reverse?

    note87(e, 0, 1);
    note87(e, sz, 0);

[ibuclaw]

Inserting fxch at both the caller and callee return ought to be the simplest thing to do.

[ibuclaw]

@WalterBright added fxch before returning from, and after calling creal functions.

[ibuclaw]

Not strictly block #12069, but this bug came up when I was testing complex number passing.

[ibuclaw]

@WalterBright OK? I've added more tests than strictly necessary, but wanted to be paranoid about changing the ABI for anything other than what is being targetted.

[12345swordy]

72 hours have pass.