-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Support byval the msvc++ way. #5436
Comments
assigned to @rnk |
I really think this PR should be closed as 'wontfix'. |
I don't see how this is an LLVM issue. LLVM is low-level, it is up to the |
Name mangling is not LLVM's bussiness. (Does LLVM know about GCC's name mangling?). If you have a pointer to a function, you don't care about its name mangling. This PR is not about supporting the full VC++ ABI, just about being able to do one precise thing, not necessarily exclusive of VC++. With current LLVM, the front end is unable to implement this feature because LLVM makes it impossible. In other words, LLVM lacks features required for managing certain function parameters on a certain, natural, way. GCC uses a hack, possibly forced by its legacy, multi-layered, increasingly generic, internal architecture. The GCC hack is not elegant nor efficient (two pointer indirections instead of one for referencing the C++ class). Closing this as wontfix is handwaving. Is saying "I don't care about LLVM's current limitations as long as it is hack-compatible with GCC". I opened this PR not hoping for a fix overnight. This is more geared towards documenting a fact and influencing future evolution of LLVM design. |
struct Foo { void bar(Foo f); void foo(void) { is turned into: define void @_Z3foov() { declare void @_Z3bar3Foo(i32, double)
|
Wouldn't this create data alignment issues? Register/stack confusions (the callee expects the data on the stack, but due to the hacked signature the caller passes data on registers). Performance issues (pushing class data members one by one)? Debug info nuisances? Headaches while reading LLVM or assembler code? :-)
... which was heavily influenced by how GCC does things. But I don't care. Call it GCC, Itanium C++ ABI or whatever, it is a hack. You would agree on that passing C++ classes by "referenced value" has no justification on terms of efficiency. Quite the contrary. |
*** Bug llvm/llvm-bugzilla-archive#16390 has been marked as a duplicate of this bug. *** |
There some discussion here about how to do fix this in LLVM: The consensus seems to be that LLVM needs some way to expose the stack memory used for the call args. Then the frontend can emit the copy ctor call. It's not pretty, it's not portable, but it's probably better than teaching LLVM how to emit a copy ctor, especially given the need to emit EH cleanups. There could also be some side benefits of letting the IR optimizers work on the memory accesses. |
*** Bug llvm/llvm-bugzilla-archive#17167 has been marked as a duplicate of this bug. *** |
llvm/utils/TableGen/CodeGenDAGPatterns.h uses StringMap<SmallVector<...>>, which clang miscompiles due to this bug. We'll have to tackle it sooner rather than later. |
Bug 17167, which was set as a Release Blocker was tagged as a duplicate of this one. In my opinion, that should make this a release blocker as well. It means that Clang/llvm can't be used in a production environment. I haven't looked at the code in llvm/utils/TableGen/CodeGenDAGPatterns.h, but would it be feasible to rework it there so that it used a different method that would allow a debug version to work? |
Yes, I agree, this is basically the most important bug for MSVC compatibility, and I'm working on it. LLVM does time-based releases, so I don't think it will block the 3.4 release. I'd like to fix it by then, but that's up to me.
Yes, the Microsoft C++ ABI support is experimental and can't really be used in a production environment.
CodeGenDAGPatterns.h doesn't cause the bug. It exhibits the bug when we compile it with clang. |
This is basically fixed with inalloca: http://llvm.org/docs/InAlloca.html I've been amazed at how few miscompiles we've hit with it so far. A lot of work is needed to teach LLVM how to optimize it well, especially in the presence of exceptions, but that's TODO. |
Extended Description
When a C++ struct function argument has a destructor or copy constructor and is passed by value, GCC creates a temporary copy somewhere and passes a pointer to the temporary copy to the function. LLVM does not support this directly and is the front-end the responsible of creating the temporary and use the pointer as the function's parameter.
MSVC++ does not use this trick: the C++ class is put on the stack along with the rest of function parameters. As LLVM is the responsible of arranging the parameters on the stack for the function call, this requires that LLVM know about the copy constructor and the destructor.
Right now there is no workaround for this, so LLVM code is unable to call functions compiled with MSVC++ that take C++ classes by value (when those classes have a copy constructor and/or destructor).
LLVM's byval should be extended to allow the front-end to specify a copy constructor and a destructor, and insert calls to them when the temporary is created and after the call is performed.
The text was updated successfully, but these errors were encountered: