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

Handle jitNewValue in code generators #8520

Merged
merged 3 commits into from
Feb 14, 2020

Conversation

Leonardo2718
Copy link
Contributor

Because newvalue support is not implemented in the code generators, the opcode is lowered to new. When lowering happens, the symbol on the new is set to be the same as the one for newvalue, namely jitNewValue.

As a result, new evaluators must check the node's symbol to determine if the allocation is for a value type.

To avoid having to implement full support for inline value type allocations, for now, we unconditionally call the JIT helper. While not ideal for performance, it is a necessary step in incrementally implementing value types.

Note that the AARCH64 code generator already calls whatever helper is set on the new node unconditionally, so no special handling for jitNewValue is required.

@andrewcraik
Copy link
Contributor

implementation seems reasonable - could we create a capability query (maybe on object model?) to store whether value types are supported and use the macro to determine the result to be returned. Ideally, the function should be inlined by the compiler so the dead code is removed.

@Leonardo2718
Copy link
Contributor Author

Leonardo2718 commented Feb 6, 2020

@andrewcraik As suggested, I've added a query to the object model in afe38c1deda099e698962ecd0bfe6e184c81f8f3 1d6cd0481acb5f2819a5be5927400409262090dd and fixed the rest of the code accordingly.

@Leonardo2718 Leonardo2718 force-pushed the jitnewvalue branch 2 times, most recently from 30d16a6 to ecf8e52 Compare February 6, 2020 21:31
@andrewcraik
Copy link
Contributor

Changes to p and z for value types - FYI @gita-omr and @fjeremic

@andrewcraik
Copy link
Contributor

@gita-omr and @fjeremic could you review the codegen part of the change - this is some helpers needed for value types - performance is not critical at this point, we need functionality so we are just always going to call the helper. Improvements to follow later once we have the JIT functional using only helper calls.

Would be ideal to get this merged in the next day or so if possible.

@fjeremic
Copy link
Contributor

Jenkins test sanity all jdk8

@gita-omr
Copy link
Contributor

Would it be possible to add a link to the main issue that contains the whole value type design?

@andrewcraik
Copy link
Contributor

@gita-omr that overarching issue has not been populated - I'll let @tajila add a link once he has something setup.

@Leonardo2718 build failures on ppc - please investigate.

@Leonardo2718
Copy link
Contributor Author

@andrewcraik Power build failures should be fixed now.

@andrewcraik
Copy link
Contributor

Jenkins test sanity all jdk11

@andrewcraik
Copy link
Contributor

Other than the use of auto in place of TR::Compilation this looks good.

@tajila
Copy link
Contributor

tajila commented Feb 12, 2020

#1390

This new query allows checking for value types support with a function
call instead of an #ifdef. Although it is currently a compile type
option, value types support will eventually be decided at runtime, so
using a function call now is preferable to using an #ifdef everywhere.

Signed-off-by: Leonardo Banderali <leonardo2718@protonmail.com>
Because `newvalue` support is not implemented in the code generators,
the opcode is lowered to `new`. When lowering happens, the symbol on
the `new` is set to be the same as the one for `newvalue`, namely
jitNewValue.

As a result, `new` evaluators must check the node's symbol to determine
if the allocation is for a value type.

To avoid having to implement full support for inline value type
allocations, for now, we unconditionally call the JIT helper. While not
ideal for performance, it is a necessary step in incrementally
implementing value types.

Note that the AARCH64 code generator already calls whatever helper is
set on the `new` node unconditionally, so no special handling for
jitNewValue is required.

Signed-off-by: Leonardo Banderali <leonardo2718@protonmail.com>
Some file in the POWER and ARM code generators are guarded with
TR_TARGET_ARM and TR_TARGET_POWER, respectively. These are unnecessary
because those files are only ever built on their respective platform.

Signed-off-by: Leonardo Banderali <leonardo2718@protonmail.com>
@andrewcraik
Copy link
Contributor

Jenkins test sanity all jdk11

@andrewcraik
Copy link
Contributor

Jenkins test xlinux jdk11

@andrewcraik
Copy link
Contributor

xlinux failure is infra - everything else is good so I will merge this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants