RFR: 8215473: [lworld] Support for substitutability checks in C2

[Tobias Hartmann]

Hi Roland,

looks good, thanks for making these changes!

I've run the valhalla tiers and found some failures with TestNewAcmp.java:

Testing testNotNull08_4([compiler.valhalla.valuetypes.MyValue1 x=66]) = true
Testing testNotNull08_4([compiler.valhalla.valuetypes.MyValue1 x=42]) = true
Testing testNotNull08_4([compiler.valhalla.valuetypes.MyValue2 x=42]) = true
----------System.err:(18/1215)----------
OpenJDK 64-Bit Server VM warning: TieredCompilation disabled because value types are not supported by C1
java.lang.RuntimeException: assertFalse: expected false, was true
	at jdk.test.lib.Asserts.fail(Asserts.java:594)
	at jdk.test.lib.Asserts.assertFalse(Asserts.java:461)
	at jdk.test.lib.Asserts.assertFalse(Asserts.java:447)
	at compiler.valhalla.valuetypes.TestNewAcmp.run(TestNewAcmp.java:1549)
	at compiler.valhalla.valuetypes.TestNewAcmp.main(TestNewAcmp.java:1566)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at
java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)
	at com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127)
	at java.base/java.lang.Thread.run(Thread.java:835)

It's executed with:

        -XX:MaxRAMPercentage=6 \\
        -XX:+CreateCoredumpOnCrash \\
        -ea \\
        -esa \\
        -XX:CompileThreshold=100 \\
        -XX:+UnlockExperimentalVMOptions \\
        -server \\
        -XX:-TieredCompilation \\
        -XX:+EnableValhalla \\

Thanks,
Tobias

On 25.01.19 11:06, Roland Westrelin wrote:
> 
> Hi Tobias,
> 
> Thanks for the review.
> 
>> Looking at the changes in sharedRuntime.cpp, wouldn't you also need those in
>> SharedRuntime::resolve_opt_virtual_call_C()?
> 
> The call to ValueBootstrapMethods::isSubstitutable() needs to be
> resolved and for that the code explicitly piggy backs on static call
> resolution:
> 
>     CallStaticJavaNode *call = new CallStaticJavaNode(C, TypeFunc::make(subst_method), SharedRuntime::get_resolve_static_call_stub(), subst_method, bci());
> 
> So, no need to change anything for virtual calls.
> 
>> I would prefer moving this code into SharedRuntime::resolve_sub_helper() and guard it by the
>> is_virtual/is_optimized arguments. There you already lookup caller_frame/caller_cb/...
>>
>> Otherwise it looks good to me. Some comments in Parse::do_acmp would be nice but we can refactor
>> that later (this acmp stuff will probably evolve further anyway).
> 
> Here is a new webrev with the change you suggested in sharedRuntime.cpp
> and some comments:
> 
> http://cr.openjdk.java.net/~roland/8215473/webrev.01/
> 
> Roland.
>