-
Notifications
You must be signed in to change notification settings - Fork 396
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
Disable vector shift and rotate opcodes #7232
Conversation
gita-omr
commented
Jan 18, 2024
- disable vector shift and rotate opcodes until they comply with the JVM Specification
- disable vector shift and rotate opcodes until they comply with the JVM Specification
@BradleyWood @Akira1Saitoh please review |
Do we need to disable vector rotate too? I think that the problem is that we do not apply AND to the shift amount operand. |
I thought we better disable them too until we confirm the exact Java semantic for rotate opcodes as well. They are not a part of the JVM Spec so we need to check when and how they are produced, also create tests (if we don't have them already). |
Jenkins build aarch64,amac,xlinux,osx |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The AArch64 part looks good to me.
See #7181 for the x86-64 macOS failure with TestJitBuilderAPIGenerator. |
Why isn't the solution to this problem to disable the opcode in a J9-specific version of I'm concerned with all the Java-specific motivation here. I don't think the OMR implementations of these opcodes need to bend exclusively to Java's requirements. Rather, if there are parts of the implementation that may have differing semantics based on the language environment then those can be specialized with front end queries. For example, introduce a frontend query to ask whether the shift value needs to be masked or not (and if so, to what value). |
As it currently stands, OpenJ9 doesn't implement |
Right, but it could is my point, and make avoiding this OpenJ9-specific problem entirely an OpenJ9-specific solution. Unless I'm misunderstanding the power of that function. Fixing it may involve introducing language-specific queries to the OMR vector implementations to ask whether masking is required or not. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In theory, an OpenJ9-specific solution should be possible by OpenJ9 providing its own implementation of getSupportsOpCodeForAutoSIMD()
. However, in practice this is hampered by the way this function is currently mplemented in OMR (it turns out that function is overloaded in the CodeGenerator class and it is not straightforward to resolve). Because this limitation is introduced by OMR and the fact that I am aware that there are no other consumers of this code other than OpenJ9 at this point, I'm willing to let this merge as-is.
Ideally, this function's implementation should be changed such that it doesn't rely on an overloaded function name.