changeset 55905:2315f0cda727 lworld

8225762: [lworld][c1] deoptimization fails with -XX:+StressValueTypeReturnedAsFields Reviewed-by: thartmann
author iklam
date Sun, 16 Jun 2019 21:28:53 -0700
parents 79ba063a282e
children 98a5917e7932
files src/hotspot/cpu/x86/c1_LIRAssembler_x86.cpp src/hotspot/share/c1/c1_IR.cpp src/hotspot/share/c1/c1_IR.hpp src/hotspot/share/c1/c1_LIR.cpp src/hotspot/share/c1/c1_LIR.hpp src/hotspot/share/c1/c1_LIRAssembler.cpp src/hotspot/share/c1/c1_LIRAssembler.hpp test/hotspot/jtreg/compiler/valhalla/valuetypes/TestCallingConvention.java
diffstat 8 files changed, 121 insertions(+), 23 deletions(-) [+]
line wrap: on
line diff
--- a/src/hotspot/cpu/x86/c1_LIRAssembler_x86.cpp	Sun Jun 16 21:24:15 2019 -0700
+++ b/src/hotspot/cpu/x86/c1_LIRAssembler_x86.cpp	Sun Jun 16 21:28:53 2019 -0700
@@ -2882,13 +2882,13 @@
   assert((__ offset() + NativeCall::displacement_offset) % BytesPerWord == 0,
          "must be aligned");
   __ call(AddressLiteral(op->addr(), rtype));
-  add_call_info(code_offset(), op->info());
+  add_call_info(code_offset(), op->info(), op->maybe_return_as_fields());
 }
 
 
 void LIR_Assembler::ic_call(LIR_OpJavaCall* op) {
   __ ic_call(op->addr());
-  add_call_info(code_offset(), op->info());
+  add_call_info(code_offset(), op->info(), op->maybe_return_as_fields());
   assert((__ offset() - NativeCall::instruction_size + NativeCall::displacement_offset) % BytesPerWord == 0,
          "must be aligned");
 }
--- a/src/hotspot/share/c1/c1_IR.cpp	Sun Jun 16 21:24:15 2019 -0700
+++ b/src/hotspot/share/c1/c1_IR.cpp	Sun Jun 16 21:28:53 2019 -0700
@@ -179,6 +179,25 @@
     return false;
 }
 
+void IRScopeDebugInfo::record_debug_info(DebugInformationRecorder* recorder, int pc_offset, bool topmost, bool is_method_handle_invoke, bool maybe_return_as_fields) {
+  if (caller() != NULL) {
+    // Order is significant:  Must record caller first.
+    caller()->record_debug_info(recorder, pc_offset, false/*topmost*/);
+  }
+  DebugToken* locvals = recorder->create_scope_values(locals());
+  DebugToken* expvals = recorder->create_scope_values(expressions());
+  DebugToken* monvals = recorder->create_monitor_values(monitors());
+  // reexecute allowed only for the topmost frame
+  bool reexecute = topmost ? should_reexecute() : false;
+  bool return_oop = false; // This flag will be ignored since it used only for C2 with escape analysis.
+  bool rethrow_exception = false;
+  bool return_vt = false;
+  if (maybe_return_as_fields) {
+    return_oop = true;
+    return_vt = true;
+  }
+  recorder->describe_scope(pc_offset, methodHandle(), scope()->method(), bci(), reexecute, rethrow_exception, is_method_handle_invoke, return_oop, return_vt, locvals, expvals, monvals);
+}
 
 // Implementation of CodeEmitInfo
 
@@ -211,10 +230,10 @@
 }
 
 
-void CodeEmitInfo::record_debug_info(DebugInformationRecorder* recorder, int pc_offset) {
+void CodeEmitInfo::record_debug_info(DebugInformationRecorder* recorder, int pc_offset, bool maybe_return_as_fields) {
   // record the safepoint before recording the debug info for enclosing scopes
   recorder->add_safepoint(pc_offset, _oop_map->deep_copy());
-  _scope_debug_info->record_debug_info(recorder, pc_offset, true/*topmost*/, _is_method_handle_invoke);
+  _scope_debug_info->record_debug_info(recorder, pc_offset, true/*topmost*/, _is_method_handle_invoke, maybe_return_as_fields);
   recorder->end_safepoint(pc_offset);
 }
 
--- a/src/hotspot/share/c1/c1_IR.hpp	Sun Jun 16 21:24:15 2019 -0700
+++ b/src/hotspot/share/c1/c1_IR.hpp	Sun Jun 16 21:28:53 2019 -0700
@@ -232,20 +232,7 @@
   //Whether we should reexecute this bytecode for deopt
   bool should_reexecute();
 
-  void record_debug_info(DebugInformationRecorder* recorder, int pc_offset, bool topmost, bool is_method_handle_invoke = false) {
-    if (caller() != NULL) {
-      // Order is significant:  Must record caller first.
-      caller()->record_debug_info(recorder, pc_offset, false/*topmost*/);
-    }
-    DebugToken* locvals = recorder->create_scope_values(locals());
-    DebugToken* expvals = recorder->create_scope_values(expressions());
-    DebugToken* monvals = recorder->create_monitor_values(monitors());
-    // reexecute allowed only for the topmost frame
-    bool reexecute = topmost ? should_reexecute() : false;
-    bool return_oop = false; // This flag will be ignored since it used only for C2 with escape analysis.
-    bool rethrow_exception = false;
-    recorder->describe_scope(pc_offset, methodHandle(), scope()->method(), bci(), reexecute, rethrow_exception, is_method_handle_invoke, return_oop, false, locvals, expvals, monvals);
-  }
+  void record_debug_info(DebugInformationRecorder* recorder, int pc_offset, bool topmost, bool is_method_handle_invoke = false, bool maybe_return_as_fields = false);
 };
 
 
