[15] RFR: 8237894: CTW: C1 compilation fails with assert(x->type()->tag() == f->type()->tag()) failed: should have same type

[Aleksey Shipilev]

On 2/12/20 8:12 AM, Jamsheed C M wrote:
> Request for review the code change made for disabling value numbering 
> for always null access instruction in c1 code. value numbering these 
> instructions was causing asserts while processing dead code paths in 
> debug builds.
> 
> webrev: http://cr.openjdk.java.net/~jcm/8237894/webrev/

I have no strong preference, but I wonder if this flows better:

diff -r 396a47e0860f src/hotspot/share/c1/c1_Instruction.hpp
--- a/src/hotspot/share/c1/c1_Instruction.hpp   Tue Feb 11 15:23:28 2020 +0000
+++ b/src/hotspot/share/c1/c1_Instruction.hpp   Wed Feb 12 10:03:06 2020 +0100
@@ -455,4 +455,6 @@
   bool can_be_linked()                           { return as_Local() == NULL && as_Phi() == NULL; }

+  bool is_null_obj()                             { return as_Constant() != NULL &&
type()->as_ObjectType()->constant_value()->is_null_object(); }
+
   bool has_uses() const                          { return use_count() > 0; }
   ValueStack* state_before() const               { return _state_before; }
@@ -835,6 +837,6 @@
   ciType* declared_type() const;

-  // generic
-  HASHING2(LoadField, !needs_patching() && !field()->is_volatile(), obj()->subst(), offset())  //
cannot be eliminated if needs patching or if volatile
+  // generic; cannot be eliminated if needs patching or if volatile or if null object
+  HASHING2(LoadField, !needs_patching() && !field()->is_volatile() &&
!obj()->subst()->is_null_obj(), obj()->subst(), offset())
 };

@@ -965,6 +967,6 @@
   ciType* declared_type() const;

-  // generic
-  HASHING2(LoadIndexed, true, array()->subst(), index()->subst())
+  // generic; disabled if _array is null
+  HASHING2(LoadIndexed, !array()->subst()->is_null_obj(), array()->subst(), index()->subst());
 };


-- 
Thanks,
-Aleksey