changeset 53732:2fd66be05722 lworld

8217241: [lworld] Calling convention should handle abstract methods and null-tolerant value args
author thartmann
date Wed, 16 Jan 2019 12:21:44 +0100
parents 7e7acdba6556
children 266942398494
files src/hotspot/share/oops/method.cpp src/hotspot/share/oops/method.hpp src/hotspot/share/opto/callGenerator.cpp src/hotspot/share/opto/graphKit.cpp src/hotspot/share/opto/parse1.cpp src/hotspot/share/opto/type.cpp src/hotspot/share/runtime/sharedRuntime.cpp test/hotspot/jtreg/compiler/valhalla/valuetypes/TestC2CCalls.java
diffstat 8 files changed, 174 insertions(+), 35 deletions(-) [+]
line wrap: on
line diff
--- a/src/hotspot/share/oops/method.cpp	Tue Jan 15 20:50:42 2019 -0800
+++ b/src/hotspot/share/oops/method.cpp	Wed Jan 16 12:21:44 2019 +0100
@@ -487,10 +487,6 @@
 }
 #endif
 
-bool Method::has_scalarized_args() const {
-  return adapter() != NULL ? (adapter()->get_sig_cc() != NULL) : false;
-}
-
 bool Method::needs_stack_repair() const {
   return adapter() != NULL ? (adapter()->get_res_entry()._offset != -1) : false;
 }
--- a/src/hotspot/share/oops/method.hpp	Tue Jan 15 20:50:42 2019 -0800
+++ b/src/hotspot/share/oops/method.hpp	Wed Jan 16 12:21:44 2019 +0100
@@ -91,7 +91,8 @@
     _has_injected_profile  = 1 << 4,
     _running_emcp          = 1 << 5,
     _intrinsic_candidate   = 1 << 6,
-    _reserved_stack_access = 1 << 7
+    _reserved_stack_access = 1 << 7,
+    _scalarized_args       = 1 << 8
   };
   mutable u2 _flags;
 
@@ -582,7 +583,6 @@
 #endif
 
   // Support for scalarized value type calling convention
-  bool has_scalarized_args() const;
   bool needs_stack_repair() const;
   SigEntry get_res_entry() const;
 
@@ -898,6 +898,14 @@
     _flags = x ? (_flags | _reserved_stack_access) : (_flags & ~_reserved_stack_access);
   }
 