@@ -280,7 +267,7 @@
   bool deoptimize_on_exception() const           { return _deoptimize_on_exception; }
 
   void add_register_oop(LIR_Opr opr);
-  void record_debug_info(DebugInformationRecorder* recorder, int pc_offset);
+  void record_debug_info(DebugInformationRecorder* recorder, int pc_offset, bool maybe_return_as_fields = false);
 
   bool     is_method_handle_invoke() const { return _is_method_handle_invoke;     }
   void set_is_method_handle_invoke(bool x) {        _is_method_handle_invoke = x; }
--- a/src/hotspot/share/c1/c1_LIR.cpp	Sun Jun 16 21:24:15 2019 -0700
+++ b/src/hotspot/share/c1/c1_LIR.cpp	Sun Jun 16 21:28:53 2019 -0700
@@ -28,6 +28,7 @@
 #include "c1/c1_LIRAssembler.hpp"
 #include "c1/c1_ValueStack.hpp"
 #include "ci/ciInstance.hpp"
+#include "ci/ciValueKlass.hpp"
 #include "runtime/sharedRuntime.hpp"
 
 Register LIR_OprDesc::as_register() const {
@@ -1021,6 +1022,28 @@
   masm->emit_call(this);
 }
 
+bool LIR_OpJavaCall::maybe_return_as_fields() const {
+  if (ValueTypeReturnedAsFields) {
+    if (method()->signature()->returns_never_null()) {
+      ciType* return_type = method()->return_type();
+      if (return_type->is_valuetype()) {
+        ciValueKlass* vk = return_type->as_value_klass();
+        if (vk->can_be_returned_as_fields()) {
+          return true;
+        }
+      }
+    } else if (is_method_handle_invoke()) {
+      BasicType bt = method()->return_type()->basic_type();
+      if (bt == T_OBJECT || bt == T_VALUETYPE) {
+        // A value type might be returned from the call but we don't know its
+        // type.
+        return true;
+      }
+    }
+  }
+  return false;
+}
+
 void LIR_OpRTCall::emit_code(LIR_Assembler* masm) {
   masm->emit_rtcall(this);
 }
--- a/src/hotspot/share/c1/c1_LIR.hpp	Sun Jun 16 21:24:15 2019 -0700
+++ b/src/hotspot/share/c1/c1_LIR.hpp	Sun Jun 16 21:28:53 2019 -0700
@@ -1227,6 +1227,8 @@
   virtual void emit_code(LIR_Assembler* masm);
   virtual LIR_OpJavaCall* as_OpJavaCall() { return this; }
   virtual void print_instr(outputStream* out) const PRODUCT_RETURN;
+
+  bool maybe_return_as_fields() const;
 };
 
 // --------------------------------------------------
--- a/src/hotspot/share/c1/c1_LIRAssembler.cpp	Sun Jun 16 21:24:15 2019 -0700
+++ b/src/hotspot/share/c1/c1_LIRAssembler.cpp	Sun Jun 16 21:28:53 2019 -0700
@@ -337,9 +337,9 @@
 }
 
 
