Fixed even more ToDos

src/hotspot/share/c1/c1_LIRGenerator.cpp

@@ -2086,7 +2086,7 @@ bool LIRGenerator::inline_type_field_access_prolog(AccessField* x) {
                        !x->is_static() && x->needs_patching();
   // Deoptimize if we load from a static field with an uninitialized type because we
   // need to throw an exception if initialization of the type failed.
-  // TODO it seems that this fix for JDK-8273594 is not needed anymore or our tests don't trigger the issue anymore
+  // TODO 8324949 It seems that this fix for JDK-8273594 is not needed anymore or our tests don't trigger the issue anymore
   bool not_initialized = false && x->is_static() && x->as_LoadField() != nullptr &&
       !field->type()->as_instance_klass()->is_initialized();
   if (could_be_flat || not_initialized) {

src/hotspot/share/opto/inlinetypenode.cpp

@@ -299,9 +299,9 @@ void InlineTypeNode::make_scalar_in_safepoints(PhaseIterGVN* igvn, bool allow_oo
   // If the inline type has a constant or loaded oop, use the oop instead of scalarization
   // in the safepoint to avoid keeping field loads live just for the debug info.
   Node* oop = get_oop();
-  // TODO TestBasicFunctionality::test3 fails without this
-  // TODO add proj nodes here?
-  // TODO recursive handling of phis required? we need a test that fails without
+  // TODO 8324949
+  // TestBasicFunctionality::test3 fails without this. Add more tests?
+  // Add proj nodes here? Recursive handling of phis required? We need a test that fails without.
   bool use_oop = false;
   if (allow_oop && is_allocated(igvn) && oop->is_Phi()) {
     Unique_Node_List worklist;

src/hotspot/share/opto/library_call.cpp

@@ -4422,8 +4422,8 @@ Node* LibraryCallKit::generate_array_guard_common(Node* kls, RegionNode* region,
 //-----------------------inline_newNullRestrictedArray--------------------------
 // public static native Object[] newNullRestrictedArray(Class<?> componentType, int length);
 bool LibraryCallKit::inline_newNullRestrictedArray() {
-  // TODO improve this
-  // TODO add required checks
+  // TODO 8324949
+  // Improve this and add required runtime checks
   Node* componentType = argument(0);
   Node* length = argument(1);
 
@@ -4437,7 +4437,6 @@ bool LibraryCallKit::inline_newNullRestrictedArray() {
         ciArrayKlass* array_klass = ciArrayKlass::make(t, true);
         if (array_klass->is_loaded() && array_klass->element_klass()->as_inline_klass()->is_initialized()) {
           const TypeKlassPtr* array_klass_type = TypeKlassPtr::make(array_klass, Type::trust_interfaces);
-
           Node* obj = new_array(makecon(array_klass_type), length, 0);  // no arguments to push
           set_result(obj);
           return true;
@@ -4446,26 +4445,6 @@ bool LibraryCallKit::inline_newNullRestrictedArray() {
     }
   }
   return false;
-
-  //_gvn.type(componentType)->is_klassptr()->
-
-  /*
-  // Check that array_klass object is loaded
-  if (!array_klass->is_loaded()) {
-    // Generate uncommon_trap for unloaded array_class
-    uncommon_trap(Deoptimization::Reason_unloaded,
-                  Deoptimization::Action_reinterpret,
-                  array_klass);
-    return;
-  } else if (array_klass->element_klass() != nullptr &&
-             array_klass->element_klass()->is_inlinetype() &&
-             !array_klass->element_klass()->as_inline_klass()->is_initialized()) {
-    uncommon_trap(Deoptimization::Reason_uninitialized,
-                  Deoptimization::Action_reinterpret,
-                  nullptr);
-    return;
-  }
-  */
 }
 
 //-----------------------inline_native_newArray--------------------------

src/hotspot/share/opto/type.cpp

@@ -2180,7 +2180,7 @@ const TypeTuple *TypeTuple::LONG_CC_PAIR;
 static void collect_inline_fields(ciInlineKlass* vk, const Type** field_array, uint& pos) {
   for (int j = 0; j < vk->nof_nonstatic_fields(); j++) {
     ciField* field = vk->nonstatic_field_at(j);
-    // TODO the field could be null free, right?
+    // TODO 8324949 The field could be null free, right? Shouldn't we set the type to null-free here?
     BasicType bt = field->type()->basic_type();
     const Type* ft = Type::get_const_type(field->type());
     field_array[pos++] = ft;

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

@@ -214,10 +214,10 @@ public void test8_verifier() {
         counts = {ALLOC, "= 1", LOAD, "= 19",
                   STORE, "= 3"}, // InitializeNode::coalesce_subword_stores merges stores
         failOn = {TRAP})
-// TODO fix
-//    @IR(applyIf = {"InlineTypePassFieldsAsArgs", "false"},
-//        counts = {ALLOC, "= 2", STORE, "= 19"},
-//        failOn = {LOAD, TRAP})
+    // TODO 8324949
+    // @IR(applyIf = {"InlineTypePassFieldsAsArgs", "false"},
+    //     counts = {ALLOC, "= 2", STORE, "= 19"},
+    //     failOn = {LOAD, TRAP})
     public MyValue1 test9(boolean b, int localrI, long localrL) {
         MyValue1 v;
         if (b) {

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

@@ -1778,8 +1778,6 @@ public void test86_verifier() {
         Asserts.assertEQ(result, n);
     }
 
-// TODO is this now obsolete?
-
     //-------------------------------------------------------------------------------
     // Tests for how C1 handles InlineTypeReturnedAsFields in both calls and returns
     //-------------------------------------------------------------------------------

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

@@ -839,14 +839,6 @@ public void test43_verifier() {
         Asserts.assertEQ(result, null);
         result = test43(MyValue1.class, vt);
         Asserts.assertEQ(result, vt);
-        // TODO can not cast to null free
-        /*
-        try {
-            test43(MyValue1.class, null);
-            throw new RuntimeException("should have thrown");
-        } catch (NullPointerException npe) {
-        }
-        */
         result = test43(Integer.class, null);
         Asserts.assertEQ(result, null);
     }
@@ -871,14 +863,6 @@ public void test44_verifier() {
             throw new RuntimeException("should have thrown");
         } catch (ClassCastException cce) {
         }
-        // TODO can not cast to null free
-        /*
-        try {
-            test44(MyValue2.class, null);
-            throw new RuntimeException("should have thrown");
-        } catch (NullPointerException npe) {
-        }
-        */
     }
 
     @Test
@@ -922,14 +906,8 @@ public void test47_verifier() {
         MyValue1 vt = MyValue1.createWithFieldsInline(rI, rL);
         Object result = test47(vt);
         Asserts.assertEQ(((MyValue1)result).hash(), vt.hash());
-        // TODO can not cast to null free
-        /*
-        try {
-            test47(null);
-            throw new RuntimeException("should have thrown");
-        } catch (NullPointerException npe) {
-        }
-        */
+        result = test47(null);
+        Asserts.assertEQ(result, null);
     }
 
     @Test
@@ -942,14 +920,8 @@ public void test48_verifier() {
         MyValue1 vt = MyValue1.createWithFieldsInline(rI, rL);
         Object result = test48(MyValue1.class, vt);
         Asserts.assertEQ(((MyValue1)result).hash(), vt.hash());
-        // TODO can not cast to null free
-        /*
-        try {
-            test48(MyValue1.class, null);
-            throw new RuntimeException("should have thrown");
-        } catch (NullPointerException npe) {
-        }
-        */
+        result = test48(MyValue1.class, null);
+        Asserts.assertEQ(result, null);
     }
 
     @Test
@@ -984,19 +956,13 @@ public void test50_verifier() {
         Asserts.assertEQ(result, vba);
         result = test50(MyValue1[].class, va);
         Asserts.assertEQ(result, va);
-        // TODO can not cast to null free
-        /*
-        try {
-            test50(MyValue1.class, null);
-            throw new RuntimeException("should have thrown");
-        } catch (NullPointerException npe) {
-        }
+        result = test50(MyValue1.class, null);
+        Asserts.assertEQ(result, null);
         try {
-            test50(MyValue1[].class, vba);
+            test50(va.getClass(), vba);
             throw new RuntimeException("should have thrown");
         } catch (ClassCastException cce) {
         }
-        */
     }
 
     // Value class array creation via reflection
@@ -1017,7 +983,6 @@ public void test51_verifier() {
     // multidimensional value class array creation via reflection
     @Test
     public Object[][] test52(int len, int val) {
-// TODO
         MyValue1[][] va1 = (MyValue1[][])Array.newInstance(MyValue1[].class, len);
         MyValue1[][] va2 = (MyValue1[][])Array.newInstance(MyValue1[].class, len);
         Object[][] result;
@@ -1045,7 +1010,6 @@ public void test52_verifier() {
 
     @Test
     public Object[][] test53(Class<?> c1, Class<?> c2, int len, int val) {
-    // TODO
         MyValue1[][] va1 = (MyValue1[][])Array.newInstance(MyValue1[].class, len);
         MyValue1[][] va2 = (MyValue1[][])Array.newInstance(MyValue1[].class, len);
         Object[][] va3 = (Object[][])Array.newInstance(c1, len);

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

@@ -4176,7 +4176,7 @@ public void test150_verifier() {
         Asserts.assertEquals(test150(), testValue2.hash());
     }
 
-// TODO triggers #  assert(false) failed: Should have been buffered
+// TODO 8324949 This triggers #  assert(false) failed: Should have been buffered
 /*
     // Same as test150 but with val not being allocated in the scope of the method
     @Test

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

@@ -901,7 +901,7 @@ public void test28_verifier() {
     // non escaping allocations
     // TODO 8227588: shouldn't this have the same IR matching rules as test6?
     @Test
-    // TODO Currently disabled in LibraryCallKit::arraycopy_restore_alloc_state
+    // TODO 8324949 Currently disabled in LibraryCallKit::arraycopy_restore_alloc_state
     //@IR(failOn = {ALLOCA, LOOP, TRAP})
     public MyValue2 test29(MyValue2[] src) {
         MyValue2[] dst = new MyValue2[10];

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

@@ -2376,8 +2376,8 @@ public Object test85_helper(Object obj, int i) {
 
     // Same as test81 but with wrapper
     @Test
-    // TODO fix, fails with Scenario 5
-//    @IR(failOn = {ALLOC, LOAD, STORE})
+    // TODO 8324949 Fails with Scenario 5
+    // @IR(failOn = {ALLOC, LOAD, STORE})
     public long test85() {
         Object val = new MyValue1Wrapper(null);
         for (int i = 0; i < 10; ++i) {
@@ -2806,7 +2806,7 @@ public CircularValue2(CircularValue1 val) {
         }
     }
 
-// TODO fix assert in InlineTypeNode::merge_with
+    // TODO 8324949 fix assert in InlineTypeNode::merge_with
 /*
     // Same as test98 but with circularity in class of flattened field
     @Test

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2019, 2024, 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
@@ -23,8 +23,6 @@
 
 /**
  * @test
- // TODO why is this disabled with ZGC?
- * @requires vm.gc != "Z"
  * @bug 8233415
  * @summary Verify that TLAB allocated buffer initialization when returning a value object works properly with oops.
  * @library /test/lib