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

Enable the simd_conversions test for AArch64 #3082

Merged
merged 1 commit into from
Jul 16, 2021

Conversation

akirilov-arm
Copy link
Contributor

No description provided.

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

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

This looks good to me, but I don't have a ton of experience with arm64 assembly, so I've got what's perhaps a newbie question if you're ok entertaining me. According ot the documentation of scvtf it says:

A floating-point exception can be generated by this instruction. Depending on the settings in FPCR, the exception results in either a flag being set in FPSR, or a synchronous exception being generated. Floating-point exception traps in the ARM Architecture Reference Manual ARMv8, for ARMv8-A architecture profile.

Unfortunately the link on the page is a dead link, but I'm curious if this is something we need to worry about in the codegen here? I think that because all i32 values fit into an f64 we shouldn't have to worry about exceptions being generated, right? (not sure if there are other issues though)

@akirilov-arm
Copy link
Contributor Author

akirilov-arm commented Jul 15, 2021

@alexcrichton The question is not a problem at all; I am afraid that you have not been reading the best documentation on the subject, though that is not your fault. The proper reference for individual instructions (other than the Arm Architecture Reference Manual or Arm ARM, as people tend to call it) can be found on Arm's developer Web site - just click the "Architectures" drop-down menu, then "CPU Architecture", followed by the drop-down menu "A-Profile" in the middle of the page, and finally, probably the most counter-intuitive step, "Exploration tools" (or just use this direct link). This page links both to HTML and to XML renditions of the instruction descriptions (and the system register descriptions like the FPCR that is relevant here), each of which contains the pseudo-code that specifies how instructions behave, including the conditions that cause exceptions. So, browsing through the documentation I think that you are right - since the inputs are 32-bit numbers, there is no danger of getting exceptions. However, there is another subtlety - an architecture implementation (that is, a processor core) is not required to support any of the relevant exceptions; indeed, that is the case for a couple of Arm cores like Cortex-A76. That's why I think that operating systems usually keep them disabled (which is confirmed by a quick check on the machine I have access to); in the worst case, the Wasm runtime can ensure it.

BTW the SCVTF instruction has several variants and you have been looking at the wrong one - "(vector, fixed-point)" instead of "(vector, integer)".

@alexcrichton
Copy link
Member

Ah ok that all definitely makes sense, thanks for the background info! In that case this definitely seems ok to me!

Copyright (c) 2021, Arm Limited.
@akirilov-arm akirilov-arm merged commit 6c3d709 into bytecodealliance:main Jul 16, 2021
@akirilov-arm akirilov-arm deleted the simd_conversions branch July 16, 2021 21:04
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.

2 participants