Quantcast

RFR(XS) 8178047: Aliasing problem with raw memory accesses

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

RFR(XS) 8178047: Aliasing problem with raw memory accesses

Igor Veresov
This seems like a long-standing bug. Analysis in MemNode::find_previous_store() tries to relax memory dependencies by proving that memory accesses don’t alias. The code is fine for oops: if [offset, offset+length) intervals don’t overlap it proves the accesses don’t alias because bases always point to the start of an object. For raw accesses that’s not true. Offset analysis doesn’t mean much without proving that bases are not the same.

Webrev: http://cr.openjdk.java.net/~iveresov/8178047/webrev.00

Thanks,
igor
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(XS) 8178047: Aliasing problem with raw memory accesses

Andrew Haley
On 14/04/17 04:30, Igor Veresov wrote:
> This seems like a long-standing bug. Analysis in MemNode::find_previous_store() tries to relax memory dependencies by proving that memory accesses don’t alias. The code is fine for oops: if [offset, offset+length) intervals don’t overlap it proves the accesses don’t alias because bases always point to the start of an object. For raw accesses that’s not true. Offset analysis doesn’t mean much without proving that bases are not the same.
>
> Webrev: http://cr.openjdk.java.net/~iveresov/8178047/webrev.00

Please make sure that the test in https://bugs.openjdk.java.net/browse/JDK-8176513
doesn't regress again.

Thanks,

Andrew.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(XS) 8178047: Aliasing problem with raw memory accesses

Vladimir Kozlov
In reply to this post by Igor Veresov
Klass pointers should be also fine I think.
The problem is only Raw pointers which could point to the same memory but different offset.
May be use

bool is_raw = adr->bottom_type()->isa_rawptr() != NULL;
...
if (is_raw && st_base != base) {

Also what if pointer type is mixed: original is oop and mem node is raw?
May we should check that if when one of them is raw we should compare bases.

Thanks,
Vladimir

On 4/13/17 8:30 PM, Igor Veresov wrote:
> This seems like a long-standing bug. Analysis in MemNode::find_previous_store() tries to relax memory dependencies by proving that memory accesses don’t alias. The code is fine for oops: if [offset, offset+length) intervals don’t overlap it proves the accesses don’t alias because bases always point to the start of an object. For raw accesses that’s not true. Offset analysis doesn’t mean much without proving that bases are not the same.
>
> Webrev: http://cr.openjdk.java.net/~iveresov/8178047/webrev.00
>
> Thanks,
> igor
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(XS) 8178047: Aliasing problem with raw memory accesses

Igor Veresov

> On Apr 14, 2017, at 9:53 AM, Vladimir Kozlov <[hidden email]> wrote:
>
> Klass pointers should be also fine I think.
> The problem is only Raw pointers which could point to the same memory but different offset.
> May be use
>
> bool is_raw = adr->bottom_type()->isa_rawptr() != NULL;
> ...
> if (is_raw && st_base != base) {

I guess in that case AnyPtr should be be also checked for, because it includes RawPtr, right?

>
> Also what if pointer type is mixed: original is oop and mem node is raw?
> May we should check that if when one of them is raw we should compare bases.

Is it possible for that to happen?

igor

>
> Thanks,
> Vladimir
>
> On 4/13/17 8:30 PM, Igor Veresov wrote:
>> This seems like a long-standing bug. Analysis in MemNode::find_previous_store() tries to relax memory dependencies by proving that memory accesses don’t alias. The code is fine for oops: if [offset, offset+length) intervals don’t overlap it proves the accesses don’t alias because bases always point to the start of an object. For raw accesses that’s not true. Offset analysis doesn’t mean much without proving that bases are not the same.
>>
>> Webrev: http://cr.openjdk.java.net/~iveresov/8178047/webrev.00
>>
>> Thanks,
>> igor
>>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(XS) 8178047: Aliasing problem with raw memory accesses

Igor Veresov
In reply to this post by Andrew Haley

> On Apr 14, 2017, at 9:44 AM, Andrew Haley <[hidden email]> wrote:
>
> On 14/04/17 04:30, Igor Veresov wrote:
>> This seems like a long-standing bug. Analysis in MemNode::find_previous_store() tries to relax memory dependencies by proving that memory accesses don’t alias. The code is fine for oops: if [offset, offset+length) intervals don’t overlap it proves the accesses don’t alias because bases always point to the start of an object. For raw accesses that’s not true. Offset analysis doesn’t mean much without proving that bases are not the same.
>>
>> Webrev: http://cr.openjdk.java.net/~iveresov/8178047/webrev.00
>
> Please make sure that the test in https://bugs.openjdk.java.net/browse/JDK-8176513
> doesn't regress again.


It might be a problem with a test attached to the bug (it also needs b.capacity() / 4 in floss(), otherwise it would throw), but I don’t see vectorization happening at all. With or without my fix.

igor

>
> Thanks,
>
> Andrew.
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(XS) 8178047: Aliasing problem with raw memory accesses

Vladimir Kozlov
In reply to this post by Igor Veresov
On 4/14/17 10:31 AM, Igor Veresov wrote:

>
>> On Apr 14, 2017, at 9:53 AM, Vladimir Kozlov <[hidden email]> wrote:
>>
>> Klass pointers should be also fine I think.
>> The problem is only Raw pointers which could point to the same memory but different offset.
>> May be use
>>
>> bool is_raw = adr->bottom_type()->isa_rawptr() != NULL;
>> ...
>> if (is_raw && st_base != base) {
>
> I guess in that case AnyPtr should be be also checked for, because it includes RawPtr, right?

Yes.

>
>>
>> Also what if pointer type is mixed: original is oop and mem node is raw?
>> May we should check that if when one of them is raw we should compare bases.
>
> Is it possible for that to happen?

May be not. Usually we will have membars around unsafe memory access. But if we have AnyPtr, as you said, it could be
mixed I think. I think it is better to check for such case even if it may not happen.

Vladimir

>
> igor
>
>>
>> Thanks,
>> Vladimir
>>
>> On 4/13/17 8:30 PM, Igor Veresov wrote:
>>> This seems like a long-standing bug. Analysis in MemNode::find_previous_store() tries to relax memory dependencies by proving that memory accesses don’t alias. The code is fine for oops: if [offset, offset+length) intervals don’t overlap it proves the accesses don’t alias because bases always point to the start of an object. For raw accesses that’s not true. Offset analysis doesn’t mean much without proving that bases are not the same.
>>>
>>> Webrev: http://cr.openjdk.java.net/~iveresov/8178047/webrev.00
>>>
>>> Thanks,
>>> igor
>>>
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(XS) 8178047: Aliasing problem with raw memory accesses

Andrew Haley
In reply to this post by Igor Veresov
On 14/04/17 19:04, Igor Veresov wrote:
> It might be a problem with a test attached to the bug (it also needs b.capacity() / 4 in floss(), otherwise it would throw), but I don’t see vectorization happening at all. With or without my fix.

Lack of vectorization is OK for now: the thing I'm trying to ensure
is that it doesn't get worse.

Thanks,

Andrew.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(XS) 8178047: Aliasing problem with raw memory accesses

Igor Veresov
In reply to this post by Vladimir Kozlov
I’ve updated the webrev: http://cr.openjdk.java.net/~iveresov/8178047/webrev.01
I did an RBT run and the results look good.

igor


> On Apr 14, 2017, at 11:21 AM, Vladimir Kozlov <[hidden email]> wrote:
>
> On 4/14/17 10:31 AM, Igor Veresov wrote:
>>
>>> On Apr 14, 2017, at 9:53 AM, Vladimir Kozlov <[hidden email]> wrote:
>>>
>>> Klass pointers should be also fine I think.
>>> The problem is only Raw pointers which could point to the same memory but different offset.
>>> May be use
>>>
>>> bool is_raw = adr->bottom_type()->isa_rawptr() != NULL;
>>> ...
>>> if (is_raw && st_base != base) {
>>
>> I guess in that case AnyPtr should be be also checked for, because it includes RawPtr, right?
>
> Yes.
>
>>
>>>
>>> Also what if pointer type is mixed: original is oop and mem node is raw?
>>> May we should check that if when one of them is raw we should compare bases.
>>
>> Is it possible for that to happen?
>
> May be not. Usually we will have membars around unsafe memory access. But if we have AnyPtr, as you said, it could be mixed I think. I think it is better to check for such case even if it may not happen.
>
> Vladimir
>
>>
>> igor
>>
>>>
>>> Thanks,
>>> Vladimir
>>>
>>> On 4/13/17 8:30 PM, Igor Veresov wrote:
>>>> This seems like a long-standing bug. Analysis in MemNode::find_previous_store() tries to relax memory dependencies by proving that memory accesses don’t alias. The code is fine for oops: if [offset, offset+length) intervals don’t overlap it proves the accesses don’t alias because bases always point to the start of an object. For raw accesses that’s not true. Offset analysis doesn’t mean much without proving that bases are not the same.
>>>>
>>>> Webrev: http://cr.openjdk.java.net/~iveresov/8178047/webrev.00
>>>>
>>>> Thanks,
>>>> igor
>>>>
>>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(XS) 8178047: Aliasing problem with raw memory accesses

Vladimir Kozlov
Nice.

Thanks,
Vladimir

On 4/19/17 9:48 AM, Igor Veresov wrote:

> I’ve updated the webrev: http://cr.openjdk.java.net/~iveresov/8178047/webrev.01
> I did an RBT run and the results look good.
>
> igor
>
>
>> On Apr 14, 2017, at 11:21 AM, Vladimir Kozlov <[hidden email]> wrote:
>>
>> On 4/14/17 10:31 AM, Igor Veresov wrote:
>>>
>>>> On Apr 14, 2017, at 9:53 AM, Vladimir Kozlov <[hidden email]> wrote:
>>>>
>>>> Klass pointers should be also fine I think.
>>>> The problem is only Raw pointers which could point to the same memory but different offset.
>>>> May be use
>>>>
>>>> bool is_raw = adr->bottom_type()->isa_rawptr() != NULL;
>>>> ...
>>>> if (is_raw && st_base != base) {
>>>
>>> I guess in that case AnyPtr should be be also checked for, because it includes RawPtr, right?
>>
>> Yes.
>>
>>>
>>>>
>>>> Also what if pointer type is mixed: original is oop and mem node is raw?
>>>> May we should check that if when one of them is raw we should compare bases.
>>>
>>> Is it possible for that to happen?
>>
>> May be not. Usually we will have membars around unsafe memory access. But if we have AnyPtr, as you said, it could be mixed I think. I think it is better to check for such case even if it may not happen.
>>
>> Vladimir
>>
>>>
>>> igor
>>>
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>> On 4/13/17 8:30 PM, Igor Veresov wrote:
>>>>> This seems like a long-standing bug. Analysis in MemNode::find_previous_store() tries to relax memory dependencies by proving that memory accesses don’t alias. The code is fine for oops: if [offset, offset+length) intervals don’t overlap it proves the accesses don’t alias because bases always point to the start of an object. For raw accesses that’s not true. Offset analysis doesn’t mean much without proving that bases are not the same.
>>>>>
>>>>> Webrev: http://cr.openjdk.java.net/~iveresov/8178047/webrev.00
>>>>>
>>>>> Thanks,
>>>>> igor
>>>>>
>>>
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(XS) 8178047: Aliasing problem with raw memory accesses

Vladimir Ivanov
In reply to this post by Igor Veresov
Looks good.

Best regards,
Vladimir Ivanov

On 4/19/17 7:48 PM, Igor Veresov wrote:

> I’ve updated the webrev: http://cr.openjdk.java.net/~iveresov/8178047/webrev.01
> I did an RBT run and the results look good.
>
> igor
>
>
>> On Apr 14, 2017, at 11:21 AM, Vladimir Kozlov <[hidden email]> wrote:
>>
>> On 4/14/17 10:31 AM, Igor Veresov wrote:
>>>
>>>> On Apr 14, 2017, at 9:53 AM, Vladimir Kozlov <[hidden email]> wrote:
>>>>
>>>> Klass pointers should be also fine I think.
>>>> The problem is only Raw pointers which could point to the same memory but different offset.
>>>> May be use
>>>>
>>>> bool is_raw = adr->bottom_type()->isa_rawptr() != NULL;
>>>> ...
>>>> if (is_raw && st_base != base) {
>>>
>>> I guess in that case AnyPtr should be be also checked for, because it includes RawPtr, right?
>>
>> Yes.
>>
>>>
>>>>
>>>> Also what if pointer type is mixed: original is oop and mem node is raw?
>>>> May we should check that if when one of them is raw we should compare bases.
>>>
>>> Is it possible for that to happen?
>>
>> May be not. Usually we will have membars around unsafe memory access. But if we have AnyPtr, as you said, it could be mixed I think. I think it is better to check for such case even if it may not happen.
>>
>> Vladimir
>>
>>>
>>> igor
>>>
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>> On 4/13/17 8:30 PM, Igor Veresov wrote:
>>>>> This seems like a long-standing bug. Analysis in MemNode::find_previous_store() tries to relax memory dependencies by proving that memory accesses don’t alias. The code is fine for oops: if [offset, offset+length) intervals don’t overlap it proves the accesses don’t alias because bases always point to the start of an object. For raw accesses that’s not true. Offset analysis doesn’t mean much without proving that bases are not the same.
>>>>>
>>>>> Webrev: http://cr.openjdk.java.net/~iveresov/8178047/webrev.00
>>>>>
>>>>> Thanks,
>>>>> igor
>>>>>
>>>
>
Loading...