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

Fix Bug with Condy Primitive Long and Double #2878

Merged
merged 1 commit into from
Sep 15, 2018

Conversation

r30shah
Copy link
Contributor

@r30shah r30shah commented Sep 14, 2018

While generating IL for byte code "ldc2lw" and "ldc2dw" we were assuming the constant is long and double type respectively. This is not true in case of Constant Dynamic where these primitives are boxed into corresponding object and we need to take special steps for generating appropriate IL.

Signed-off-by: Rahil Shah rahil@ca.ibm.com

While generating IL for byte code "ldc2lw" and "ldc2dw"
we were assuming the constant is long and double type respectively.
This is not true in case of Constant Dynamic where these primitives
are boxed into corresponding object and we need to take special
steps for generating appropriate IL.

Signed-off-by: Rahil Shah <rahil@ca.ibm.com>
@r30shah
Copy link
Contributor Author

r30shah commented Sep 14, 2018

@fjeremic @andrewcraik @yanluo7 This is the fix needed for JIT to identify correct type for primitive long and double condy
This query should handle the case returning appropriate type. https://github.com/eclipse/openj9/blob/9f53aaca6252e507649e90fa51aab06df12bf018/runtime/compiler/ilgen/Walker.cpp#L6344

@yanluo7
Copy link
Contributor

yanluo7 commented Sep 14, 2018

LGTM.
The method()->getLDCType(cpIndex) upfront in loadFromCP should determine the appropriate type and address this issue.

@fjeremic fjeremic self-assigned this Sep 14, 2018
@fjeremic
Copy link
Contributor

Jenkins test sanity xlinux,plinux,zlinux JDK11

@andrewcraik
Copy link
Contributor

LGTM from me - I'll let @fjeremic marshal it through

@fjeremic
Copy link
Contributor

JDK11 failures due to #2708. This was way before we merged any of the JIT changes in this area so it cannot be related to this change.

@fjeremic fjeremic merged commit 8048525 into eclipse-openj9:master Sep 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants