RFR(L): 8031755: Type speculation should be used to optimize explicit null checks

[Roland Westrelin]

Hi Vladimir,

Thanks for taking a look at this.

>> http://cr.openjdk.java.net/~roland/8031755/webrev.00/
>>
>> Most Type profiling points record whether a null reference was seen
>> but type speculation doesn’t currently take advantage of it. With
>> this change, not only is the type seen at a profiling point feed to
>> type speculation but also whether a null pointer was seen or
>> not. This is then used to optimize null checks.
>>
>> The _speculative field becomes:
>> const TypePtr*   _speculative;
>>
>> so that if all we know from profiling is that a reference is not null:
>> _speculative = TypePtr::NOT NULL;
>>
>> When a type is met with a TypePtr (NULL_PTR for instance), the meet
>> must be applied to the speculative types as well. To keep the type
>> system symmetric, the _speculative field had to be moved to class
>> TypePtr. I also moved the _inline_depth there to keep all speculative
>> stuff together but with the current code it could have stayed in
>> class TypeOopPtr.
>>
>> Traps caused by a failed speculative null check are recorded with
>> speculative traps, similarly to what is done for a failed class
>> check.
>
> The idea is good, I think, but I need time to go through changes.
>
> What about changes in arguments.cpp and phaseX.cpp?

In c2_globals.hpp

  product(intx, MaxNodeLimit, 80000,                                        \
          "Maximum number of nodes")                                        \

So today, we're decreasing the number of nodes if incremental inlining is on.

In phaseX.cpp, the problem is that during an IGVN, some nodes that
become disconnected from the graph are not removed from the IGVN's hash
table. It's a bit ugly but maybe good enough for some verification code?

> I don't understand why you changed higher_equal_speculative() in
> parse2.cpp.

I added a cleanup_speculative() to Type that removes the speculative
type if it's useless (not an exact klass for instance). So:

1289             TypeNode* ccast = new (C) CheckCastPPNode(control(), obj, tboth);
1290             const Type* tcc = ccast->as_Type()->type();
1291             assert(tcc != obj_type && tcc->higher_equal(obj_type), "must improve");

The speculative type may disappear and
tcc->higher_equal_speculative(obj_type) is no longer true.

>> I made the following change:
>>
>> *** 3561,3571 ****
>>
>>
>>        // Since klasses are different, we require a LCA in the Java
>>        // class hierarchy - which means we have to fall to at least NotNull.
>>        if( ptr == TopPTR || ptr == AnyNull || ptr == Constant )
>>          ptr = NotNull;
>>
>> -     instance_id = InstanceBot;
>>
>>
>>        // Now we find the LCA of Java classes
>>        ciKlass* k = this_klass->least_common_ancestor(tinst_klass);
>>        return make(ptr, k, false, NULL, off, instance_id, speculative, depth);
>>      } // End of case InstPtr
>>
>> --- 3706,3715 ——
>>
>> because I hit a type not symmetric failure that I think it causes:
>>
>> === Meet Not Symmetric ===
>> t   =                   javax/management/openmbean/OpenType:AnyNull * (inline_depth=-2)
>> this=                   javax/management/openmbean/ArrayType:TopPTR *,iid=top (inline_depth=InlineDepthTop)
>> mt=(t meet this)=       javax/management/openmbean/ArrayType:AnyNull * (inline_depth=-2)
>> t_dual=                 javax/management/openmbean/OpenType:NotNull *,iid=top (inline_depth=2)
>> this_dual=              javax/management/openmbean/ArrayType *
>> mt_dual=                javax/management/openmbean/ArrayType:NotNull *,iid=top (inline_depth=2)
>> mt_dual meet t_dual=    javax/management/openmbean/OpenType:NotNull * (inline_depth=2)
>> mt_dual meet this_dual= javax/management/openmbean/ArrayType *
>
> The question is why OpenType:AnyNull does not have iid=top. It is in 
> upper type lattice.

I'll see if I can come up with a test case for this one.

Roland.