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

[Vladimir Ivanov]

On 5/5/16 10:43 PM, Vladimir Kozlov wrote:
> 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.
>
...
>>
>>> 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.

Sometimes we can. But my point is it's not always possible to 
distinguish between on-heap and off-heap accesses at compile time.

Ideally it should be Type::AnyPtr. And it seems to me it's the right 
type for such broad access. The question is how to keep other parts of 
the compiler happy.

Considering classify_unsafe_addr() returns Type::OopPtr for 
NULL+smallOffset, maybe AddPNode::Value should also produce a type with 
Type::OopPtr base?

It will make alias analysis happy for the particular case that is 
failing (NULL+12).

But it will not help with generic accesses produced in double-addressing 
mode. IMO the compiler still has to be able to convert AnyPtr to 
RawPtr/OopPtr depending on circumstances. Though we can limit such 
conversions only for unsafe accesses (AddPNode::unsafe?).

Best regards,
Vladimir Ivanov

>
> 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.