Mercurial > hg > graal-compiler
changeset 13045:94a83e0f9ce1
8017065: C2 allows safepoint checks to leak into G1 pre-barriers
Summary: Make all raw loads strictly respect control dependencies, make sure RCE doesn't move raw loads, add verification of G1 pre-barriers.
Reviewed-by: kvn, roland
author | iveresov |
---|---|
date | Tue, 05 Nov 2013 01:57:18 -0800 |
parents | a905d33ce13a |
children | 613e6a6fc328 |
files | src/share/vm/opto/compile.cpp src/share/vm/opto/compile.hpp src/share/vm/opto/loopTransform.cpp src/share/vm/opto/memnode.hpp |
diffstat | 4 files changed, 82 insertions(+), 21 deletions(-) [+] |
line wrap: on
line diff
--- a/src/share/vm/opto/compile.cpp Tue Nov 05 00:59:30 2013 -0800 +++ b/src/share/vm/opto/compile.cpp Tue Nov 05 01:57:18 2013 -0800 @@ -848,6 +848,7 @@ } #endif + NOT_PRODUCT( verify_barriers(); ) // Now that we know the size of all the monitors we can add a fixed slot // for the original deopt pc. @@ -3368,6 +3369,72 @@ } } } + +// Verify GC barriers consistency +// Currently supported: +// - G1 pre-barriers (see GraphKit::g1_write_barrier_pre()) +void Compile::verify_barriers() { + if (UseG1GC) { + // Verify G1 pre-barriers + const int marking_offset = in_bytes(JavaThread::satb_mark_queue_offset() + PtrQueue::byte_offset_of_active()); + + ResourceArea *area = Thread::current()->resource_area(); + Unique_Node_List visited(area); + Node_List worklist(area); + // We're going to walk control flow backwards starting from the Root + worklist.push(_root); + while (worklist.size() > 0) { + Node* x = worklist.pop(); + if (x == NULL || x == top()) continue; + if (visited.member(x)) { + continue; + } else { + visited.push(x); + } + + if (x->is_Region()) { + for (uint i = 1; i < x->req(); i++) { + worklist.push(x->in(i)); + } + } else { + worklist.push(x->in(0)); + // We are looking for the pattern: + // /->ThreadLocal + // If->Bool->CmpI->LoadB->AddP->ConL(marking_offset) + // \->ConI(0) + // We want to verify that the If and the LoadB have the same control + // See GraphKit::g1_write_barrier_pre() + if (x->is_If()) { + IfNode *iff = x->as_If(); + if (iff->in(1)->is_Bool() && iff->in(1)->in(1)->is_Cmp()) { + CmpNode *cmp = iff->in(1)->in(1)->as_Cmp(); + if (cmp->Opcode() == Op_CmpI && cmp->in(2)->is_Con() && cmp->in(2)->bottom_type()->is_int()->get_con() == 0 + && cmp->in(1)->is_Load()) { + LoadNode* load = cmp->in(1)->as_Load(); + if (load->Opcode() == Op_LoadB && load->in(2)->is_AddP() && load->in(2)->in(2)->Opcode() == Op_ThreadLocal + && load->in(2)->in(3)->is_Con() + && load->in(2)->in(3)->bottom_type()->is_intptr_t()->get_con() == marking_offset) { + + Node* if_ctrl = iff->in(0); + Node* load_ctrl = load->in(0); + + if (if_ctrl != load_ctrl) { + // Skip possible CProj->NeverBranch in infinite loops + if ((if_ctrl->is_Proj() && if_ctrl->Opcode() == Op_CProj) + && (if_ctrl->in(0)->is_MultiBranch() && if_ctrl->in(0)->Opcode() == Op_NeverBranch)) { + if_ctrl = if_ctrl->in(0)->in(0); + } + } + assert(load_ctrl != NULL && if_ctrl == load_ctrl, "controls must match"); + } + } + } + } + } + } + } +} + #endif // The Compile object keeps track of failure reasons separately from the ciEnv.
--- a/src/share/vm/opto/compile.hpp Tue Nov 05 00:59:30 2013 -0800 +++ b/src/share/vm/opto/compile.hpp Tue Nov 05 01:57:18 2013 -0800 @@ -1148,6 +1148,9 @@ // graph is strongly connected from root in both directions. void verify_graph_edges(bool no_dead_code = false) PRODUCT_RETURN; + // Verify GC barrier patterns + void verify_barriers() PRODUCT_RETURN; + // End-of-run dumps. static void print_statistics() PRODUCT_RETURN;
--- a/src/share/vm/opto/loopTransform.cpp Tue Nov 05 00:59:30 2013 -0800 +++ b/src/share/vm/opto/loopTransform.cpp Tue Nov 05 01:57:18 2013 -0800 @@ -1964,7 +1964,7 @@ // Find loads off the surviving projection; remove their control edge for (DUIterator_Fast imax, i = dp->fast_outs(imax); i < imax; i++) { Node* cd = dp->fast_out(i); // Control-dependent node - if( cd->is_Load() ) { // Loads can now float around in the loop + if (cd->is_Load() && cd->depends_only_on_test()) { // Loads can now float around in the loop // Allow the load to float around in the loop, or before it // but NOT before the pre-loop. _igvn.replace_input_of(cd, 0, ctrl); // ctrl, not NULL
--- a/src/share/vm/opto/memnode.hpp Tue Nov 05 00:59:30 2013 -0800 +++ b/src/share/vm/opto/memnode.hpp Tue Nov 05 01:57:18 2013 -0800 @@ -204,6 +204,17 @@ protected: const Type* load_array_final_field(const TypeKlassPtr *tkls, ciKlass* klass) const; + // depends_only_on_test is almost always true, and needs to be almost always + // true to enable key hoisting & commoning optimizations. However, for the + // special case of RawPtr loads from TLS top & end, and other loads performed by + // GC barriers, the control edge carries the dependence preventing hoisting past + // a Safepoint instead of the memory edge. (An unfortunate consequence of having + // Safepoints not set Raw Memory; itself an unfortunate consequence of having Nodes + // which produce results (new raw memory state) inside of loops preventing all + // manner of other optimizations). Basically, it's ugly but so is the alternative. + // See comment in macro.cpp, around line 125 expand_allocate_common(). + virtual bool depends_only_on_test() const { return adr_type() != TypeRawPtr::BOTTOM; } + }; //------------------------------LoadBNode-------------------------------------- @@ -370,16 +381,6 @@ virtual uint ideal_reg() const { return Op_RegP; } virtual int store_Opcode() const { return Op_StoreP; } virtual BasicType memory_type() const { return T_ADDRESS; } - // depends_only_on_test is almost always true, and needs to be almost always - // true to enable key hoisting & commoning optimizations. However, for the - // special case of RawPtr loads from TLS top & end, the control edge carries - // the dependence preventing hoisting past a Safepoint instead of the memory - // edge. (An unfortunate consequence of having Safepoints not set Raw - // Memory; itself an unfortunate consequence of having Nodes which produce - // results (new raw memory state) inside of loops preventing all manner of - // other optimizations). Basically, it's ugly but so is the alternative. - // See comment in macro.cpp, around line 125 expand_allocate_common(). - virtual bool depends_only_on_test() const { return adr_type() != TypeRawPtr::BOTTOM; } }; @@ -393,16 +394,6 @@ virtual uint ideal_reg() const { return Op_RegN; } virtual int store_Opcode() const { return Op_StoreN; } virtual BasicType memory_type() const { return T_NARROWOOP; } - // depends_only_on_test is almost always true, and needs to be almost always - // true to enable key hoisting & commoning optimizations. However, for the - // special case of RawPtr loads from TLS top & end, the control edge carries - // the dependence preventing hoisting past a Safepoint instead of the memory - // edge. (An unfortunate consequence of having Safepoints not set Raw - // Memory; itself an unfortunate consequence of having Nodes which produce - // results (new raw memory state) inside of loops preventing all manner of - // other optimizations). Basically, it's ugly but so is the alternative. - // See comment in macro.cpp, around line 125 expand_allocate_common(). - virtual bool depends_only_on_test() const { return adr_type() != TypeRawPtr::BOTTOM; } }; //------------------------------LoadKlassNode----------------------------------