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

[Vladimir Kozlov]

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