RFR: 8227407: ZGC: C2 loads and load barriers can get separated by safepoints

[Erik Österlund]

Hi Stuart,

On 2019-07-23 12:37, Stuart Monteith wrote:
> Hi Erik,
>    I tried applying your patch from:
>      http://cr.openjdk.java.net/~eosterlund/8227407/webrev.03/
>
> and get:
>     patch: **** malformed patch at line 345:    // LoadNode::hash()
> doesn't take the _control_dependency field
>
> If I change the "17" to "18" it does apply successfully:
>   @@ -152,18 +152,16 @@
>
> It is line 323 in the webrev.patch.
> That the line count from before is incorrect is odd. This was applied
> to a fresh repository this morning.

Sorry, that's my bad. I'm an early adopter of the Skara tools, and use git.
There seems to be a bug in the current git based webrev tool that causes 
the patch file to be malformed.
I re-uploaded 03 in-place with a HG based webrev, which should apply.

> The aarch64 code seems fine with the 02 patch, running tier1 tests and
> some other workloads.

Awesome.

> Thanks for adding the verification.

Thanks for taking this for a spin.

/Erik

> BR,
>   Stuart
>
> On Mon, 22 Jul 2019 at 17:51, Erik Österlund <erik.osterlund at oracle.com> wrote:
>> Hi Roland,
>>
>> How about UnknownControl? "Dependency" is kind of implied by the enum type.
>>
>> Webrev:
>> http://cr.openjdk.java.net/~eosterlund/8227407/webrev.03/
>>
>> Incremental:
>> http://cr.openjdk.java.net/~eosterlund/8227407/webrev.02_03/
>>
>> What do you think?
>>
>> Thanks,
>> /Erik
>>
>> On 2019-07-22 16:57, Roland Westrelin wrote:
>>>> How would you feel about adding a new enum value for UnsafePinned, and
>>>> modifying the LoadNode::Ideal transformation to
>>>> explicitly allow the floating if the !depends_only_on_test() ||
>>>> is_unsafe_pinned()?
>>> That's fine with me but the name UnsafePinned wouldn't work because
>>> Pinned is currently used for one thing other than unsafe: some arraycopy
>>> related optimizations. UnknownControlDependency or something shorter?
>>>
>>> Roland.
>>>