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

Abort ILGen for non supported features in AOT #9071

Merged
merged 2 commits into from
Apr 3, 2020

Conversation

dsouzai
Copy link
Contributor

@dsouzai dsouzai commented Apr 1, 2020

Constant Dynamic, Method Handle Constant, and Method Type Constant
is not currently supported in AOT. Therefore, in this
PR, the compiler throws the appropriate exception in ILGen if this
condition is met. However, because all of these exceptions are a
TR::RecoverableILGenException, if the compiler is generating IL for an
inlined method, it will not abort the compile, but simply refuse to
inline that particular method.

Fixes #6011

@dsouzai
Copy link
Contributor Author

dsouzai commented Apr 1, 2020

jenkins test sanity.functional+aot xlinux jdk11

@dsouzai
Copy link
Contributor Author

dsouzai commented Apr 1, 2020

Yay the AOT test passed.

@dsouzai dsouzai force-pushed the condyPrimitiveAOT branch from e19a280 to 104a8de Compare April 1, 2020 20:53
@dsouzai
Copy link
Contributor Author

dsouzai commented Apr 1, 2020

@andrewcraik @cathyzhyi could you please review?

@dsouzai
Copy link
Contributor Author

dsouzai commented Apr 1, 2020

@andrewcraik do we need to be also aborting an AOT compile in these conditions?

         else if (method()->isMethodHandleConstant(cpIndex))
            {
            loadSymbol(TR::aload, symRefTab()->findOrCreateMethodHandleSymbol(_methodSymbol, cpIndex));
            }
         else
            {
            TR_ASSERT(method()->isMethodTypeConstant(cpIndex), "Address-type CP entry %d must be class, string, methodHandle, or methodType", cpIndex);
            loadSymbol(TR::aload, symRefTab()->findOrCreateMethodTypeSymbol(_methodSymbol, cpIndex));
            }

@cathyzhyi
Copy link
Contributor

The change looks good to me.

@andrewcraik
Copy link
Contributor

@dsouzai I think the answer to your question on constant handles is yes - those constants could change in a new run.

dsouzai added 2 commits April 2, 2020 10:57
Constant Dynamic, Method Handle Constant, and Method Type Constant
 is not currently supported in AOT. Therefore, this
commit throws the appropriate exception in ILGen if this
condition is met. However, because all of these exceptions are a
TR::RecoverableILGenException, if the compiler is generating IL for an
inlined method, it will not abort the compile, but simply refuse to
inline that particular method.

Signed-off-by: Irwin D'Souza <[email protected]>
@dsouzai dsouzai force-pushed the condyPrimitiveAOT branch from 104a8de to c33a120 Compare April 2, 2020 15:05
@dsouzai dsouzai changed the title Abort ILGen if Constant Dynamic under AOT Abort ILGen for non supported features in AOT Apr 2, 2020
@dsouzai
Copy link
Contributor Author

dsouzai commented Apr 2, 2020

@andrewcraik @cathyzhyi PR is ready for full review now.

@andrewcraik
Copy link
Contributor

The aborts for the 292 handles are conservative - I am fine with doing it as discussed above, but there should be an issue to track disabling the code that would fold these so that we just leave them as loads. They would need a relocation to load them from the constant pool, but assuming we did that then it should work, just the scope of optimization will be limited because we cannot treat them as constant.

@dsouzai
Copy link
Contributor Author

dsouzai commented Apr 3, 2020

@andrewcraik opened #9106 to track this (and also added it to #2921).

@dsouzai
Copy link
Contributor Author

dsouzai commented Apr 3, 2020

jenkins test sanity.functional all jdk11

@dsouzai
Copy link
Contributor Author

dsouzai commented Apr 3, 2020

@andrewcraik all tests passed, do you mind merging if everything looks good to you?

@andrewcraik andrewcraik merged commit 763c464 into eclipse-openj9:master Apr 3, 2020
@dsouzai dsouzai deleted the condyPrimitiveAOT branch January 4, 2021 22:39
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.

Crash in compiler in CondyPrimitive_* tests under forceAOT
3 participants