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

Mon, 15 Oct 2018 11:00:27 +0200

author
thartmann
date
Mon, 15 Oct 2018 11:00:27 +0200
changeset 9513
e044997c2eda
parent 9512
992120803410
child 9514
d853bac073f8

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

src/share/vm/opto/cfgnode.cpp file | annotate | diff | comparison | revisions
src/share/vm/opto/coalesce.cpp file | annotate | diff | comparison | revisions
src/share/vm/opto/machnode.cpp file | annotate | diff | comparison | revisions
src/share/vm/opto/matcher.cpp file | annotate | diff | comparison | revisions
test/compiler/gcbarriers/TestMembarDependencies.java file | annotate | diff | comparison | revisions
     1.1 --- a/src/share/vm/opto/cfgnode.cpp	Mon Oct 22 05:26:38 2018 -0400
     1.2 +++ b/src/share/vm/opto/cfgnode.cpp	Mon Oct 15 11:00:27 2018 +0200
     1.3 @@ -2016,6 +2016,7 @@
     1.4    uint ideal_reg = _type->ideal_reg();
     1.5    assert( ideal_reg != Node::NotAMachineReg, "invalid type at Phi" );
     1.6    if( ideal_reg == 0 ) return RegMask::Empty;
     1.7 +  assert(ideal_reg != Op_RegFlags, "flags register is not spillable");
     1.8    return *(Compile::current()->matcher()->idealreg2spillmask[ideal_reg]);
     1.9  }
    1.10  
     2.1 --- a/src/share/vm/opto/coalesce.cpp	Mon Oct 22 05:26:38 2018 -0400
     2.2 +++ b/src/share/vm/opto/coalesce.cpp	Mon Oct 15 11:00:27 2018 +0200
     2.3 @@ -292,7 +292,14 @@
     2.4                // Copy any flags as well
     2.5                _phc.clone_projs(pred, pred->end_idx(), m, copy, _phc._lrg_map);
     2.6              } else {
     2.7 -              const RegMask *rm = C->matcher()->idealreg2spillmask[m->ideal_reg()];
     2.8 +              int ireg = m->ideal_reg();
     2.9 +              if (ireg == 0 || ireg == Op_RegFlags) {
    2.10 +                assert(false, err_msg("attempted to spill a non-spillable item: %d: %s, ireg = %d",
    2.11 +                                      m->_idx, m->Name(), ireg));
    2.12 +                C->record_method_not_compilable("attempted to spill a non-spillable item");
    2.13 +                return;
    2.14 +              }
    2.15 +              const RegMask *rm = C->matcher()->idealreg2spillmask[ireg];
    2.16                copy = new (C) MachSpillCopyNode(m, *rm, *rm);
    2.17                // Find a good place to insert.  Kinda tricky, use a subroutine
    2.18                insert_copy_with_overlap(pred,copy,phi_name,src_name);
    2.19 @@ -326,7 +333,14 @@
    2.20                b->insert_node(copy, l++);
    2.21                l += _phc.clone_projs(b, l, m, copy, _phc._lrg_map);
    2.22              } else {
    2.23 -              const RegMask *rm = C->matcher()->idealreg2spillmask[m->ideal_reg()];
    2.24 +              int ireg = m->ideal_reg();
    2.25 +              if (ireg == 0 || ireg == Op_RegFlags) {
    2.26 +                assert(false, err_msg("attempted to spill a non-spillable item: %d: %s, ireg = %d",
    2.27 +                                      m->_idx, m->Name(), ireg));
    2.28 +                C->record_method_not_compilable("attempted to spill a non-spillable item");
    2.29 +                return;
    2.30 +              }
    2.31 +              const RegMask *rm = C->matcher()->idealreg2spillmask[ireg];
    2.32                copy = new (C) MachSpillCopyNode(m, *rm, *rm);
    2.33                // Insert the copy in the basic block, just before us
    2.34                b->insert_node(copy, l++);
    2.35 @@ -373,7 +387,14 @@
    2.36                if( k < b->_num_succs )
    2.37                  continue;     // Live out; do not pre-split
    2.38                // Split the lrg at this use
    2.39 -              const RegMask *rm = C->matcher()->idealreg2spillmask[inp->ideal_reg()];
    2.40 +              int ireg = inp->ideal_reg();
    2.41 +              if (ireg == 0 || ireg == Op_RegFlags) {
    2.42 +                assert(false, err_msg("attempted to spill a non-spillable item: %d: %s, ireg = %d",
    2.43 +                                      inp->_idx, inp->Name(), ireg));
    2.44 +                C->record_method_not_compilable("attempted to spill a non-spillable item");
    2.45 +                return;
    2.46 +              }
    2.47 +              const RegMask *rm = C->matcher()->idealreg2spillmask[ireg];
    2.48                Node *copy = new (C) MachSpillCopyNode( inp, *rm, *rm );
    2.49                // Insert the copy in the use-def chain
    2.50                n->set_req(inpidx, copy );
     3.1 --- a/src/share/vm/opto/machnode.cpp	Mon Oct 22 05:26:38 2018 -0400
     3.2 +++ b/src/share/vm/opto/machnode.cpp	Mon Oct 15 11:00:27 2018 +0200
     3.3 @@ -619,6 +619,7 @@
     3.4    }
     3.5  
     3.6    // Values outside the domain represent debug info
     3.7 +  assert(in(idx)->ideal_reg() != Op_RegFlags, "flags register is not spillable");
     3.8    return *Compile::current()->matcher()->idealreg2spillmask[in(idx)->ideal_reg()];
     3.9  }
    3.10  
     4.1 --- a/src/share/vm/opto/matcher.cpp	Mon Oct 22 05:26:38 2018 -0400
     4.2 +++ b/src/share/vm/opto/matcher.cpp	Mon Oct 15 11:00:27 2018 +0200
     4.3 @@ -95,6 +95,7 @@
     4.4    idealreg2spillmask  [Op_VecD] = NULL;
     4.5    idealreg2spillmask  [Op_VecX] = NULL;
     4.6    idealreg2spillmask  [Op_VecY] = NULL;
     4.7 +  idealreg2spillmask  [Op_RegFlags] = NULL;
     4.8  
     4.9    idealreg2debugmask  [Op_RegI] = NULL;
    4.10    idealreg2debugmask  [Op_RegN] = NULL;
    4.11 @@ -106,6 +107,7 @@
    4.12    idealreg2debugmask  [Op_VecD] = NULL;
    4.13    idealreg2debugmask  [Op_VecX] = NULL;
    4.14    idealreg2debugmask  [Op_VecY] = NULL;
    4.15 +  idealreg2debugmask  [Op_RegFlags] = NULL;
    4.16  
    4.17    idealreg2mhdebugmask[Op_RegI] = NULL;
    4.18    idealreg2mhdebugmask[Op_RegN] = NULL;
    4.19 @@ -117,6 +119,7 @@
    4.20    idealreg2mhdebugmask[Op_VecD] = NULL;
    4.21    idealreg2mhdebugmask[Op_VecX] = NULL;
    4.22    idealreg2mhdebugmask[Op_VecY] = NULL;
    4.23 +  idealreg2mhdebugmask[Op_RegFlags] = NULL;
    4.24  
    4.25    debug_only(_mem_node = NULL;)   // Ideal memory node consumed by mach node
    4.26  }
     5.1 --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
     5.2 +++ b/test/compiler/gcbarriers/TestMembarDependencies.java	Mon Oct 15 11:00:27 2018 +0200
     5.3 @@ -0,0 +1,98 @@
     5.4 +/*
     5.5 + * Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved.
     5.6 + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
     5.7 + *
     5.8 + * This code is free software; you can redistribute it and/or modify it
     5.9 + * under the terms of the GNU General Public License version 2 only, as
    5.10 + * published by the Free Software Foundation.
    5.11 + *
    5.12 + * This code is distributed in the hope that it will be useful, but WITHOUT
    5.13 + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
    5.14 + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
    5.15 + * version 2 for more details (a copy is included in the LICENSE file that
    5.16 + * accompanied this code).
    5.17 + *
    5.18 + * You should have received a copy of the GNU General Public License version
    5.19 + * 2 along with this work; if not, write to the Free Software Foundation,
    5.20 + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
    5.21 + *
    5.22 + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
    5.23 + * or visit www.oracle.com if you need additional information or have any
    5.24 + * questions.
    5.25 + */
    5.26 +
    5.27 +/*
    5.28 + * @test TestMembarDependencies
    5.29 + * @bug 8172850
    5.30 + * @summary Tests correct scheduling of memory loads around MembarVolatile emitted by GC barriers.
    5.31 + * @library /testlibrary
    5.32 + * @run driver compiler.membars.TestMembarDependencies
    5.33 + */
    5.34 +
    5.35 +package compiler.membars;
    5.36 +
    5.37 +import com.oracle.java.testlibrary.*;
    5.38 +
    5.39 +public class TestMembarDependencies {
    5.40 +    private static TestMembarDependencies f1;
    5.41 +    private static TestMembarDependencies f2;
    5.42 +
    5.43 +    public static void main(String args[]) throws Throwable {
    5.44 +        if (args.length == 0) {
    5.45 +            // For debugging, add "-XX:+TraceOptoPipelining"
    5.46 +            OutputAnalyzer oa = ProcessTools.executeTestJvm("-XX:+IgnoreUnrecognizedVMOptions",
    5.47 +                "-XX:-TieredCompilation", "-XX:-BackgroundCompilation", "-XX:+PrintOpto",
    5.48 +                "-XX:CompileCommand=compileonly,compiler.membars.TestMembarDependencies::test*",
    5.49 +                "-XX:CompileCommand=dontinline,compiler.membars.TestMembarDependencies::test_m1",
    5.50 +                TestMembarDependencies.class.getName(), "run");
    5.51 +            // C2 should not crash or bail out from compilation
    5.52 +            oa.shouldHaveExitValue(0);
    5.53 +            oa.shouldNotMatch("Bailout: Recompile without subsuming loads");
    5.54 +            System.out.println(oa.getOutput());
    5.55 +        } else {
    5.56 +            f2 = new TestMembarDependencies();
    5.57 +            // Trigger compilation of test1 and test2
    5.58 +            for (int i = 0; i < 10_000; ++i) {
    5.59 +              f2.test1(f2);
    5.60 +              f2.test2(f2);
    5.61 +            }
    5.62 +        }
    5.63 +    }
    5.64 +
    5.65 +    public void test_m1() { }
    5.66 +    public void test_m2() { }
    5.67 +
    5.68 +    public void test1(TestMembarDependencies obj) {
    5.69 +        // Try/catch/finally is used to create a CFG block without a test + jmpCon
    5.70 +        // allowing GCM to schedule the testN_mem_reg0 instruction into that block.
    5.71 +        try {
    5.72 +            // Method call defines memory state that is then
    5.73 +            // used by subsequent instructions/blocks (see below).
    5.74 +            test_m1();
    5.75 +        } catch (Exception e) {
    5.76 +
    5.77 +        } finally {
    5.78 +            // Oop write to field emits a GC post-barrier with a MembarVolatile
    5.79 +            // which has a wide memory effect (kills all memory). This creates an
    5.80 +            // anti-dependency on all surrounding memory loads.
    5.81 +            f1 = obj;
    5.82 +        }
    5.83 +        // The empty method m2 is inlined but the null check of f2 remains. It is encoded
    5.84 +        // as CmpN(LoadN(MEM), NULL) where MEM is the memory after the call to test_m1().
    5.85 +        // This is matched to testN_mem_reg0 on x86 which is scheduled before the barrier
    5.86 +        // in the try/catch block due to the anti-dependency on the MembarVolatile.
    5.87 +        // C2 crashes in the register allocator when trying to spill the flag register
    5.88 +        // to keep the result of the testN instruction live from the try/catch block
    5.89 +        // until it is here.
    5.90 +        f2.test_m2();
    5.91 +    }
    5.92 +
    5.93 +    public void test2(TestMembarDependencies obj) {
    5.94 +        // Same as test1 but without try/catch/finally.
    5.95 +        // This causes C2 to bail out in block local scheduling because testN_mem_reg0 is
    5.96 +        // scheduled into a block that already contains another test + jmpCon instruction.
    5.97 +        test_m1();
    5.98 +        f1 = obj;
    5.99 +        f2.test_m2();
   5.100 +    }
   5.101 +}

mercurial