8352647: [lworld] Remove larval InlineTypeNode in Unsafe intrinsics

Reviewed-by: jbhateja, thartmann

Author: Quan Anh Mai

src/hotspot/share/opto/doCall.cpp

@@ -29,6 +29,7 @@
 #include "compiler/compileBroker.hpp"
 #include "compiler/compileLog.hpp"
 #include "interpreter/linkResolver.hpp"
+#include "jvm_io.h"
 #include "logging/log.hpp"
 #include "logging/logLevel.hpp"
 #include "logging/logMessage.hpp"
@@ -146,7 +147,21 @@ CallGenerator* Compile::call_generator(ciMethod* callee, int vtable_index, bool
   // methods.  If these methods are replaced with specialized code,
   // then we return it as the inlined version of the call.
   CallGenerator* cg_intrinsic = nullptr;
-  if (allow_inline && allow_intrinsics) {
+  if (callee->intrinsic_id() == vmIntrinsics::_makePrivateBuffer || callee->intrinsic_id() == vmIntrinsics::_finishPrivateBuffer) {
+    // These methods must be inlined so that we don't have larval value objects crossing method
+    // boundaries
+    assert(!call_does_dispatch, "callee should not be virtual %s", callee->name()->as_utf8());
+    CallGenerator* cg = find_intrinsic(callee, call_does_dispatch);
+
+    if (cg == nullptr) {
+      // This is probably because the intrinsics is disabled from the command line
+      char reason[256];
+      jio_snprintf(reason, sizeof(reason), "cannot find an intrinsics for %s", callee->name()->as_utf8());
+      C->record_method_not_compilable(reason);
+      return nullptr;
+    }
+    return cg;
+  } else if (allow_inline && allow_intrinsics) {
     CallGenerator* cg = find_intrinsic(callee, call_does_dispatch);
     if (cg != nullptr) {
       if (cg->is_predicated()) {
@@ -644,6 +659,10 @@ void Parse::do_call() {
   // This call checks with CHA, the interpreter profile, intrinsics table, etc.
   // It decides whether inlining is desirable or not.
   CallGenerator* cg = C->call_generator(callee, vtable_index, call_does_dispatch, jvms, try_inline, prof_factor(), speculative_receiver_type);
+  if (failing()) {
+    return;
+  }
+  assert(cg != nullptr, "must find a CallGenerator for callee %s", callee->name()->as_utf8());
 
   // NOTE:  Don't use orig_callee and callee after this point!  Use cg->method() instead.
   orig_callee = callee = nullptr;

src/hotspot/share/opto/graphKit.cpp

@@ -3503,9 +3503,22 @@ Node* GraphKit::gen_instanceof(Node* obj, Node* superklass, bool safe_for_replac
 // If failure_control is supplied and not null, it is filled in with
 // the control edge for the cast failure.  Otherwise, an appropriate
 // uncommon trap or exception is thrown.
-Node* GraphKit::gen_checkcast(Node *obj, Node* superklass, Node* *failure_control, bool null_free) {
+Node* GraphKit::gen_checkcast(Node* obj, Node* superklass, Node* *failure_control, bool null_free) {
   kill_dead_locals();           // Benefit all the uncommon traps
   const TypeKlassPtr* klass_ptr_type = _gvn.type(superklass)->is_klassptr();
+  const Type* obj_type = _gvn.type(obj);
+  if (obj_type->is_inlinetypeptr() && !obj_type->maybe_null() && klass_ptr_type->klass_is_exact() && obj_type->inline_klass() == klass_ptr_type->exact_klass(true)) {
+    // Special case: larval inline objects must not be scalarized. They are also generally not
+    // allowed to participate in most operations except as the first operand of putfield, or as an
+    // argument to a constructor invocation with it being a receiver, Unsafe::putXXX with it being
+    // the first argument, or Unsafe::finishPrivateBuffer. This allows us to aggressively scalarize
+    // value objects in all other places. This special case comes from the limitation of the Java
+    // language, Unsafe::makePrivateBuffer returns an Object that is checkcast-ed to the concrete
+    // value type. We must do this first because C->static_subtype_check may do nothing when
+    // StressReflectiveCode is set.
+    return obj;
+  }
+
   const TypeKlassPtr* improved_klass_ptr_type = klass_ptr_type->try_improve();
   const TypeOopPtr* toop = improved_klass_ptr_type->cast_to_exactness(false)->as_instance_type();
   bool safe_for_replace = (failure_control == nullptr);
@@ -3519,13 +3532,13 @@ Node* GraphKit::gen_checkcast(Node *obj, Node* superklass, Node* *failure_contro
   // for example, in some objArray manipulations, such as a[i]=a[j].)
   if (improved_klass_ptr_type->singleton()) {
     const TypeKlassPtr* kptr = nullptr;
-    const Type* t = _gvn.type(obj);
-    if (t->isa_oop_ptr()) {
-      kptr = t->is_oopptr()->as_klass_type();
+    if (obj_type->isa_oop_ptr()) {
+      kptr = obj_type->is_oopptr()->as_klass_type();
     } else if (obj->is_InlineType()) {
-      ciInlineKlass* vk = t->inline_klass();
+      ciInlineKlass* vk = obj_type->inline_klass();
       kptr = TypeInstKlassPtr::make(TypePtr::NotNull, vk, Type::Offset(0));
     }
+
     if (kptr != nullptr) {
       switch (C->static_subtype_check(improved_klass_ptr_type, kptr)) {
       case Compile::SSC_always_true:
@@ -3545,7 +3558,7 @@ Node* GraphKit::gen_checkcast(Node *obj, Node* superklass, Node* *failure_contro
           obj = null_check(obj);
         }
         // It needs a null check because a null will *pass* the cast check.
-        if (t->isa_oopptr() != nullptr && !t->is_oopptr()->maybe_null()) {
+        if (obj_type->isa_oopptr() != nullptr && !obj_type->is_oopptr()->maybe_null()) {
           bool is_aastore = (java_bc() == Bytecodes::_aastore);
           Deoptimization::DeoptReason reason = is_aastore ?
             Deoptimization::Reason_array_check : Deoptimization::Reason_class_check;

src/hotspot/share/opto/library_call.cpp

@@ -2663,40 +2663,65 @@ bool LibraryCallKit::inline_unsafe_access(bool is_store, const BasicType type, c
 bool LibraryCallKit::inline_unsafe_make_private_buffer() {
   Node* receiver = argument(0);
   Node* value = argument(1);
-  if (!value->is_InlineType()) {
+
+  const Type* type = gvn().type(value);
+  if (!type->is_inlinetypeptr()) {
+    C->record_method_not_compilable("value passed to Unsafe::makePrivateBuffer is not of a constant value type");
     return false;
   }
 
-  receiver = null_check(receiver);
+  null_check(receiver);
+  if (stopped()) {
+    return true;
+  }
+
+  value = null_check(value);
   if (stopped()) {
     return true;
   }
 
-  set_result(value->as_InlineType()->make_larval(this, true));
+  ciInlineKlass* vk = type->inline_klass();
+  Node* klass = makecon(TypeKlassPtr::make(vk));
+  Node* obj = new_instance(klass);
+  AllocateNode::Ideal_allocation(obj)->_larval = true;
+
+  assert(value->is_InlineType(), "must be an InlineTypeNode");
+  value->as_InlineType()->store(this, obj, obj, vk);
+
+  set_result(obj);
   return true;
 }
 
 bool LibraryCallKit::inline_unsafe_finish_private_buffer() {
   Node* receiver = argument(0);
   Node* buffer = argument(1);
-  if (!buffer->is_InlineType()) {
-    return false;
-  }
-  InlineTypeNode* vt = buffer->as_InlineType();
-  if (!vt->is_allocated(&_gvn)) {
+
+  const Type* type = gvn().type(buffer);
+  if (!type->is_inlinetypeptr()) {
+    C->record_method_not_compilable("value passed to Unsafe::finishPrivateBuffer is not of a constant value type");
     return false;
   }
-  // TODO 8239003 Why is this needed?
-  if (AllocateNode::Ideal_allocation(vt->get_oop()) == nullptr) {
+
+  AllocateNode* alloc = AllocateNode::Ideal_allocation(buffer);
+  if (alloc == nullptr) {
+    C->record_method_not_compilable("value passed to Unsafe::finishPrivateBuffer must be allocated by Unsafe::makePrivateBuffer");
     return false;
   }
 
-  receiver = null_check(receiver);
+  null_check(receiver);
   if (stopped()) {
     return true;
   }
 
-  set_result(vt->finish_larval(this));
+  // Unset the larval bit in the object header
+  Node* old_header = make_load(control(), buffer, TypeX_X, TypeX_X->basic_type(), MemNode::unordered, LoadNode::Pinned);
+  Node* new_header = gvn().transform(new AndXNode(old_header, MakeConX(~markWord::larval_bit_in_place)));
+  access_store_at(buffer, buffer, type->is_ptr(), new_header, TypeX_X, TypeX_X->basic_type(), MO_UNORDERED | IN_HEAP);
+
+  // We must ensure that the buffer is properly published
+  insert_mem_bar(Op_MemBarStoreStore, alloc->proj_out(AllocateNode::RawAddress));
+  assert(!type->maybe_null(), "result of an allocation should not be null");
+  set_result(InlineTypeNode::make_from_oop(this, buffer, type->inline_klass(), false));
   return true;
 }
 

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

@@ -1697,6 +1697,28 @@ public void test83_verifier(RunInfo info) {
         }
     }
 
+    /*
+    TODO: 8335256: Properly handle merging of value object oops
+    @Test
+    @IR(failOn = {CALL_UNSAFE, ALLOC})
+    public MyValue1 test84(MyValue1 v) {
+        v = U.makePrivateBuffer(v);
+        for (int i = 0; i < 10; i++) {
+            U.putInt(v, X_OFFSET, i);
+        }
+        U.putInt(v, X_OFFSET, rI);
+        v = U.finishPrivateBuffer(v);
+        return v;
+    }
+
+    @Run(test = "test84")
+    public void test84_verifier() {
+        MyValue1 v1 = MyValue1.createWithFieldsInline(rI, rL);
+        MyValue1 v2 = test84(MyValue1.setX(v1, 0));
+        Asserts.assertEQ(v1.hash(), v2.hash());
+    }
+    */
+
     static value class MyValueClonable implements Cloneable {
         int x;