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

[Vladimir Kozlov]

On 5/5/16 12:10 PM, Vladimir Ivanov wrote:
>>>>
>>>> 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().
>
> The type I see during compilation for "opaque" access (mixed on-heap & off-heap) is the following:
>
>    Oop:UnsafeAddP:exact+any * [narrow]
>
> LibraryCallKit::classify_unsafe_addr() produces Type::AnyPtr for such shapes. But it is used in LibraryCallKit::make_unsafe_address() just to decide on base value:

MAy be classify_unsafe_addr() should produce RawPtr type in such case.

>
> inline Node* LibraryCallKit::make_unsafe_address(Node* base, Node* offset) {
>    int kind = classify_unsafe_addr(base, offset);
>    if (kind == Type::RawPtr) {
>      return basic_plus_adr(top(), base, offset);
>    } else {
>      return basic_plus_adr(base, offset);
>    }
> }
>
> Maybe we need to enhance the logic and produce a special shape for such accesses.

Yes, I think it is good place to enhance it since we know that we do unsafe access and can generate RAW acccess.

>
>> Who produces ADDR? May be we can't set flag to indicate that it is RAW
>> address.
>
> I don't see how we can achieve that. It's just a long constant (or value) which represents an absolute address at runtime:
>
>      static final long addr = UNSAFE.allocateMemory(10);

Yes, flag will not work. But we access such addr only through UNSAFE api. So we should be able to produce RAW type for it at that point.

Thanks,
Vladimir

>
>> We should discuss it with John who is *the* expert in this.
>
> John, what are your thoughts about that?
>
> Best regards,
> Vladimir Ivanov
>
>>
>> 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.