RFR(XS): 8168662: Intrinsic support for event based tracing needs explicit control dependency

[Markus Gronlund]

Thanks a lot for taking a look Vladimir,

Good points - I have updated accordingly.

(please see updated webrev):

http://cr.openjdk.java.net/~mgronlun/8168662/webrev02/

Thanks again
Markus

-----Original Message-----
From: Vladimir Kozlov 
Sent: den 26 oktober 2016 03:05
To: Markus Gronlund; hotspot-compiler-dev at openjdk.java.net
Subject: Re: RFR(XS): 8168662: Intrinsic support for event based tracing needs explicit control dependency

I missed this in review :(

You don't need to use set_control() I think. You should just use jobj_is_not_null:

Node* res = make_load(jobj_is_not_null, jobj, TypeInstPtr::NOTNULL, T_OBJECT, MemNode::unordered);

But I don't insist on this.

Also set_result() will do record_for_igvn(region) so you don't need it.

Thanks,
Vladimir

On 10/25/16 4:47 PM, Markus Gronlund wrote:
> Greetings,
>
> I recently integrated intrinsic support for event based tracing which was tracked in JDK-8166806 (https://bugs.openjdk.java.net/browse/JDK-8166806 ).
>
> Unfortunately, the changes for 8166806 led to issues seen in testing 
> on SPARC and AARCH64 platforms in that the intrinsic code was missing 
> an explicit control dependency for C2. On the x86 platform, it seems that there is an implicit control dependency that makes the original code work correctly - but on the former platforms, the lack of dependency allows a load to "float" up before the implicit null check dispatch for the uncommon trap.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8168662
>
> Webrev: http://cr.openjdk.java.net/~mgronlun/8168662/webrev01/
>
> I have managed to reproduce and analyze the assembler output for SPARC with the updated changes (please see bug for details).
>
> I would need to integrate this to resolve some testing issues, so reviews very much appreciated.
>
> Thanks in advance and sorry for any inconveniences related to 8166806.
>
> Best regards
>
> Markus
>
> PS also thanks for Nils Eliasson for assistance on this issue.
>