Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Calling convention differs from MSVC for member functions returning SIMD types #104

Open
lhallam opened this issue Jan 22, 2020 · 7 comments
Labels
ABI Application Binary Interface backend:X86 extension:microsoft good first issue https://github.com/llvm/llvm-project/contribute platform:windows

Comments

@lhallam
Copy link

lhallam commented Jan 22, 2020

#include "xmmintrin.h"

class Foo {
public:
  __declspec(noinline) __m128 bar() { 
    return __m128{}; 
  }
};

clang-cl returns in xmm0, msvc takes a second hidden parameter to write to, so when the caller and callee come from objects created by different compilers any integer parameters are offset and the return value is incorrect.

The visual studio documentation (https://docs.microsoft.com/en-us/cpp/build/x64-calling-convention?view=vs-2019#return-values) seem to imply that clang's behaviour is correct, unless __m128 etc are considered user defined types.

Bigcheese pushed a commit to Bigcheese/llvm-project that referenced this issue Feb 27, 2020
@llvmbot
Copy link
Member

llvmbot commented Apr 12, 2022

@llvm/issue-subscribers-backend-x86

mohammed-nurulhoque added a commit to mohammed-nurulhoque/llvm-project that referenced this issue Nov 17, 2023
deletes slots that have lifetime markers and the lifetime ranges are empty.
searlmc1 referenced this issue in ROCm/llvm-project Nov 27, 2023
…2023.06.30

Promote develop branch (as of e1491f6) to mainline
mohammed-nurulhoque added a commit to mohammed-nurulhoque/llvm-project that referenced this issue Dec 13, 2023
deletes slots that have lifetime markers and the lifetime ranges are empty.
mjklemm pushed a commit to mjklemm/llvm-project that referenced this issue Jun 21, 2024
There was mismatch between exec mode represented
by global variable and kernel environment struct
RevySR pushed a commit to revyos/llvm-project that referenced this issue Jul 27, 2024
…ic family (llvm#104)

* [Clang][XTHeadVector] Implement 15.1-15.4 `vred/vfred/vfwred` intrinsic family

* [Clang][XTHeadVector] Test 15.1-15.4 `vred/vfred/vfwred` intrinsic family

* [Clang][XTHeadVector] Implement wrappers for 15.1-15.4 `vred/vfred/vfwred` intrinsic family
@tru
Copy link
Collaborator

tru commented Aug 12, 2024

We just got bit by this as well where we linked to a MSVC compiled library that returned a SIMD type. I would like to fix this so that since I think not being compatible with the MSVC ABI is a bug.

If someone can point me in the right direction of where to look I can try to put up a PR.

cc @rnk @efriedma-quic @aganea

@efriedma-quic
Copy link
Collaborator

MicrosoftCXXABI::classifyReturnType in clang/lib/CodeGen/MicrosoftCXXABI.cpp is the code that handles this sort of thing. Assuming I'm understanding the issue correctly; this is specifically for instance methods of C++ classes, right?

@sylvain-audi
Copy link
Contributor

Here is a simplified repro: https://godbolt.org/z/TM6rW738v

@EugeneZelenko EugeneZelenko added the ABI Application Binary Interface label Aug 12, 2024
@rnk
Copy link
Collaborator

rnk commented Aug 12, 2024

I think this is a better godbolt reproducer:
https://godbolt.org/z/adxj84Tjq

The __m128 naming is significant, because that is a struct definition provided by xmmintrin.h, which is different between MSVC and Clang.

We could return all vector types from C++ instance methods indirectly, but that includes user-defined vector types, which are not required to be ABI-compatible with MSVC. We probably can't reliably differentiate vector types provided by intrinsic headers, so that's probably the correct solution. Code is here:
https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/MicrosoftCXXABI.cpp#L1177

Maybe this is a good first issue for a potential contributor.

@rnk rnk added the good first issue https://github.com/llvm/llvm-project/contribute label Aug 12, 2024
@llvmbot
Copy link
Member

llvmbot commented Aug 12, 2024

Hi!

This issue may be a good introductory issue for people new to working on LLVM. If you would like to work on this issue, your first steps are:

  1. Check that no other contributor has already been assigned to this issue. If you believe that no one is actually working on it despite an assignment, ping the person. After one week without a response, the assignee may be changed.
  2. In the comments of this issue, request for it to be assigned to you, or just create a pull request after following the steps below. Mention this issue in the description of the pull request.
  3. Fix the issue locally.
  4. Run the test suite locally. Remember that the subdirectories under test/ create fine-grained testing targets, so you can e.g. use make check-clang-ast to only run Clang's AST tests.
  5. Create a Git commit.
  6. Run git clang-format HEAD~1 to format your changes.
  7. Open a pull request to the upstream repository on GitHub. Detailed instructions can be found in GitHub's documentation. Mention this issue in the description of the pull request.

If you have any further questions about this issue, don't hesitate to ask via a comment in the thread below.

@llvmbot
Copy link
Member

llvmbot commented Aug 12, 2024

@llvm/issue-subscribers-good-first-issue

Author: Lewis Hallam (lhallam)

```c++ #include "xmmintrin.h"

class Foo {
public:
__declspec(noinline) __m128 bar() {
return __m128{};
}
};


`clang-cl` returns in `xmm0`, msvc takes a second hidden parameter to write to, so when the caller and callee come from objects created by different compilers any integer parameters are offset and the return value is incorrect.

The visual studio documentation (https://docs.microsoft.com/en-us/cpp/build/x64-calling-convention?view=vs-2019#return-values) seem to imply that clang's behaviour is correct, unless __m128 etc are considered user defined types.
</details>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ABI Application Binary Interface backend:X86 extension:microsoft good first issue https://github.com/llvm/llvm-project/contribute platform:windows
Projects
None yet
Development

No branches or pull requests

8 participants