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