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

aarch64: Implement uextend/sextend for i128 values #3005

Merged
merged 2 commits into from
Jun 22, 2021

Conversation

afonso360
Copy link
Contributor

Hey, a few more ops for i128 support

@github-actions github-actions bot added cranelift Issues related to the Cranelift code generator cranelift:area:aarch64 Issues related to AArch64 backend. labels Jun 20, 2021
Copy link
Member

@cfallin cfallin left a comment

Choose a reason for hiding this comment

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

Thanks! Just a suggestion for a clarification to the branching logic below but otherwise looks good.


let needs_extend = from_bits < to_bits && to_bits <= 64;
// For i128, we want to extend the lower half, except if it is already 64 bits.
let needs_lower_extend = to_bits > 64 && from_bits < 64;
Copy link
Member

Choose a reason for hiding this comment

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

Can we clarify the logic a bit by adding a let pass_through_lower = to_bits > 64 && !needs_lower_extend here, then moving the move-emission below (will comment separately)?

});
}
}

Copy link
Member

Choose a reason for hiding this comment

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

With the above pass_through_lower, could we do an else if pass_through_lower { ctx.emit(/* move */) } here? I like that arrangement a bit better as it gives a clear separation between "produce lower register" and "produce upper register" parts of the code.

Copy link
Member

@cfallin cfallin left a comment

Choose a reason for hiding this comment

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

Thanks!

@cfallin cfallin merged commit fa1a04d into bytecodealliance:main Jun 22, 2021
@afonso360 afonso360 deleted the aarch64-i128-extend branch June 25, 2021 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cranelift:area:aarch64 Issues related to AArch64 backend. cranelift Issues related to the Cranelift code generator
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants