8229897: [lworld] C1 should avoid allocation when reading a field from a flattened field

Reviewed-by: thartmann

Author: fparain

src/hotspot/share/c1/c1_GraphBuilder.cpp

@@ -1837,25 +1837,27 @@ void GraphBuilder::access_field(Bytecodes::Code code) {
         assert(Interpreter::bytecode_should_reexecute(code), "should reexecute");
         state_before = copy_state_before();
       }
-      obj = apop();
-      ObjectType* obj_type = obj->type()->as_ObjectType();
-      if (field->is_constant() && !field->is_flattened() && obj_type->is_constant() && !PatchALot) {
-        ciObject* const_oop = obj_type->constant_value();
-        if (!const_oop->is_null_object() && const_oop->is_loaded()) {
-          ciConstant field_value = field->constant_value_of(const_oop);
-          if (field_value.is_valid()) {
-            if (field->signature()->is_Q_signature() && field_value.is_null_or_zero()) {
-              // Non-flattened inline type field. Replace null by the default value.
-              constant = new Constant(new InstanceConstant(field->type()->as_inline_klass()->default_instance()));
-            } else {
-              constant = make_constant(field_value, field);
-            }
-            // For CallSite objects add a dependency for invalidation of the optimization.
-            if (field->is_call_site_target()) {
-              ciCallSite* call_site = const_oop->as_call_site();
-              if (!call_site->is_fully_initialized_constant_call_site()) {
-                ciMethodHandle* target = field_value.as_object()->as_method_handle();
-                dependency_recorder()->assert_call_site_target_value(call_site, target);
+      if (!has_delayed_flattened_field_access()) {
+        obj = apop();
+        ObjectType* obj_type = obj->type()->as_ObjectType();
+        if (field->is_constant() && !field->is_flattened() && obj_type->is_constant() && !PatchALot) {
+          ciObject* const_oop = obj_type->constant_value();
+          if (!const_oop->is_null_object() && const_oop->is_loaded()) {
+            ciConstant field_value = field->constant_value_of(const_oop);
+            if (field_value.is_valid()) {
+              if (field->signature()->is_Q_signature() && field_value.is_null_or_zero()) {
+                // Non-flattened inline type field. Replace null by the default value.
+                constant = new Constant(new InstanceConstant(field->type()->as_inline_klass()->default_instance()));
+              } else {
+                constant = make_constant(field_value, field);
+              }
+              // For CallSite objects add a dependency for invalidation of the optimization.
+              if (field->is_call_site_target()) {
+                ciCallSite* call_site = const_oop->as_call_site();
+                if (!call_site->is_fully_initialized_constant_call_site()) {
+                  ciMethodHandle* target = field_value.as_object()->as_method_handle();
+                  dependency_recorder()->assert_call_site_target_value(call_site, target);
+                }
               }
             }
           }
@@ -1868,7 +1870,15 @@ void GraphBuilder::access_field(Bytecodes::Code code) {
           state_before = copy_state_for_exception();
         }
         if (!field->is_flattened()) {
-          LoadField* load = new LoadField(obj, offset, field, false, state_before, needs_patching);
+          LoadField* load;
+          if (!has_delayed_flattened_field_access()) {
+            load = new LoadField(obj, offset, field, false, state_before, needs_patching);
+          } else {
+            load = new LoadField(_delayed_flattened_field_access->obj(),
+                                 _delayed_flattened_field_access->offset() + offset - field->holder()->as_inline_klass()->first_field_offset(),
+                                 field, false, state_before, needs_patching);
+            _delayed_flattened_field_access = NULL;
+          }
           Value replacement = !needs_patching ? _memory->load(load) : load;
           if (replacement != load) {
             assert(replacement->is_linked() || !replacement->can_be_linked(), "should already by linked");
@@ -1893,18 +1903,38 @@ void GraphBuilder::access_field(Bytecodes::Code code) {
           } else {
             push(type, append(load));
           }
-        } else { // flattened field, not optimized solution: re-instantiate the flattened value
-          assert(field->type()->is_inlinetype(), "Sanity check");
-          ciInlineKlass* inline_klass = field->type()->as_inline_klass();
-          int flattening_offset = field->offset() - inline_klass->first_field_offset();
-          assert(field->type()->is_inlinetype(), "Sanity check");
-          scope()->set_wrote_final();
-          scope()->set_wrote_fields();
-          NewInlineTypeInstance* new_instance = new NewInlineTypeInstance(inline_klass, state_before, false);
-          _memory->new_instance(new_instance);
-          apush(append_split(new_instance));
-          copy_inline_content(inline_klass, obj, field->offset(), new_instance, inline_klass->first_field_offset(),
-                       state_before, needs_patching);
+        } else {
+          ciBytecodeStream s(method());
+          s.force_bci(bci());
+          s.next();
+          if (s.cur_bc() == Bytecodes::_getfield && !needs_patching) {
+            if (!has_delayed_flattened_field_access()) {
+              null_check(obj);
+              DelayedFlattenedFieldAccess* dffa = new DelayedFlattenedFieldAccess(obj, field, field->offset());
+              _delayed_flattened_field_access = dffa;
+            } else {
+              _delayed_flattened_field_access->update(field, offset - field->holder()->as_inline_klass()->first_field_offset());
+            }
+          } else {
+            assert(field->type()->is_inlinetype(), "Sanity check");
+            ciInlineKlass* inline_klass = field->type()->as_inline_klass();
+            assert(field->type()->is_inlinetype(), "Sanity check");
+            scope()->set_wrote_final();
+            scope()->set_wrote_fields();
+            NewInlineTypeInstance* new_instance = new NewInlineTypeInstance(inline_klass, state_before, false);
+            _memory->new_instance(new_instance);
+            apush(append_split(new_instance));
+            if (!has_delayed_flattened_field_access()) {
+              copy_inline_content(inline_klass, obj, field->offset(), new_instance, inline_klass->first_field_offset(),
+                          state_before, needs_patching);
+            } else {
+              copy_inline_content(inline_klass, _delayed_flattened_field_access->obj(),
+                                  _delayed_flattened_field_access->offset() + field->offset() - field->holder()->as_inline_klass()->first_field_offset(),
+                                  new_instance, inline_klass->first_field_offset(),
+                                  state_before, needs_patching);
+              _delayed_flattened_field_access = NULL;
+            }
+          }
         }
       }
       break;
@@ -3491,6 +3521,7 @@ GraphBuilder::GraphBuilder(Compilation* compilation, IRScope* scope)
   , _inline_bailout_msg(NULL)
   , _instruction_count(0)
   , _osr_entry(NULL)
+  , _delayed_flattened_field_access(NULL)
 {
   int osr_bci = compilation->osr_bci();
 

src/hotspot/share/c1/c1_GraphBuilder.hpp

@@ -35,6 +35,26 @@
 
 class MemoryBuffer;
 
+class DelayedFlattenedFieldAccess : public CompilationResourceObj {
+private:
+  Value _obj;
+  ciField* _field;
+  int _offset;
+public:
+  DelayedFlattenedFieldAccess(Value obj, ciField* field, int offset)
+  : _obj(obj)
+  , _field(field)
+  , _offset(offset) { }
+
+  void update(ciField* field, int offset) {
+    _field = field;
+    _offset += offset;
+  }
+  Value obj() { return _obj; }
+  ciField* field() { return _field; }
+  int offset() { return _offset; }
+};
+
 class GraphBuilder {
  private:
   // Per-scope data. These are pushed and popped as we descend into
@@ -191,6 +211,10 @@ class GraphBuilder {
   Instruction*      _last;                       // the last instruction added
   bool              _skip_block;                 // skip processing of the rest of this block
 
+  DelayedFlattenedFieldAccess* _delayed_flattened_field_access;
+  bool              has_delayed_flattened_field_access() { return _delayed_flattened_field_access != NULL; }
+
+
   // accessors
   ScopeData*        scope_data() const           { return _scope_data; }
   Compilation*      compilation() const          { return _compilation; }

test/hotspot/jtreg/compiler/valhalla/inlinetypes/GetfieldChains.jcod

@@ -0,0 +1,40 @@
+/*
+ * Copyright (c) 2020, 2020, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+package compiler.valhalla.inlinetypes;
+
+public class NamedRectangle {
+    Rectangle rect = new Rectangle();
+    String name = "";
+
+    static int getP1X(NamedRectangle nr) {
+        return nr.rect
+            .p1
+            .x;
+    }
+
+    static Point getP1(NamedRectangle nr) {
+        return nr.rect
+            .p1;
+    }
+}

test/hotspot/jtreg/compiler/valhalla/inlinetypes/NamedRectangle.java

@@ -0,0 +1,29 @@
+/*
+ * Copyright (c) 2020, 2020, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+package compiler.valhalla.inlinetypes;
+
+public inline class Point {
+    int x = 4;
+    int y = 7;
+}

test/hotspot/jtreg/compiler/valhalla/inlinetypes/Point.java

@@ -0,0 +1,29 @@
+/*
+ * Copyright (c) 2020, 2020, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+package compiler.valhalla.inlinetypes;
+
+public inline class Rectangle {
+    Point p0 = new Point();
+    Point p1 = new Point();
+}

test/hotspot/jtreg/compiler/valhalla/inlinetypes/Rectangle.java

@@ -0,0 +1,185 @@
+/*
+ * Copyright (c) 2020, 2020, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+
+package compiler.valhalla.inlinetypes;
+
+import java.lang.invoke.*;
+import java.lang.reflect.Method;
+import java.nio.file.NoSuchFileException;
+import java.util.Arrays;
+
+import jdk.experimental.value.MethodHandleBuilder;
+import jdk.test.lib.Asserts;
+
+/*
+ * @test
+ * @key randomness
+ * @summary Verify that chains of getfields on flattened fields are correctly optimized
+ * @modules java.base/jdk.experimental.value
+ * @library /testlibrary /test/lib /compiler/whitebox /
+ * @requires os.simpleArch == "x64"
+ * @compile TestGetfieldChains.java NamedRectangle.java Rectangle.java Point.java GetfieldChains.jcod
+ * @run driver ClassFileInstaller sun.hotspot.WhiteBox jdk.test.lib.Platform
+ * @run main/othervm/timeout=300 -Xbootclasspath/a:. -XX:+IgnoreUnrecognizedVMOptions -XX:+UnlockDiagnosticVMOptions
+ *                               -XX:+UnlockExperimentalVMOptions -XX:+WhiteBoxAPI
+ *                               compiler.valhalla.inlinetypes.InlineTypeTest
+ *                               compiler.valhalla.inlinetypes.TestGetfieldChains
+ */
+
+public class TestGetfieldChains extends InlineTypeTest {
+    public static final int C1 = COMP_LEVEL_SIMPLE;
+    public static final int C2 = COMP_LEVEL_FULL_OPTIMIZATION;
+
+    public static void main(String[] args) throws Throwable {
+        TestGetfieldChains test = new TestGetfieldChains();
+        test.run(args, TestGetfieldChains.class);
+    }
+
+    @Override
+    public int getNumScenarios() {
+        return 5;
+    }
+
+    @Override
+    public String[] getVMParameters(int scenario) {
+        switch (scenario) {
+        case 0: return new String[] { // C1 only
+                "-XX:TieredStopAtLevel=1",
+                "-XX:+TieredCompilation",
+            };
+        case 1: return new String[] { // C2 only. (Make sure the tests are correctly written)
+                "-XX:TieredStopAtLevel=4",
+                "-XX:-TieredCompilation",
+                "-XX:-OmitStackTraceInFastThrow",
+            };
+        case 2: return new String[] { // interpreter only
+                "-Xint",
+            };
+        case 3: return new String[] {
+                // Xcomp Only C1.
+                "-XX:TieredStopAtLevel=1",
+                "-XX:+TieredCompilation",
+                "-Xcomp",
+            };
+        case 4: return new String[] {
+                // Xcomp Only C2.
+                "-XX:TieredStopAtLevel=4",
+                "-XX:-TieredCompilation",
+                "-XX:-OmitStackTraceInFastThrow",
+                "-Xcomp",
+            };
+        }
+        return null;
+    }
+
+    // Simple chain of getfields ending with primitive field
+    @Test(compLevel=C1)
+    public int test1() {
+            return NamedRectangle.getP1X(new NamedRectangle());
+    }
+
+    @DontCompile
+    public void test1_verifier(boolean warmup) {
+            int res = test1();
+            Asserts.assertEQ(res, 4);
+    }
+
+    // Simple chain of getfields ending with a flattened field
+    @Test(compLevel=C1)
+    public Point test2() {
+            return NamedRectangle.getP1(new NamedRectangle());
+    }
+
+    @DontCompile
+    public void test2_verifier(boolean warmup) {
+        Point p = test2();
+        Asserts.assertEQ(p.x, 4);
+        Asserts.assertEQ(p.y, 7);
+    }
+
+    // Chain of getfields but the initial receiver is null
+    @Test(compLevel=C1)
+    public NullPointerException test3() {
+        NullPointerException npe = null;
+        try {
+            NamedRectangle.getP1X(null);
+        } catch(NullPointerException e) {
+            npe = e;
+        }
+        return npe;
+    }
+
+    @DontCompile
+    public void test3_verifier(boolean warmup) {
+        NullPointerException npe = test3();
+        Asserts.assertNE(npe, null);
+        StackTraceElement st = npe.getStackTrace()[0];
+        Asserts.assertEQ(st.getMethodName(), "getP1X");
+        Asserts.assertEQ(st.getLineNumber(), 31);       // line number depends on file NamedRectangle.java
+    }
+
+    // Chain of getfields but one getfield in the middle of the chain trigger an illegal access
+    @Test(compLevel=C1)
+    public IllegalAccessError test4() {
+        IllegalAccessError iae = null;
+        try {
+            int i = NamedRectangleP.getP1X(new NamedRectangleP());
+        } catch(IllegalAccessError e) {
+            iae = e;
+        }
+        return iae;
+    }
+
+    @DontCompile
+    public void test4_verifier(boolean warmup) {
+        IllegalAccessError iae = test4();
+        Asserts.assertNE(iae, null);
+        StackTraceElement st = iae.getStackTrace()[0];
+        Asserts.assertEQ(st.getMethodName(), "getP1X");
+        Asserts.assertEQ(st.getLineNumber(), 31);       // line number depends on jcod file generated from NamedRectangle.java
+        Asserts.assertTrue(iae.getMessage().contains("class compiler.valhalla.inlinetypes.NamedRectangleP tried to access private field compiler.valhalla.inlinetypes.RectangleP.p1"));
+    }
+
+    // Chain of getfields but the last getfield trigger a NoSuchFieldError
+    @Test(compLevel=C1)
+    public NoSuchFieldError test5() {
+        NoSuchFieldError nsfe = null;
+        try {
+            int i = NamedRectangleN.getP1X(new NamedRectangleN());
+        } catch(NoSuchFieldError e) {
+            nsfe = e;
+        }
+        return nsfe;
+    }
+
+    @DontCompile
+    public void test5_verifier(boolean warmup) {
+        NoSuchFieldError nsfe = test5();
+        Asserts.assertNE(nsfe, null);
+        StackTraceElement st = nsfe.getStackTrace()[0];
+        Asserts.assertEQ(st.getMethodName(), "getP1X");
+        Asserts.assertEQ(st.getLineNumber(), 31);       // line number depends on jcod file generated from NamedRectangle.java
+        Asserts.assertEQ(nsfe.getMessage(), "x");
+    }
+}