Skip to content

Consider using pair mode to return scalar pair bools as i1 #52198

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

Open
cuviper opened this issue Jul 10, 2018 · 2 comments
Open

Consider using pair mode to return scalar pair bools as i1 #52198

cuviper opened this issue Jul 10, 2018 · 2 comments
Labels
A-codegen Area: Code generation A-cranelift Things relevant to the [future] cranelift backend T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@cuviper
Copy link
Member

cuviper commented Jul 10, 2018

Starting in #51583, we're representing scalar pair bools as i8 in LLVM aggregates to match their memory storage, whereas they are i1 as immediate values.

When a pair is the argument to a function, we use PassMode::Pair and pass each part like independent immediate values. We don't use that mode for return values though, so a paired bool will be extended to i8 for return, then truncated back to i1 when the caller unpacks it.

Quoting @eddyb in #51583 (comment):

I wonder if they should be using the pair "passing mode" that arguments do, and create a LLVM aggregate type on the fly, using the immediate types for the pair components. That way we'd get {i1, i1} for returns, but everything else would see {i8, i8}.

Not sure it's worth the complexity though. When inlining, LLVM should collapse the zext followed by trunc, just like it gets rid of packing into a pair and unpacking it.

@cuviper cuviper added A-codegen Area: Code generation T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 10, 2018
@eddyb
Copy link
Member

eddyb commented Jul 13, 2018

I think that for compatibility with backends that support "true multiple return" (i.e. Cranelift), we should be using PassMode::Pair for returns as well, with more explicit packing. cc @sunfishcode

@sunfishcode
Copy link
Member

That sounds right to me. Even in LLVM, an aggregate return type in LLVM is essentially LLVM's convention for multiple return values, so it sounds desirable to use i1 there too.

@Centril Centril added the A-cranelift Things relevant to the [future] cranelift backend label Mar 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen Area: Code generation A-cranelift Things relevant to the [future] cranelift backend T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants