<div dir="ltr">Thanks!<div><br></div><div>Regards</div><div>Hui</div></div><div class="gmail_extra"><br><div class="gmail_quote">On 14 November 2015 at 04:37, Vladimir Ivanov <span dir="ltr"><<a href="mailto:vladimir.x.ivanov@oracle.com" target="_blank">vladimir.x.ivanov@oracle.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><br>
<br>
On 11/13/15 6:30 PM, Hui Shi wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Thanks Vladimir & Roland!<br>
<br>
Test is cleaned up and new patch in<br>
<a href="http://cr.openjdk.java.net/~hshi/8139758/webrev_v5/" rel="noreferrer" target="_blank">http://cr.openjdk.java.net/~hshi/8139758/webrev_v5/</a><br>
</blockquote></span>
Looks good.<br>
<br>
I think "@build TestStableMemoryBarrier" is redundant (sorry for not noticing that before), but no need to send new webrev for that.<span class=""><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
BTW, What is the advantage to write test in "@runmain/bootclasspath"<br>
form?  There still some tests written as "@run<br>
main/othervm -Xbootclasspath/a:."  under hotspot/test.<br>
</blockquote></span>
As I remember, @run bootclasspath works only if the test resides in a single source file. If there are multiple Java sources involved, you have to use @build + ClassFileInstall + @run /othervm -Xbootclasspath/a. <br>
<br>
Best regards,<br>
Vladimir Ivanov<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
<br>
Regards<br>
Hui<br>
<br>
On 13 November 2015 at 22:22, Vladimir Ivanov<br></span><span class="">
<<a href="mailto:vladimir.x.ivanov@oracle.com" target="_blank">vladimir.x.ivanov@oracle.com</a> <mailto:<a href="mailto:vladimir.x.ivanov@oracle.com" target="_blank">vladimir.x.ivanov@oracle.com</a>>> wrote:<br>
<br>
    Sorry, Hui. I planned to write you, but bogged down in other activities.<br>
<br>
    I suggest to do some additional cleanups in the test.<br>
<br>
    After you switch to bootclasspath these lines shouldn't be needed:<br>
       31  * @run main ClassFileInstaller<br>
       32  *           java/lang/invoke/TestStableMemoryBarrier<br>
       33  *           java/lang/invoke/TestStableMemoryBarrier$NotDominate<br>
<br>
    Also, othervm is redundant for bootclasspath:<br>
       35  * @run main/othervm/bootclasspath -Xcomp<br>
    -XX:CompileOnly=::testCompile<br>
       36  *                   java.lang.invoke.TestStableMemoryBarrier<br>
<br>
    Best regards,<br>
    Vladimir Ivanov<br>
<br>
    On 11/13/15 4:53 PM, Hui Shi wrote:<br>
<br>
        Would some one help review this patch?<br>
<br>
        Regards<br>
        Hui<br>
<br>
        On 8 November 2015 at 22:12, Hui Shi <<a href="mailto:hui.shi@linaro.org" target="_blank">hui.shi@linaro.org</a><br>
        <mailto:<a href="mailto:hui.shi@linaro.org" target="_blank">hui.shi@linaro.org</a>><br></span><span class="">
        <mailto:<a href="mailto:hui.shi@linaro.org" target="_blank">hui.shi@linaro.org</a> <mailto:<a href="mailto:hui.shi@linaro.org" target="_blank">hui.shi@linaro.org</a>>>> wrote:<br>
<br>
             Thanks Vladimir!<br>
<br>
             Yes, isStableEnabled and isServerWithStable variable can be<br>
        deleted<br>
             from test case.<br>
<br>
             New patch in<br>
        <a href="http://cr.openjdk.java.net/~hshi/8139758/webrev_v4/" rel="noreferrer" target="_blank">http://cr.openjdk.java.net/~hshi/8139758/webrev_v4/</a><br>
<br>
             Keep "othervm" in tag same with other StableTest and use @run<br>
             main/othervm/bootclasspath<br>
<br>
             Regards<br>
             Shi Hui<br>
<br>
             On 5 November 2015 at 22:01, Vladimir Ivanov<br>
             <<a href="mailto:vladimir.x.ivanov@oracle.com" target="_blank">vladimir.x.ivanov@oracle.com</a><br>
        <mailto:<a href="mailto:vladimir.x.ivanov@oracle.com" target="_blank">vladimir.x.ivanov@oracle.com</a>><br></span>
        <mailto:<a href="mailto:vladimir.x.ivanov@oracle.com" target="_blank">vladimir.x.ivanov@oracle.com</a><div><div class="h5"><br>
        <mailto:<a href="mailto:vladimir.x.ivanov@oracle.com" target="_blank">vladimir.x.ivanov@oracle.com</a>>>><br>
<br>
             wrote:<br>
<br>
                 Thanks for the test case!<br>
<br>
                 You don't need TestStableMemoryBarrier, since<br>
        isStableEnabled<br>
                 and isServerWithStable aren't used anywere.<br>
<br>
                 After you eliminate it, you can use @run<br>
        main/bootclasspath. For<br>
                 example, see [1].<br>
<br>
                 Best regards,<br>
                 Vladimir Ivanov<br>
<br>
                 [1]<br>
        <a href="http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/file/tip/test/compiler/unsafe/UnsafeGetConstantField.java" rel="noreferrer" target="_blank">http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/file/tip/test/compiler/unsafe/UnsafeGetConstantField.java</a><br>
<br>
<br>
                 On 11/5/15 4:23 PM, Hui Shi wrote:<br>
<br>
                     Thanks Roland!<br>
