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

[Jamsheed C M]

Thank you for the feedback Aleksey. I agree, updated webrev in place[1].

Best regards,

Jamsheed

[1]http://cr.openjdk.java.net/~jcm/8237894/webrev/

On 12/02/2020 14:35, Aleksey Shipilev wrote:
> 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());
>   };
>
>