[10] RFR(S): 8188785: CCP sets invalid type for java mirror load

[Vladimir Kozlov]

Hmm. Is this only LoadP or general problem?

May be add code to next lines when m->is_AddP() :

1734         if (m->bottom_type() != type(m)) { // If not already 
bottomed out
1735           worklist.push(m);     // Propagate change to user

I think we should do similar to PhaseIterGVN::add_users_to_worklist().

Thanks,
Vladimir

On 10/20/17 1:04 AM, Tobias Hartmann wrote:
> Hi,
> 
> please review the following patch:
> https://bugs.openjdk.java.net/browse/JDK-8188785
> http://cr.openjdk.java.net/~thartmann/8188785/webrev.00/
> 
> Since 8186777 [1], we require two loads to retrieve the java mirror from 
> a klass oop:
> 
> LoadP(LoadP(AddP(klass_oop, java_mirror_offset)))
> 
> The problem is that now the type of the outermost LoadP does not depend 
> on the inner LoadP (which has a raw pointer type) but on the type of the 
> AddP which is one level up. CPP only propagates the types downwards to 
> the direct users and as a result, the mirror LoadP ends up with an 
> incorrect (too narrow/optimistic) type.
> 
> I've verified the fix with the failing test and also verified that 
> 8188835 [2] is a duplicate.
> 
> Gory details:
> During CCP, we compute the type of a Phi that merges oops of type A and 
> B where B is a subtype of A. Since the type of the A input was not 
> computed yet (it was initialized to TOP at the beginning of CCP), the 
> Phi temporarily ends up with type B (i.e. with a type that is too 
> narrow/optimistic). This type is propagated downwards and is being used 
> to optimize a java mirror load from the klass oop:
> 
> LoadP(LoadP(AddP(DecodeNKlass(LoadNKlass(AddP(CastPP(Phi)))))))
> 
> The mirror load is then folded to TypeInstPtr::make(B) which is not 
> correct because the oop can be of type A at runtime.
> 
> Thanks,
> Tobias
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8186777
> [2] https://bugs.openjdk.java.net/browse/JDK-8188835