Skip to content

add ABIInfo trait and separate x86-64 ABI implementation from foreign trans #4590

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
Jan 24, 2013

Conversation

crabtw
Copy link
Contributor

@crabtw crabtw commented Jan 23, 2013

ABIInfo provides a method to translate the type of foreign function.
The foreign trans uses the method to get ABI-specific type and uses the result to generate LLVM instructions for wrapper and shim functions.

@catamorphism
Copy link
Contributor

Sorry, but can you rebase this against the latest incoming and re-submit? foreign has already been changed, and I'm afraid of losing something if I try to do the merge.

@brson
Copy link
Contributor

brson commented Jan 23, 2013

Very cool. I'm glad to see this refactoring.

… trans

ABIInfo provides a method to translate the type of foreign function.
The foreign trans uses the method to get ABI-specific type and
uses the result to generate LLVM instructions for wrapper and shim functions.
@crabtw
Copy link
Contributor Author

crabtw commented Jan 24, 2013

I rebased and pushed.

catamorphism added a commit that referenced this pull request Jan 24, 2013
add ABIInfo trait and separate x86-64 ABI implementation from foreign trans
@catamorphism catamorphism merged commit e4e5d98 into rust-lang:incoming Jan 24, 2013
@catamorphism
Copy link
Contributor

Thanks!

catamorphism added a commit that referenced this pull request Jan 24, 2013
This reverts commit e4e5d98, reversing
changes made to ab8d774.
@catamorphism
Copy link
Contributor

Sorry, I had to revert this because it broke the Windows build. One of us will un-revert once we figure out what's going on.

@graydon
Copy link
Contributor

graydon commented Jan 24, 2013

Thanks for the quick backout. Sorry for the abrupt comment -- was on a cell phone. Just trying to get an all-green to snapshot.

@catamorphism
Copy link
Contributor

No worries, a snapshot will be amazing!

@catamorphism
Copy link
Contributor

@crabtw - Do you have access to a Windows machine? This patch is still failing on Windows: http://buildbot.rust-lang.org/builders/try-win/builds/210/steps/compile/logs/stdio -- and I don't really know where to start as to figuring out why.

@crabtw
Copy link
Contributor Author

crabtw commented Jan 25, 2013

@catamorphism Sorry, I have no Windows machine. https://github.com/mozilla/rust/pull/4590/files#L0R166 is the problem. Replacing the condition with llvm::LLVMGetTypeKind(self.ret_ty.ty) == Void should work.

@catamorphism
Copy link
Contributor

@crabtw Ok, thanks -- I tried making that change and am running try.

@catamorphism
Copy link
Contributor

@crabtw Thanks, that more or less did the trick. I landed this for real in 41adf9d

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.

4 participants