+  bool has_scalarized_args() {
+    return (_flags & _scalarized_args) != 0;
+  }
+
+  void set_has_scalarized_args(bool x) {
+    _flags = x ? (_flags | _scalarized_args) : (_flags & ~_scalarized_args);
+  }
+
   JFR_ONLY(DEFINE_TRACE_FLAG_ACCESSOR;)
 
   ConstMethod::MethodType method_type() const {
--- a/src/hotspot/share/opto/callGenerator.cpp	Tue Jan 15 20:50:42 2019 -0800
+++ b/src/hotspot/share/opto/callGenerator.cpp	Wed Jan 16 12:21:44 2019 +0100
@@ -436,27 +436,24 @@
   uint j = TypeFunc::Parms;
   for (uint i1 = 0; i1 < nargs; i1++) {
     const Type* t = domain_sig->field_at(TypeFunc::Parms + i1);
-    if (method()->get_Method()->has_scalarized_args()) {
+    // TODO for now, don't scalarize value type receivers because of interface calls
+    if (method()->get_Method()->has_scalarized_args() && t->is_valuetypeptr() && !t->maybe_null() && (method()->is_static() || i1 != 0)) {
+      // Value type arguments are not passed by reference: we get an argument per
+      // field of the value type. Build ValueTypeNodes from the value type arguments.
       GraphKit arg_kit(jvms, &gvn);
-      // TODO for now, don't scalarize value type receivers because of interface calls
-      if (t->is_valuetypeptr() && (method()->is_static() || i1 != 0)) {
-        arg_kit.set_control(map->control());
-        ValueTypeNode* vt = ValueTypeNode::make_from_multi(&arg_kit, call, t->value_klass(), j, true);
-        map->set_control(arg_kit.control());
-        map->set_argument(jvms, i1, vt);
-      } else {
-        int index = j;
-        SigEntry res_entry = method()->get_Method()->get_res_entry();
-        if (res_entry._offset != -1 && (index - TypeFunc::Parms) >= res_entry._offset) {
-          // Skip reserved entry
-          index += type2size[res_entry._bt];
-        }
-        map->set_argument(jvms, i1, call->in(index));
-        j++;
+      arg_kit.set_control(map->control());
+      ValueTypeNode* vt = ValueTypeNode::make_from_multi(&arg_kit, call, t->value_klass(), j, true);
+      map->set_control(arg_kit.control());
+      map->set_argument(jvms, i1, vt);
+    } else {
+      int index = j;
+      SigEntry res_entry = method()->get_Method()->get_res_entry();
+      if (res_entry._offset != -1 && (index - TypeFunc::Parms) >= res_entry._offset) {
+        // Skip reserved entry
+        index += type2size[res_entry._bt];
       }
-    } else {
-      Node* arg = call->in(TypeFunc::Parms + i1);
-      map->set_argument(jvms, i1, arg);
+      map->set_argument(jvms, i1, call->in(index));
+      j++;
     }
   }
 
--- a/src/hotspot/share/opto/graphKit.cpp	Tue Jan 15 20:50:42 2019 -0800
+++ b/src/hotspot/share/opto/graphKit.cpp	Wed Jan 16 12:21:44 2019 +0100
@@ -1792,7 +1792,7 @@
       assert(t->is_oopptr()->can_be_value_type(), "wrong argument type");
       ValueTypeNode* vt = arg->as_ValueType();
       // TODO for now, don't scalarize value type receivers because of interface calls
-      if (call->method()->get_Method()->has_scalarized_args() && t->is_valuetypeptr() && (call->method()->is_static() || i != TypeFunc::Parms)) {
+      if (call->method()->get_Method()->has_scalarized_args() && t->is_valuetypeptr() && !t->maybe_null() && (call->method()->is_static() || i != TypeFunc::Parms)) {
         // We don't pass value type arguments by reference but instead
         // pass each field of the value type
         idx += vt->pass_fields(call, idx, *this);
--- a/src/hotspot/share/opto/parse1.cpp	Tue Jan 15 20:50:42 2019 -0800
+++ b/src/hotspot/share/opto/parse1.cpp	Wed Jan 16 12:21:44 2019 +0100
@@ -860,7 +860,7 @@
     const Type* t = tf->domain_sig()->field_at(i);
     Node* parm = NULL;
     // TODO for now, don't scalarize value type receivers because of interface calls
-    if (has_scalarized_args() && t->is_valuetypeptr() && (method()->is_static() || i != TypeFunc::Parms)) {
+    if (has_scalarized_args() && t->is_valuetypeptr() && !t->maybe_null() && (method()->is_static() || i != TypeFunc::Parms)) {
       // Value type arguments are not passed by reference: we get an argument per
       // field of the value type. Build ValueTypeNodes from the value type arguments.
       GraphKit kit(jvms, &gvn);
--- a/src/hotspot/share/opto/type.cpp	Tue Jan 15 20:50:42 2019 -0800
+++ b/src/hotspot/share/opto/type.cpp	Wed Jan 16 12:21:44 2019 +0100
@@ -2083,10 +2083,11 @@
       field_array[pos++] = TypeInt::INT;
       break;
     case T_VALUETYPE: {
-      if (vt_fields_as_args) {
+      bool never_null = sig->is_never_null_at(i);
+      if (vt_fields_as_args && never_null) {
         collect_value_fields(type->as_value_klass(), field_array, pos, &res_entry);
       } else {
-        field_array[pos++] = get_const_type(type)->join_speculative(sig->is_never_null_at(i) ? TypePtr::NOTNULL : TypePtr::BOTTOM);
+        field_array[pos++] = get_const_type(type)->join_speculative(never_null ? TypePtr::NOTNULL : TypePtr::BOTTOM);
       }
       break;
     }
--- a/src/hotspot/share/runtime/sharedRuntime.cpp	Tue Jan 15 20:50:42 2019 -0800
+++ b/src/hotspot/share/runtime/sharedRuntime.cpp	Wed Jan 16 12:21:44 2019 +0100
@@ -1153,7 +1153,7 @@
       }
     }
     // TODO for now, don't scalarize value type receivers because of interface calls
-    if (ValueTypePassFieldsAsArgs && callee->method_holder()->is_value() && false) {
+    if (callee->has_scalarized_args() && callee->method_holder()->is_value() && false) {
       // If the receiver is a value type that is passed as fields, no oop is available.
       // Resolve the call without receiver null checking.
       assert(bc == Bytecodes::_invokevirtual, "only allowed with invokevirtual");
@@ -2711,10 +2711,6 @@
     // make sure data structure is initialized
     initialize();
 
-    if (method->is_abstract()) {
-      return _abstract_method_handler;
-    }
-
     bool has_value_arg = false;
     GrowableArray<SigEntry> sig(method->size_of_parameters());
     if (!method->is_static()) {
@@ -2731,6 +2727,11 @@
       SigEntry::add_entry(&sig, bt);
     }
 
+    // Process abstract method if it has value type args to set has_scalarized_args accordingly
+    if (!has_value_arg && method->is_abstract()) {
+      return _abstract_method_handler;
+    }
+
     // Get a description of the compiled java calling convention and the largest used (VMReg) stack slot usage
     VMRegPair* regs = NEW_RESOURCE_ARRAY(VMRegPair, sig.length());
     int args_on_stack = SharedRuntime::java_calling_convention(&sig, regs);
@@ -2805,9 +2806,15 @@
         sig_cc = sig;
         regs_cc = regs;
         args_on_stack_cc = args_on_stack;
+      } else {
+        method->set_has_scalarized_args(true);
       }
     }
 
+    if (method->is_abstract()) {
+      return _abstract_method_handler;
+    }
+
     // Lookup method signature's fingerprint
     entry = _adapters->lookup(sig_cc.length(), &sig_cc);
 
@@ -2854,7 +2861,7 @@
 
       if (regs != regs_cc) {
         // Save a C heap allocated version of the scalarized signature and store it in the adapter
-        GrowableArray<SigEntry>* heap_sig = new (ResourceObj::C_HEAP, mtInternal)GrowableArray<SigEntry>(method->size_of_parameters(), true);
+        GrowableArray<SigEntry>* heap_sig = new (ResourceObj::C_HEAP, mtInternal) GrowableArray<SigEntry>(method->size_of_parameters(), true);
         heap_sig->appendAll(&sig_cc);
         entry->set_sig_cc(heap_sig);
         entry->set_res_entry(reserved_entry);
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/hotspot/jtreg/compiler/valhalla/valuetypes/TestC2CCalls.java	Wed Jan 16 12:21:44 2019 +0100
@@ -0,0 +1,130 @@
+/*
+ * Copyright (c) 2019, 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.
+ */
+
+/**
+ * @test
+ * @library /test/lib
+ * @summary Test value type calling convention with compiled to compiled calls.
+ * @run main/othervm -XX:+EnableValhalla -XX:-UseBimorphicInlining -Xbatch
+ *                   -XX:CompileCommand=compileonly,TestC2CCalls*::test*
+ *                   -XX:CompileCommand=dontinline,TestC2CCalls*::test*
+ *                   TestC2CCalls
+ */
+
+import jdk.test.lib.Asserts;
+import jdk.test.lib.Utils;
+
+public class TestC2CCalls {
+
+    public static final int rI = Utils.getRandomInstance().nextInt() % 1000;
+
+    static value class OtherVal {
+        public final int x;
+
+        private OtherVal(int x) {
+            this.x = x;
+        }
+    }
+
+    static interface MyInterface {
+        public int test1(OtherVal other, int y);
+        public int test2(OtherVal.val other1, OtherVal.box other2, int y);
+    }
+
+    static value class MyValue implements MyInterface {
+        public final int x;
+
+        private MyValue(int x) {
+            this.x = x;
+        }
+
+        @Override
+        public int test1(OtherVal other, int y) {
+            return x + other.x + y;
+        }
+
+        @Override
+        public int test2(OtherVal.val other1, OtherVal.box other2, int y) {
+            return x + other1.x + other2.x + y;
+        }
+    }
+
+    static class MyObject implements MyInterface {
+        private final int x;
+
+        private MyObject(int x) {
+            this.x = x;
+        }
+
+        @Override
+        public int test1(OtherVal other, int y) {
+            return x + other.x + y;
+        }
+
+        @Override
+        public int test2(OtherVal.val other1, OtherVal.box other2, int y) {
+            return x + other1.x + other2.x + y;
+        }
+    }
+
+    // Test calling methods with value type arguments through an interface
+    public static int test1(MyInterface intf, OtherVal other, int y) {
+        return intf.test1(other, y);
+    }
+
+    public static int test2(MyInterface intf, OtherVal other, int y) {
+        return intf.test2(other, other, y);
+    }
+
+    // Test mixing null-tolerant and null-free value type arguments
+    public static int test3(MyValue vt, OtherVal other, int y) {
+        return vt.test2(other, other, y);
+    }
+
+    public static int test4(MyObject obj, OtherVal other, int y) {
+        return obj.test2(other, other, y);
+    }
+
+    public static void main(String[] args) {
+        MyValue val = new MyValue(rI);
+        OtherVal other = new OtherVal(rI+1);
+        MyObject obj = new MyObject(rI+2);
+
+        // Make sure callee methods are compiled
+        for (int i = 0; i < 10_000; ++i) {
+            Asserts.assertEQ(val.test1(other, rI), 3*rI+1);
+            Asserts.assertEQ(obj.test1(other, rI), 3*rI+3);
+            Asserts.assertEQ(val.test2(other, other, rI), 4*rI+2);
+            Asserts.assertEQ(obj.test2(other, other, rI), 4*rI+4);
+        }
+
+        for (int i = 0; i < 100_000; ++i) {
+            Asserts.assertEQ(test1(val, other, rI), 3*rI+1);
+            Asserts.assertEQ(test1(obj, other, rI), 3*rI+3);
+            Asserts.assertEQ(test2(val, other, rI), 4*rI+2);
+            Asserts.assertEQ(test2(obj, other, rI), 4*rI+4);
+            Asserts.assertEQ(test3(val, other, rI), 4*rI+2);
+            Asserts.assertEQ(test4(obj, other, rI), 4*rI+4);
+        }
+    }
+}