# HG changeset patch

# User thartmann

# Date 1539594027 -7200

# Node ID e044997c2edaeae97866394a7f8e2ddebbd41392

# Parent  99212080341058548d449a22d1381e79353ec5b5

8172850: Anti-dependency on membar causes crash in register allocator due to invalid instruction scheduling

Summary: Regression test and additional asserts. The problem is fixed by 8087341.

Reviewed-by: kvn

diff -r 992120803410 -r e044997c2eda src/share/vm/opto/cfgnode.cpp

--- openjdk/hotspot/src/share/vm/opto/cfgnode.cpp	Mon Oct 22 05:26:38 2018 -0400

+++ openjdk/hotspot/src/share/vm/opto/cfgnode.cpp	Mon Oct 15 11:00:27 2018 +0200

@@ -2016,6 +2016,7 @@

   uint ideal_reg = _type->ideal_reg();

   assert( ideal_reg != Node::NotAMachineReg, "invalid type at Phi" );

   if( ideal_reg == 0 ) return RegMask::Empty;

+  assert(ideal_reg != Op_RegFlags, "flags register is not spillable");

   return *(Compile::current()->matcher()->idealreg2spillmask[ideal_reg]);

 }

diff -r 992120803410 -r e044997c2eda src/share/vm/opto/coalesce.cpp

--- openjdk/hotspot/src/share/vm/opto/coalesce.cpp	Mon Oct 22 05:26:38 2018 -0400

+++ openjdk/hotspot/src/share/vm/opto/coalesce.cpp	Mon Oct 15 11:00:27 2018 +0200

@@ -292,7 +292,14 @@

               // Copy any flags as well

               _phc.clone_projs(pred, pred->end_idx(), m, copy, _phc._lrg_map);

             } else {

-              const RegMask *rm = C->matcher()->idealreg2spillmask[m->ideal_reg()];

+              int ireg = m->ideal_reg();

+              if (ireg == 0 || ireg == Op_RegFlags) {

+                assert(false, err_msg("attempted to spill a non-spillable item: %d: %s, ireg = %d",

+                                      m->_idx, m->Name(), ireg));

+                C->record_method_not_compilable("attempted to spill a non-spillable item");

+                return;

+              }

+              const RegMask *rm = C->matcher()->idealreg2spillmask[ireg];

               copy = new (C) MachSpillCopyNode(m, *rm, *rm);

               // Find a good place to insert.  Kinda tricky, use a subroutine

               insert_copy_with_overlap(pred,copy,phi_name,src_name);

@@ -326,7 +333,14 @@

               b->insert_node(copy, l++);

               l += _phc.clone_projs(b, l, m, copy, _phc._lrg_map);

             } else {

-              const RegMask *rm = C->matcher()->idealreg2spillmask[m->ideal_reg()];

+              int ireg = m->ideal_reg();

+              if (ireg == 0 || ireg == Op_RegFlags) {

+                assert(false, err_msg("attempted to spill a non-spillable item: %d: %s, ireg = %d",

+                                      m->_idx, m->Name(), ireg));

+                C->record_method_not_compilable("attempted to spill a non-spillable item");

+                return;

+              }

+              const RegMask *rm = C->matcher()->idealreg2spillmask[ireg];

               copy = new (C) MachSpillCopyNode(m, *rm, *rm);

               // Insert the copy in the basic block, just before us

               b->insert_node(copy, l++);

@@ -373,7 +387,14 @@

               if( k < b->_num_succs )

                 continue;     // Live out; do not pre-split

               // Split the lrg at this use

-              const RegMask *rm = C->matcher()->idealreg2spillmask[inp->ideal_reg()];

+              int ireg = inp->ideal_reg();

+              if (ireg == 0 || ireg == Op_RegFlags) {

+                assert(false, err_msg("attempted to spill a non-spillable item: %d: %s, ireg = %d",

+                                      inp->_idx, inp->Name(), ireg));

+                C->record_method_not_compilable("attempted to spill a non-spillable item");

+                return;

+              }

+              const RegMask *rm = C->matcher()->idealreg2spillmask[ireg];

               Node *copy = new (C) MachSpillCopyNode( inp, *rm, *rm );

               // Insert the copy in the use-def chain

               n->set_req(inpidx, copy );

diff -r 992120803410 -r e044997c2eda src/share/vm/opto/machnode.cpp

--- openjdk/hotspot/src/share/vm/opto/machnode.cpp	Mon Oct 22 05:26:38 2018 -0400

+++ openjdk/hotspot/src/share/vm/opto/machnode.cpp	Mon Oct 15 11:00:27 2018 +0200

@@ -619,6 +619,7 @@

   }

   // Values outside the domain represent debug info

+  assert(in(idx)->ideal_reg() != Op_RegFlags, "flags register is not spillable");

   return *Compile::current()->matcher()->idealreg2spillmask[in(idx)->ideal_reg()];

 }

diff -r 992120803410 -r e044997c2eda src/share/vm/opto/matcher.cpp

--- openjdk/hotspot/src/share/vm/opto/matcher.cpp	Mon Oct 22 05:26:38 2018 -0400

+++ openjdk/hotspot/src/share/vm/opto/matcher.cpp	Mon Oct 15 11:00:27 2018 +0200

@@ -95,6 +95,7 @@

   idealreg2spillmask  [Op_VecD] = NULL;

   idealreg2spillmask  [Op_VecX] = NULL;

   idealreg2spillmask  [Op_VecY] = NULL;

+  idealreg2spillmask  [Op_RegFlags] = NULL;

   idealreg2debugmask  [Op_RegI] = NULL;

   idealreg2debugmask  [Op_RegN] = NULL;

@@ -106,6 +107,7 @@

   idealreg2debugmask  [Op_VecD] = NULL;

   idealreg2debugmask  [Op_VecX] = NULL;

   idealreg2debugmask  [Op_VecY] = NULL;

+  idealreg2debugmask  [Op_RegFlags] = NULL;

   idealreg2mhdebugmask[Op_RegI] = NULL;

   idealreg2mhdebugmask[Op_RegN] = NULL;

@@ -117,6 +119,7 @@

   idealreg2mhdebugmask[Op_VecD] = NULL;

   idealreg2mhdebugmask[Op_VecX] = NULL;

   idealreg2mhdebugmask[Op_VecY] = NULL;

+  idealreg2mhdebugmask[Op_RegFlags] = NULL;

   debug_only(_mem_node = NULL;)   // Ideal memory node consumed by mach node

 }

diff -r 992120803410 -r e044997c2eda test/compiler/gcbarriers/TestMembarDependencies.java

--- /dev/null	Thu Jan 01 00:00:00 1970 +0000

+++ openjdk/hotspot/test/compiler/gcbarriers/TestMembarDependencies.java	Mon Oct 15 11:00:27 2018 +0200

@@ -0,0 +1,98 @@

+/*

+ * Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved.

+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.

+ *

+ * This code is free software; you can redistribute it and/or modify it

+ * under the terms of the GNU General Public License version 2 only, as

+ * published by the Free Software Foundation.

+ *

+ * This code is distributed in the hope that it will be useful, but WITHOUT

+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or

+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License

+ * version 2 for more details (a copy is included in the LICENSE file that

+ * accompanied this code).

+ *

+ * You should have received a copy of the GNU General Public License version

+ * 2 along with this work; if not, write to the Free Software Foundation,

+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.

+ *

+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA

+ * or visit www.oracle.com if you need additional information or have any

+ * questions.

+ */

+

+/*

+ * @test TestMembarDependencies

+ * @bug 8172850

+ * @summary Tests correct scheduling of memory loads around MembarVolatile emitted by GC barriers.

+ * @library /testlibrary

+ * @run driver compiler.membars.TestMembarDependencies

+ */

+

+package compiler.membars;

+

+import com.oracle.java.testlibrary.*;

+

+public class TestMembarDependencies {

+    private static TestMembarDependencies f1;

+    private static TestMembarDependencies f2;

+

+    public static void main(String args[]) throws Throwable {

+        if (args.length == 0) {

+            // For debugging, add "-XX:+TraceOptoPipelining"

+            OutputAnalyzer oa = ProcessTools.executeTestJvm("-XX:+IgnoreUnrecognizedVMOptions",

+                "-XX:-TieredCompilation", "-XX:-BackgroundCompilation", "-XX:+PrintOpto",

+                "-XX:CompileCommand=compileonly,compiler.membars.TestMembarDependencies::test*",

+                "-XX:CompileCommand=dontinline,compiler.membars.TestMembarDependencies::test_m1",

+                TestMembarDependencies.class.getName(), "run");

+            // C2 should not crash or bail out from compilation

+            oa.shouldHaveExitValue(0);

+            oa.shouldNotMatch("Bailout: Recompile without subsuming loads");

+            System.out.println(oa.getOutput());

+        } else {

+            f2 = new TestMembarDependencies();

+            // Trigger compilation of test1 and test2

+            for (int i = 0; i < 10_000; ++i) {

+              f2.test1(f2);

+              f2.test2(f2);

+            }

+        }

+    }

+

+    public void test_m1() { }

+    public void test_m2() { }

+

+    public void test1(TestMembarDependencies obj) {

+        // Try/catch/finally is used to create a CFG block without a test + jmpCon

+        // allowing GCM to schedule the testN_mem_reg0 instruction into that block.

+        try {

+            // Method call defines memory state that is then

+            // used by subsequent instructions/blocks (see below).

+            test_m1();

+        } catch (Exception e) {

+

+        } finally {

+            // Oop write to field emits a GC post-barrier with a MembarVolatile

+            // which has a wide memory effect (kills all memory). This creates an

+            // anti-dependency on all surrounding memory loads.

+            f1 = obj;

+        }

+        // The empty method m2 is inlined but the null check of f2 remains. It is encoded

+        // as CmpN(LoadN(MEM), NULL) where MEM is the memory after the call to test_m1().

+        // This is matched to testN_mem_reg0 on x86 which is scheduled before the barrier

+        // in the try/catch block due to the anti-dependency on the MembarVolatile.

+        // C2 crashes in the register allocator when trying to spill the flag register

+        // to keep the result of the testN instruction live from the try/catch block

+        // until it is here.

+        f2.test_m2();

+    }

+

+    public void test2(TestMembarDependencies obj) {

+        // Same as test1 but without try/catch/finally.

+        // This causes C2 to bail out in block local scheduling because testN_mem_reg0 is

+        // scheduled into a block that already contains another test + jmpCon instruction.

+        test_m1();

+        f1 = obj;

+        f2.test_m2();

+    }

+}