hg: valhalla/valhalla: 8222711: [lworld] Initial skeletal implementation of inline class instance construction via <init>

[John Rose]

On Apr 30, 2019, at 9:41 AM, Tobias Hartmann <tobias.hartmann at oracle.com> wrote:
> 
> Hi Harold,
> 
> On 29.04.19 15:52, Harold Seigel wrote:
>> Open Webrev: http://cr.openjdk.java.net/~hseigel/lworld_8222787/webrev/
> 
> The compiler changes look good to me.
> 
>> 2. Two hotspot JTReg compiler/valhalla tests fail with this change:
>>     1. compiler/valhalla/valuetypes/TestBasicFunctionality.java
>>     2. compiler/valhalla/valuetypes/TestLWorld.java
> 
> The tests fail because C2 does not inline some of the '<init>' methods although the constructor is
> annotated with @ForceInline.

Does the annotation make it into the class file?
Either javac is failing to carry it from the constructor
AST to the static init factory, or else the JVM is failing
to apply it to the static init factory, or else the JIT is
ignoring the bit.  (Or else something else!)

> The test infrastructure iterates over all methods returned by Class::getDeclaredMethods and calls
> WhiteBox::testSetForceInlineMethod for those methods annotated with @ForceInline:
> http://hg.openjdk.java.net/valhalla/valhalla/file/eeb82eab9c31/test/hotspot/jtreg/compiler/valhalla/valuetypes/ValueTypeTest.java#l528
> 
> Now that worked with old 'makeValue' because it's a normal method and therefore returned by
> getDeclaredMethods but it does not work with the new '<init>' constructor. Below is a little test
> that shows the difference. Before, the test printed "public static MyValue1/val
> MyValue1.$makeValue$(int)". Now it does not print anything.

That sounds right.  $makeValue$ has been renamed <init>
*and* it has been reclassified as a jlr.Constructor.

> If that is expected behavior, you can go ahead and fix it like this:
> 
> diff -r 51c89c6d10e6 test/hotspot/jtreg/compiler/valhalla/valuetypes/ValueTypeTest.java
> --- a/test/hotspot/jtreg/compiler/valhalla/valuetypes/ValueTypeTest.java	Fri Apr 26 23:27:18 2019 +0530
> +++ b/test/hotspot/jtreg/compiler/valhalla/valuetypes/ValueTypeTest.java	Tue Apr 30 17:35:09 2019 +0200
> @@ -121,6 +121,7 @@
>         "-XX:CompileCommand=compileonly,java.lang.invoke.*::*",
>         "-XX:CompileCommand=compileonly,java.lang.Long::sum",
>         "-XX:CompileCommand=compileonly,java.lang.Object::<init>",
> +        "-XX:CompileCommand=inline,compiler.valhalla.valuetypes.MyValue*::<init>",
>         "-XX:CompileCommand=compileonly,compiler.valhalla.valuetypes.*::*"};
>     private static final String[] printFlags = {
>         "-XX:+PrintCompilation", "-XX:+PrintIdeal", "-XX:+UnlockDiagnosticVMOptions",
> "-XX:+PrintOptoAssembly"};
> 
> I've also noticed that code compiled with -XDnoStaticInitValueFactory throws a ClassFormatError when
> executed. For example, below test fails with:
> 
> Exception in thread "main" java.lang.ClassFormatError: Method <init> in class MyValue1 (an inline
> class) has illegal modifiers: 0x1
> 	at java.base/java.lang.ClassLoader.defineClass1(Native Method)
> 	at java.base/java.lang.ClassLoader.defineClass(ClassLoader.java:1016)
> 	at java.base/java.security.SecureClassLoader.defineClass(SecureClassLoader.java:151)
> 	at java.base/jdk.internal.loader.BuiltinClassLoader.defineClass(BuiltinClassLoader.java:823)
> 
> Best regards,
> Tobias
> 
> 
> import java.lang.annotation.Retention;
> import java.lang.annotation.RetentionPolicy;
> import java.lang.annotation.Repeatable;
> import java.lang.reflect.Method;
> 
> @Retention(RetentionPolicy.RUNTIME)
> @interface ForceInline { }
> 
> value class MyValue1 {
>    public final int x;
> 
>    @ForceInline
>    public MyValue1(int x) {
>        this.x = x;
>    }
> }
> 
> public class Test {
> 
>    public static void main(String[] args) throws Throwable {
>        Method[] methods = MyValue1.class.getMethods();
>        for (Method m : methods) {
>            if (m.isAnnotationPresent(ForceInline.class)) {
>                System.out.println(m);
>            }
>        }
>    }
> }