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

[Vladimir Kozlov]

On 10/24/17 12:33 AM, Tobias Hartmann wrote:
> Hi Vladimir,
> 
> On 23.10.2017 18:19, Vladimir Kozlov wrote:
>> I think we need to file a bug or rfe to fix other cases too.
> 
> Okay, it's difficult to file a bug for a not yet known issue so I've filed an RFE to look into this:
> https://bugs.openjdk.java.net/browse/JDK-8189856

Good. RFE is fine.

> 
>> In add_users_to_worklist() you don't need to check type ut !=
>>  > type(u) - just push node on worklist.
> 
> Right, fixed:
> http://cr.openjdk.java.net/~thartmann/8188785/webrev.02/

Okay, you are right, lets use this version for the fix. We can do additional changes for 8189856.

Thanks,
Vladimir

> 
>> Also can you remove ut->isa_instptr() check in *both* cases. And use u->is_Mem() instead of u->Opcode() == Op_LoadP to 
>> cover stores too.
>> The motivation is that original LoadP is raw as result memory operations which use it may look for more precise type 
>> of the field somewhere so they should be on worklist.
> 
> Why is that necessary? If the raw LoadP changes its type, all direct users will be added to the worklist anyway.
> 
> The problem in the failing case is that the type of the AddP changed but the type of the raw LoadP didn't (it stays 
> raw). However, the InstPtr load depends on the type of the AddP:
> 
>    InstPtrLoadP(RawLoadP(AddP(..)))
> 
> Do you expect other memory users of the raw LoadP to depend on the type of the AddP? I think we should only add handling 
> for known special cases but here's the corresponding webrev:
> http://cr.openjdk.java.net/~thartmann/8188785/webrev.03/
> 
> Thanks,
> Tobias