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

Support byval the msvc++ way. #5436

Closed
oscarfv mannequin opened this issue Sep 27, 2009 · 13 comments
Closed

Support byval the msvc++ way. #5436

oscarfv mannequin opened this issue Sep 27, 2009 · 13 comments
Assignees
Labels
backend:X86 bugzilla Issues migrated from bugzilla

Comments

@oscarfv
Copy link
Mannequin

oscarfv mannequin commented Sep 27, 2009

Bugzilla Link 5064
Resolution FIXED
Resolved on Feb 27, 2014 18:23
Version trunk
OS Windows XP
Blocks llvm/llvm-bugzilla-archive#12477 llvm/llvm-bugzilla-archive#17167
CC @asl,@tritao,@rnk,@timurrrr

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.

@oscarfv
Copy link
Mannequin Author

oscarfv mannequin commented Sep 27, 2009

assigned to @rnk

@asl
Copy link
Collaborator

asl commented Sep 27, 2009

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).
How you're going to call these functions? Name mangling used by VCPP is undocumented. Even worse, I don't think this is possible at all -VCPP C++ ABI is undocumented as well.

I really think this PR should be closed as 'wontfix'.

@llvmbot
Copy link
Collaborator

llvmbot commented Sep 27, 2009

I don't see how this is an LLVM issue. LLVM is low-level, it is up to the
front-end to generate explicit calls to constructors and destructors. Yes,
this means the front-end needs to know about the target ABI but that's
already the case. I think this should be closed as wontfix.

@oscarfv
Copy link
Mannequin Author

oscarfv mannequin commented Sep 27, 2009

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.

@asl
Copy link
Collaborator

asl commented Sep 27, 2009

With current LLVM, the front end is unable to implement this feature because
LLVM makes it impossible.
You definitely can. See, how structs are passed by value, you need to do the same thing in your frontend, e.g.:

struct Foo {
int a;
double b;
};

void bar(Foo f);

void foo(void) {
Foo f = {0, 0};
bar(f);
}

is turned into:

define void @​_Z3foov() {
entry:
tail call void @​_Z3bar3Foo( i32 0, double 0.000000e+00 )
ret void
}

declare void @​_Z3bar3Foo(i32, double)

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).
This is neither called "GCC hack" nor "legacy, multi-layered, increasingly generic, internal architecture". It's called Itanium C++ ABI.

@oscarfv
Copy link
Mannequin Author

oscarfv mannequin commented Sep 27, 2009

With current LLVM, the front end is unable to implement this feature because
LLVM makes it impossible.
You definitely can. See, how structs are passed by value, you need to do the
same thing in your frontend, e.g.:

struct Foo {
int a;
double b;
};

void bar(Foo f);

void foo(void) {
Foo f = {0, 0};
bar(f);
}

is turned into:

define void @​_Z3foov() {
entry:
tail call void @​_Z3bar3Foo( i32 0, double 0.000000e+00 )
ret void
}

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? :-)

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).
This is neither called "GCC hack" nor "legacy, multi-layered, increasingly
generic, internal architecture". It's called Itanium C++ ABI.

... 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.

@asl
Copy link
Collaborator

asl commented Jun 20, 2013

*** Bug llvm/llvm-bugzilla-archive#16390 has been marked as a duplicate of this bug. ***

@rnk
Copy link
Collaborator

rnk commented Jun 20, 2013

There some discussion here about how to do fix this in LLVM:
http://llvm.org/bugs/show_bug.cgi?id=16226#c2

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.

@efriedma-quic
Copy link
Collaborator

*** Bug llvm/llvm-bugzilla-archive#17167 has been marked as a duplicate of this bug. ***

@rnk
Copy link
Collaborator

rnk commented Sep 26, 2013

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.

@llvmbot
Copy link
Collaborator

llvmbot commented Nov 4, 2013

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?

@rnk
Copy link
Collaborator

rnk commented Nov 4, 2013

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.

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.

It means that Clang/llvm can't be used in a production environment.

Yes, the Microsoft C++ ABI support is experimental and can't really 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?

CodeGenDAGPatterns.h doesn't cause the bug. It exhibits the bug when we compile it with clang.

@rnk
Copy link
Collaborator

rnk commented Feb 28, 2014

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.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 3, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:X86 bugzilla Issues migrated from bugzilla
Projects
None yet
Development

No branches or pull requests

4 participants