8336778: [lworld] C2 compilation hits "invalid node class" assert whe…

…n merging object field values
TobiHartmann · Aug 13, 2024 · e30fca0 · e30fca0
1 parent 8c0dc18
commit e30fca0
Show file tree

Hide file tree

Showing 3 changed files with 100 additions and 2 deletions.
diff --git a/src/hotspot/share/opto/inlinetypenode.cpp b/src/hotspot/share/opto/inlinetypenode.cpp
@@ -74,7 +74,7 @@ InlineTypeNode* InlineTypeNode::clone_with_phis(PhaseGVN* gvn, Node* region, Saf
     // of the same type but with different scalarization depth during GVN. To avoid inconsistencies
     // during merging, make sure that we only create Phis for fields that are guaranteed to be scalarized.
     bool no_circularity = !gvn->C->has_circular_inline_type() || field_is_flat(i);
-    if (value->is_InlineType() && no_circularity) {
+    if (type->is_inlinetype() && no_circularity) {
       // Handle inline type fields recursively
       value = value->as_InlineType()->clone_with_phis(gvn, region, map);
     } else {
@@ -112,6 +112,8 @@ bool InlineTypeNode::has_phi_inputs(Node* region) {
 
 // Merges 'this' with 'other' by updating the input PhiNodes added by 'clone_with_phis'
 InlineTypeNode* InlineTypeNode::merge_with(PhaseGVN* gvn, const InlineTypeNode* other, int pnum, bool transform) {
+  assert(inline_klass() == other->inline_klass(), "Merging incompatible types");
+
   // Merge oop inputs
   PhiNode* phi = get_oop()->as_Phi();
   phi->set_req(pnum, other->get_oop());

diff --git a/src/hotspot/share/opto/inlinetypenode.hpp b/src/hotspot/share/opto/inlinetypenode.hpp
@@ -117,7 +117,7 @@ class InlineTypeNode : public TypeNode {
   void  set_is_buffered(PhaseGVN& gvn, bool buffered = true) { set_req_X(IsBuffered, gvn.intcon(buffered ? 1 : 0), &gvn); }
 
   void set_is_larval(bool is_larval) { _is_larval = is_larval; }
-  bool is_larval() { return _is_larval; }
+  bool is_larval() const { return _is_larval; }
 
   // Inline type fields
   uint          field_count() const { return req() - Values; }

diff --git a/test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestLWorld.java b/test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestLWorld.java
@@ -4387,4 +4387,100 @@ void doNothing(Object a, int i) {}
     public void testUniqueConcreteValueSubKlass_verifier() {
         testUniqueConcreteValueSubKlass(true);
     }
+
+    static value class MyValueContainer {
+        private final Object value;
+
+        private MyValueContainer(Object value) {
+            this.value = value;
+        }
+    }
+
+    static value class MyValue161 {
+        int x = 0;
+    }
+
+    @Test
+    public MyValueContainer test161(boolean b) {
+        MyValueContainer res = b ? new MyValueContainer(new MyValue161()) : null;
+        // Cast to verify that merged values are of correct type
+        Object obj = b ? (MyValue161)res.value : null;
+        return res;
+    }
+
+    @Run(test = "test161")
+    public void test161_verifier() {
+        Asserts.assertEquals(test161(true), new MyValueContainer(new MyValue161()));
+        Asserts.assertEquals(test161(false), null);
+    }
+
+    @Test
+    public MyValueContainer test162(boolean b) {
+        MyValueContainer res = b ? null : new MyValueContainer(new MyValue161());
+        // Cast to verify that merged values are of correct type
+        Object obj = b ? null : (MyValue161)res.value;
+        return res;
+    }
+
+    @Run(test = "test162")
+    public void test162_verifier() {
+        Asserts.assertEquals(test162(true), null);
+        Asserts.assertEquals(test162(false), new MyValueContainer(new MyValue161()));
+    }
+
+    @Test
+    public MyValueContainer test163(boolean b) {
+        MyValueContainer res = b ? new MyValueContainer(new MyValue161()) : new MyValueContainer(null);
+        // Cast to verify that merged values are of correct type
+        Object obj = b ? (MyValue161)res.value : (MyValue161)res.value;
+        return res;
+    }
+
+    @Run(test = "test163")
+    public void test163_verifier() {
+        Asserts.assertEquals(test163(true), new MyValueContainer(new MyValue161()));
+        Asserts.assertEquals(test163(false), new MyValueContainer(null));
+    }
+
+    @Test
+    public MyValueContainer test164(boolean b) {
+        MyValueContainer res = b ? new MyValueContainer(null) : new MyValueContainer(new MyValue161());
+        // Cast to verify that merged values are of correct type
+        Object obj = b ? (MyValue161)res.value : (MyValue161)res.value;
+        return res;
+    }
+
+    @Run(test = "test164")
+    public void test164_verifier() {
+        Asserts.assertEquals(test164(true), new MyValueContainer(null));
+        Asserts.assertEquals(test164(false), new MyValueContainer(new MyValue161()));
+    }
+
+    @Test
+    public MyValueContainer test165(boolean b) {
+        MyValueContainer res = b ? new MyValueContainer(new MyValue161()) : new MyValueContainer(42);
+        // Cast to verify that merged values are of correct type
+        Object obj = b ? (MyValue161)res.value : (Integer)res.value;
+        return res;
+    }
+
+    @Run(test = "test165")
+    public void test165_verifier() {
+        Asserts.assertEquals(test165(true), new MyValueContainer(new MyValue161()));
+        Asserts.assertEquals(test165(false), new MyValueContainer(42));
+    }
+
+    @Test
+    public MyValueContainer test166(boolean b) {
+        MyValueContainer res = b ? new MyValueContainer(42) : new MyValueContainer(new MyValue161());
+        // Cast to verify that merged values are of correct type
+        Object obj = b ? (Integer)res.value : (MyValue161)res.value;
+        return res;
+    }
+
+    @Run(test = "test166")
+    public void test166_verifier() {
+        Asserts.assertEquals(test166(true), new MyValueContainer(42));
+        Asserts.assertEquals(test166(false), new MyValueContainer(new MyValue161()));
+    }
 }