-void LIR_Assembler::add_call_info(int pc_offset, CodeEmitInfo* cinfo) {
+void LIR_Assembler::add_call_info(int pc_offset, CodeEmitInfo* cinfo, bool maybe_return_as_fields) {
   flush_debug_info(pc_offset);
-  cinfo->record_debug_info(compilation()->debug_info_recorder(), pc_offset);
+  cinfo->record_debug_info(compilation()->debug_info_recorder(), pc_offset, maybe_return_as_fields);
   if (cinfo->exception_handlers() != NULL) {
     compilation()->add_exception_handlers_for_pco(pc_offset, cinfo->exception_handlers());
   }
--- a/src/hotspot/share/c1/c1_LIRAssembler.hpp	Sun Jun 16 21:24:15 2019 -0700
+++ b/src/hotspot/share/c1/c1_LIRAssembler.hpp	Sun Jun 16 21:28:53 2019 -0700
@@ -101,7 +101,7 @@
   Address as_Address_hi(LIR_Address* addr);
 
   // debug information
-  void add_call_info(int pc_offset, CodeEmitInfo* cinfo);
+  void add_call_info(int pc_offset, CodeEmitInfo* cinfo, bool maybe_return_as_fields = false);
   void add_debug_info_for_branch(CodeEmitInfo* info);
   void add_debug_info_for_div0(int pc_offset, CodeEmitInfo* cinfo);
   void add_debug_info_for_div0_here(CodeEmitInfo* info);
--- a/test/hotspot/jtreg/compiler/valhalla/valuetypes/TestCallingConvention.java	Sun Jun 16 21:24:15 2019 -0700
+++ b/test/hotspot/jtreg/compiler/valhalla/valuetypes/TestCallingConvention.java	Sun Jun 16 21:28:53 2019 -0700
@@ -25,6 +25,7 @@
 
 import jdk.test.lib.Asserts;
 
+import java.lang.invoke.*;
 import java.lang.reflect.Method;
 
 /*
@@ -51,6 +52,23 @@
         return null;
     }
 
+    static {
+        try {
+            Class<?> clazz = TestCallingConvention.class;
+            ClassLoader loader = clazz.getClassLoader();
+            MethodHandles.Lookup lookup = MethodHandles.lookup();
+
+            MethodType mt32 = MethodType.methodType(MyValue2.class, boolean.class);
+            test32_mh = lookup.findVirtual(clazz, "test32_interp", mt32);
+
+            MethodType mt33 = MethodType.methodType(Object.class, boolean.class);
+            test33_mh = lookup.findVirtual(clazz, "test33_interp", mt33);
+        } catch (NoSuchMethodException | IllegalAccessException e) {
+            e.printStackTrace();
+            throw new RuntimeException("Method handle lookup failed");
+        }
+    }
+
     public static void main(String[] args) throws Throwable {
         TestCallingConvention test = new TestCallingConvention();
         test.run(args, MyValue1.class, MyValue2.class, MyValue2Inline.class, MyValue4.class, Test27Value1.class, Test27Value2.class, Test27Value3.class);
@@ -270,7 +288,6 @@
     }
 
     @Test()
-    @TempSkipForC1(reason = "JDK-8225374: C1 fails with -XX:+StressValueTypeReturnedAsFields")
     public MyValue2 test14(boolean flag) {
         return test14_interp(flag);
     }
@@ -618,4 +635,54 @@
         test31_vt.verify(vt);
     }
 */
+
+    // Test deoptimization at call return with return value in registers. Same as test14, except the interpreted method
+    // is called via a MethodHandle.
+    static MethodHandle test32_mh;
+
+    @DontCompile
+    public MyValue2 test32_interp(boolean deopt) {
+        if (deopt) {
+            // uncommon trap
+            WHITE_BOX.deoptimizeMethod(tests.get(getClass().getSimpleName() + "::test32"));
+        }
+        return MyValue2.createWithFieldsInline(rI+32, true);
+    }
+
+    @Test()
+    public MyValue2 test32(boolean flag) throws Throwable {
+        return (MyValue2)test32_mh.invokeExact(this, flag);
+    }
+
+    @DontCompile
+    public void test32_verifier(boolean warmup) throws Throwable {
+        MyValue2 result = test32(!warmup);
+        MyValue2 v = MyValue2.createWithFieldsInline(rI+32, true);
+        Asserts.assertEQ(result.hash(), v.hash());
+    }
+
+    // Same as test32, except the return type is not flattenable.
+    static MethodHandle test33_mh;
+
+    @DontCompile
+    public Object test33_interp(boolean deopt) {
+        if (deopt) {
+            // uncommon trap
+            WHITE_BOX.deoptimizeMethod(tests.get(getClass().getSimpleName() + "::test33"));
+        }
+        return MyValue2.createWithFieldsInline(rI+33, true);
+    }
+
+    @Test()
+    public MyValue2 test33(boolean flag) throws Throwable {
+        Object o = test33_mh.invokeExact(this, flag);
+        return (MyValue2)o;
+    }
+
+    @DontCompile
+    public void test33_verifier(boolean warmup) throws Throwable {
+        MyValue2 result = test33(!warmup);
+        MyValue2 v = MyValue2.createWithFieldsInline(rI+33, true);
+        Asserts.assertEQ(result.hash(), v.hash());
+    }
 }