<br>
                     I wrote a test case and put it together with fix in<br>
        <a href="http://cr.openjdk.java.net/~hshi/8139758/webrev_v3/" rel="noreferrer" target="_blank">http://cr.openjdk.java.net/~hshi/8139758/webrev_v3/</a><br>
                     In<br>
<br>
        hotspot/test/compiler/stable/TestStableMemoryBarrier.java,<br>
                     stable<br>
                     field’s object allocation doesn’t dominate method<br>
        exit. This<br>
                     will<br>
                     trigger assertion when verify dominance. This issue<br>
        doesn’t<br>
                     exist in<br>
                     current code, can be exposed when MemBarPrecedent<br>
        is added<br>
                     for stable<br>
                     field write’s MemBarRelease node (with following<br>
        patch).<br>
<br>
                     --- a/src/share/vm/opto/parse3.cpp      Mon Nov 02<br>
        12:34:27<br>
                     2015 +0000<br>
                     +++ b/src/share/vm/opto/parse3.cpp      Wed Nov 04<br>
        15:02:31<br>
                     2015 +0800<br>
                     @@ -313,9 +313,8 @@<br>
                            // Preserve allocation ptr to create<br>
        precedent edge<br>
                     to it in membar<br>
                            // generated on exit from constructor.<br>
                     -    if (C->eliminate_boxing() &&<br>
                     -        adr_type->isa_oopptr() &&<br>
                     adr_type->is_oopptr()->is_ptr_to_boxed_value() &&<br>
                     -        AllocateNode::Ideal_allocation(obj, &_gvn)<br>
        != NULL) {<br>
                     +    if (AllocateNode::Ideal_allocation(obj, &_gvn)<br>
        != NULL) {<br>
                              set_alloc_with_final(obj);<br>
                            }<br>
                          }<br>
<br>
                     Run with debug build on linux amd64 platform<br>
        “jtreg/bin/jtreg<br>
<br>
        hotspot/test/compiler/stable/TestStableMemoryBarrier.java “<br>
                     Assertion<br>
                     can be reproduced.<br>
                     *** Use 167 isn't dominated by def 170 ***<br>
                     # To suppress the following error report, specify<br>
        this argument<br>
                     # after -XX: or in .hotspotrc:<br>
                     SuppressErrorAt=/loopnode.cpp:3235<br>
                     #<br>
                     # A fatal error has been detected by the Java Runtime<br>
                     Environment:<br>
                     #<br>
                     #  Internal Error<br>
<br>
        (/home/shihui/jdk9-hs-comp/src/jdk9/hotspot/src/share/vm/opto/loopnode.cpp:3235),<br>
                     pid=23461, tid=23513<br>
                     #  assert(!had_error) failed: bad dominance<br>
                     #<br>
                     # JRE version: OpenJDK Runtime Environment (9.0) (build<br>
                     1.9.0-internal-debug-shihui_2015_10_20_07_17-b00)<br>
                     # Java VM: OpenJDK 64-Bit Server VM<br>
                     (1.9.0-internal-debug-shihui_2015_10_20_07_17-b00,<br>
        compiled<br>
                     mode,<br>
                     tiered, compressed oops, g1 gc, linux-amd64)<br>
                     # Core dump will be written. Default location: Core<br>
        dumps may be<br>
                     processed with "/usr/share/apport/apport %p %s %c<br>
        %P" (or<br>
                     dumping to<br>
<br>
        /home/shihui/jdk9-hs-comp/src/jdk9/JTwork/scratch/core.23461)<br>
                     #<br>
                     Unsupported internal testing APIs have been used.<br>
                     # An error report file with more information is<br>
        saved as:<br>
                     #<br>
<br>
        /home/shihui/jdk9-hs-comp/src/jdk9/JTwork/scratch/hs_err_pid23461.log<br>
<br>
<br>
                     Regards<br>
                     Shi Hui<br>
<br>
                     On 31 October 2015 at 00:04, Roland Westrelin<br>
                     <<a href="mailto:roland.westrelin@oracle.com" target="_blank">roland.westrelin@oracle.com</a><br>
        <mailto:<a href="mailto:roland.westrelin@oracle.com" target="_blank">roland.westrelin@oracle.com</a>><br>
                     <mailto:<a href="mailto:roland.westrelin@oracle.com" target="_blank">roland.westrelin@oracle.com</a><br>
        <mailto:<a href="mailto:roland.westrelin@oracle.com" target="_blank">roland.westrelin@oracle.com</a>>><br>
                     <mailto:<a href="mailto:roland.westrelin@oracle.com" target="_blank">roland.westrelin@oracle.com</a><br>
        <mailto:<a href="mailto:roland.westrelin@oracle.com" target="_blank">roland.westrelin@oracle.com</a>><br>
                     <mailto:<a href="mailto:roland.westrelin@oracle.com" target="_blank">roland.westrelin@oracle.com</a><br>
        <mailto:<a href="mailto:roland.westrelin@oracle.com" target="_blank">roland.westrelin@oracle.com</a>>>>> wrote:<br>
<br>
                           > webrev:<br>
        <a href="http://cr.openjdk.java.net/~hshi/8139758/webrev_v2/" rel="noreferrer" target="_blank">http://cr.openjdk.java.net/~hshi/8139758/webrev_v2/</a><br>
<br>
                          Thanks for re-submitting this.<br>
                          So you’re fixing an existing bug in the<br>
        process. Nice.<br>
                     We usually<br>
                          need a test case for every bug fix. Can you<br>
        write such<br>
                     a test case<br>
                          that triggers that bug. test/compiler/stable has<br>
                     example regression<br>
                          test cases for @Stable.<br>
<br>
                          Roland.<br>
<br>
<br>
<br>
<br>
<br>
</div></div></blockquote>
</blockquote></div><br></div>