Skip to content

improve C ABI #9517

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

Merged
merged 1 commit into from
Oct 11, 2013
Merged

improve C ABI #9517

merged 1 commit into from
Oct 11, 2013

Conversation

crabtw
Copy link
Contributor

@crabtw crabtw commented Sep 26, 2013

I borrow some ideas from clang's ABIInfo.h and TargetInfo.cpp.
LLVMType is replaced with ArgType, which is similar to clang's ABIArgInfo,
and I also merge attrs of FnType into it.

Now ABI implementation doesn't need to insert hidden return pointer
to arg_tys of FnType. Instead it is handled in foreign.rs.

This change also fixes LLVM assertion failure when compiling MIPS target.

cast: bool,
ty: Type
pub struct ArgType {
/// Direct - Pass the argument directly using the normal converted
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this documentation be better attached to the enum variant definitions?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@emberian
Copy link
Member

Someone familiar with this code should take a look (@nikomatsakis? @Aatch?)

@brson
Copy link
Contributor

brson commented Oct 10, 2013

Hm, this looks like some good code. We really need somebody to review.

@nikomatsakis
Copy link
Contributor

I'll take a look today.

@nikomatsakis
Copy link
Contributor

Argh, I remember now that I reviewed about 80% of this patch before! The code looks quite good to me, I'd give it r+ but for the merge conflicts. @crabtw any chance you can rebase it?

I borrow some ideas from clang's ABIInfo.h and TargetInfo.cpp.
LLVMType is replaced with ArgType, which is similar to clang's ABIArgInfo,
and I also merge attrs of FnType into it.

Now ABI implementation doesn't need to insert hidden return pointer
to arg_tys of FnType. Instead it is handled in foreign.rs.

This change also fixes LLVM assertion failure when compiling MIPS target.
@crabtw
Copy link
Contributor Author

crabtw commented Oct 11, 2013

I rebased it.

bors added a commit that referenced this pull request Oct 11, 2013
I borrow some ideas from clang's ABIInfo.h and TargetInfo.cpp.
LLVMType is replaced with ArgType, which is similar to clang's ABIArgInfo,
and I also merge attrs of FnType into it.

Now ABI implementation doesn't need to insert hidden return pointer
to arg_tys of FnType. Instead it is handled in foreign.rs.

This change also fixes LLVM assertion failure when compiling MIPS target.
@bors bors closed this Oct 11, 2013
@bors bors merged commit 95fc31a into rust-lang:master Oct 11, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants