<div dir="ltr"><div>Thanks Martin! </div><div><br></div><div>Roland, would you please run JPRT test and sponsor this change?</div><div><br></div>Regards<div>Shi Hui</div></div><div class="gmail_extra"><br><div class="gmail_quote">On 12 October 2015 at 17:39, Doerr, Martin <span dir="ltr"><<a href="mailto:martin.doerr@sap.com" target="_blank">martin.doerr@sap.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">





<div lang="DE" link="blue" vlink="purple">
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Hi Hui Shi,<u></u><u></u></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">thanks for the update. Looks good for PPC64 as well, now.<u></u><u></u></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">As you already explained, the MemBarRelease is only relevant for fields of the allocated object. Hence, the final and volatile cases can be handled
 the same way.<u></u><u></u></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Best regards,<u></u><u></u></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">  Martin<u></u><u></u></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span lang="EN-US"><u></u> <u></u></span></p>
<p class="MsoNormal"><b><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> Hui Shi [mailto:<a href="mailto:hui.shi@linaro.org" target="_blank">hui.shi@linaro.org</a>]
<br>
<b>Sent:</b> Samstag, 10. Oktober 2015 15:25<br>
<b>To:</b> Doerr, Martin<br>
<b>Cc:</b> Roland Westrelin; hotspot compiler; <a href="mailto:aarch64-port-dev@openjdk.java.net" target="_blank">aarch64-port-dev@openjdk.java.net</a></span></p><div><div class="h5"><br>
<b>Subject:</b> Re: RFR: Elide more final field's write memory barrier with escape analysis result<u></u><u></u></div></div><p></p><div><div class="h5">
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<div>
<p class="MsoNormal">Thanks Roland and Martin!<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">Agree, it's better remove extra code for PPC64.  new webrev <a href="http://cr.openjdk.java.net/~hshi/8138956/webrev_v2/" target="_blank">http://cr.openjdk.java.net/~hshi/8138956/webrev_v2/</a><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">comments below<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">Regards<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Shi Hui<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<p class="MsoNormal">On 10 October 2015 at 00:17, Doerr, Martin <<a href="mailto:martin.doerr@sap.com" target="_blank">martin.doerr@sap.com</a>> wrote:<u></u><u></u></p>
<p class="MsoNormal" style="margin-bottom:12.0pt">Hi,<br>
<br>
I basically like this change. Thanks for pointing me to it.<br>
<br>
But I don't like the extra code for PPC64. If the allocation does not escape globally it should be safe to elide the membar regardless of whether we wrote final or volatile fields.<u></u><u></u></p>
<div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">Difference is final field is always belonging to initializing object while volatile field isn't. It's reasonable to insert MemoryBarrier, when volatile field is written in its owning class' initializer method, like you example of AtomicInteger.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">In following example, class A initiailzer writes both final field and volatile field, suppose A instance doesn't escape. MemBarRelase is inserted because final and volatile field wirte. Writing b.vol in A's initializer is same with writing
 b.vol in none-initalizer method and doesn't need MemBarRelease in my understanding.  <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">1. If final and volatile field from same allocation, elide MemBarRelase when allocation doesn't escape is safe.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">2. If volatile field doesn't belong to final field's allocation object, MemBarRelase for volatile is not necessary.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">So it's safe to remove MemBarRelease when final field's allocation object doesn't escape. <u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">Class A {<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">      final int a;<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">      A(int val, B b) { a=val; b.vol = val; }<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">}<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">Class B { volatile int vol;}<u></u><u></u></p>
</div>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<blockquote style="border:none;border-left:solid #cccccc 1.0pt;padding:0cm 0cm 0cm 6.0pt;margin-left:4.8pt;margin-right:0cm">
<p class="MsoNormal">Please also note that PPC64_ONLY(...) NOT_PPC(...) is incorrect on PPC32.<br>
<br>
Actually, it's ugly that PPC64_ONLY was used here. Should better be (support_IRIW_for_not_multiple_copy_atomic_cpu && wrote_volatile()), but that's not the topic of this change.<br>
<br>
Best regards,<br>
  Martin<u></u><u></u></p>
<div>
<div>
<p class="MsoNormal"><br>
<br>
-----Original Message-----<br>
From: hotspot-compiler-dev [mailto:<a href="mailto:hotspot-compiler-dev-bounces@openjdk.java.net" target="_blank">hotspot-compiler-dev-bounces@openjdk.java.net</a>] On Behalf Of Roland Westrelin<br>
Sent: Donnerstag, 8. Oktober 2015 13:51<br>
To: Hui Shi<br>
Cc: hotspot compiler; <a href="mailto:aarch64-port-dev@openjdk.java.net" target="_blank">
aarch64-port-dev@openjdk.java.net</a><br>
Subject: Re: RFR: Elide more final field's write memory barrier with escape analysis result<br>
<br>
> <a href="http://cr.openjdk.java.net/~hshi/8138956/webrev/" target="_blank">http://cr.openjdk.java.net/~hshi/8138956/webrev/</a><br>
<br>
The code change looks good to me. The comments have a few typos and are a bit confusing. I'll tweak them before I push it.<br>
<br>
PPC folks, in case you missed it, parse1.cpp has a ppc related fix.<br>
<br>
Roland.<u></u><u></u></p>
</div>
</div>
</blockquote>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
</div>
</div></div></div>
</div>

</blockquote></div><br></div>