RFR(L): 8024070: C2 needs some form of type speculation

[Vladimir Kozlov]

Roland,

I talked with John about this. He is proposing extension to C2 type system to shift from points (in type lattice) to 
type sets/ranges. Your changes is move in this direction but I am concern about stability, especially for jdk8.
Adding 2nd type to CastPP will not work also.

Can you try an other experiment (may be you did it already)? Always (under flag) generate klass check 
(type_check_receiver()) when unique type from profiling is more precise than type from static analysis. Yes, we may get 
performance regression in cases where more precise type is not needed. We need to test it and see how bad it is.

Thanks
Vladimir

On 9/5/13 5:14 PM, Vladimir Kozlov wrote:
> On 9/5/13 7:16 AM, Roland Westrelin wrote:
>> Hi Vladimir,
>>
>> Thanks for looking at this.
>>
>>> Did you consider using second type table PhaseTransform::_speculative_types instead of using CastPP and modified
>>> TypeOopPtr (_speculative)? I may be can accept CastPP (it can cause problems not only to stringOpts) but I don't like
>>> the idea that one type can represent 2 types.
>>
>> Embedding a _speculative field in the TypeOopPtr class came after some discussion with John. He did seem to like this
>> idea ;-) I like it too.
>>
>> I did think about another table.
>> The current type flows in the graph through calls to Node::Value(). If the (i)GVN has to update 2 type tables,
>> Node::Value() still returns one type. Do we make Node::Value() update the speculative type table? Node::Value() has no
>> side effect currently so that looks ugly. Do we want a Node::Value_speculative()? Node::Value(const TypeOopPtr*&
>> speculative) to return 2 values?
>>
>> The current change makes heavy changes to type.[ch]pp but they are mostly straightforward changes and it keeps the
>> rest untouched except where we inject or retrieve speculative types.
>>
>> Also, having another table doesn't remove the need for the CastPP nodes, right? The new type information is control
>> dependent: for instance, we might get profiling at a call which is in a if branch. For all we know, this new type info
>> is only valid in the rest of the branch.
>>
>> And one type doesn't represent 2 types. It represent what we know for sure about the type and we speculatively think
>> we know about the type. The new type info complements what we know about this node's type. That TypeOopPtr has a
>> pointer to another TypeOopPtr is an artifact of the implementation that makes it simple. It could maybe simply be
>> another ciKlass reference but then more refactoring is needed.
>>
>> Roland.
>>
>
> I still think that speculative type should not be associated with another type. Original type could be used by accident
> for an other ideal node and you are screwed. I would prefer to see if not a table but may be additional field
> _speculative_type in CastPP node. Actually it should be CheckCastPP node because CastPP is mostly used only for
> NOT_NULLness casts. You would need to modify its transformation methods (Ideal(), etc). But I think it will be more safe
> than touching C2 type system.
>
> Vladimir