[9] RFR (XS): 8155635: C2: assert(flat != TypePtr::BOTTOM) failed: cannot alias-analyze an untyped ptr

[Vladimir Kozlov]

On 4/29/16 5:24 PM, Vladimir Ivanov wrote:
> Thanks for the feedback, Vladimir.
>
> On 4/30/16 2:39 AM, Vladimir Kozlov wrote:
>> I am not comfortable with this fix. You may replace in(Base) != NULL
>> with TOP.
> Do you see any cases when it is possible?
> I don't see any sense in an absolute address with a valid heap base.
>
> I can check that in(Base) == in(Address) == NULL. It will fix the problem as well.
>
>> Also it should not be RAW pointer (TOP as Base) if it is created by
>> graph transformation from normal oop pointer.
>> I think we should track which pointers are really RAW when creating them.
>>
>> Can you explain why we have such graph shape where we access memory
>> after a merge point and on one merged path has NULL as pointer to
>> object. There should be NULL check after merge before memory access in
>> such case.
> It's not necessarily a normal oop pointer. Double-register addressing mode is the source of such shapes. Consider the following example:
>
>    Object o = (flag ? INSTANCE : null);
>    long off = (flag ? F_OFFSET : ADDR);
>    UNSAFE.getLong(o, off);

I think for this graph shape C2 type system gave up and drops type to general Ptr::BOTTOM because it does not know that 'off' can be address and not a normal offset on dead path where base is NULL. We 
usually do:

long l = flag ? o.field : UNSAFE.getLong(addr);
And for UNSAFE.getLong(addr) we generate Raw pointer address (make_unsafe_address()).

What I am saying is that C2 treat long value as address only when it was used as direct parameter for unsafe. See LibraryCallKit::classify_unsafe_addr().

Who produces ADDR? May be we can't set flag to indicate that it is RAW address.

We should discuss it with John who is *the* expert in this.

Thanks,
Vladimir

>
> is translated into:
>
>    LoadL mem (AddP (Phi #NULL #NonNull) off)
>
> If such AddP is split through the Phi, it turns into (AddP #NULL #NULL off) and (AddP #NonNull #NonNull off). The former is untyped and causes problems later.
>
> What I can't replicate is how X-shaped control flow eligible for SplitIf transformation is produced.
>
> In the failing case, initial null & exact type checks of an oop local (on OSR entry) merge into redundant X-shaped block. Unsafe accesses uses the local as a base later.
>
> Best regards,
> Vladimir Ivanov
>
>>
>> On 4/29/16 4:11 PM, Vladimir Ivanov wrote:
>>> http://cr.openjdk.java.net/~vlivanov/8155635/webrev.00/
>>> https://bugs.openjdk.java.net/browse/JDK-8155635
>>>
>>> SplitIf transformation can produce untyped pointers when slitting AddP
>>> nodes for unsafe accesses through a Phi which
>>> merges non-null & null values:
>>>     AddP ... (Phi (ConP #NULL) (CheckCastPP Oop:...:NotNull))
>>>
>>> Proposed fix is to enable oop pointer to raw pointer conversion for
>>> absolute addresses.
>>>
>>> I also experimented with blocking SplitIf transformation is such
>>> cases, but the transformation seems viable and
>>> considerably simplifies the graph: X-shaped control flow is untangled
>>> by eliminating redundant and the transformation
>>> sharpens types on both branches.
>>>
>>> I checked specifically how Phi merges raw & oop pointers after the
>>> split and it works fine.
>>>
>>> Testing: failing test, JPRT, RBT (hs-tier0-comp.js).
>>>
>>> Thanks!
>>>
>>> Best regards,
>>> Vladimir Ivanov
>>>
>>> PS: though AddP (Phi #NULL #NotNull) shape is common, I wasn't able to
>>> write a simplified test case which triggers
>>> SplitIf transformation.