Skip to content

Commit bf666bc

Browse files
committedJan 17, 2024
8322692: ZGC: avoid over-unrolling due to hidden barrier size
Reviewed-by: eosterlund, kvn
1 parent de97c0e commit bf666bc

File tree

7 files changed

+112
-3
lines changed

7 files changed

+112
-3
lines changed
 

‎src/hotspot/share/gc/shared/c2/barrierSetC2.hpp

+3
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,9 @@ class BarrierSetC2: public CHeapObj<mtGC> {
278278
virtual bool optimize_loops(PhaseIdealLoop* phase, LoopOptsMode mode, VectorSet& visited, Node_Stack& nstack, Node_List& worklist) const { return false; }
279279
virtual bool strip_mined_loops_expanded(LoopOptsMode mode) const { return false; }
280280
virtual bool is_gc_specific_loop_opts_pass(LoopOptsMode mode) const { return false; }
281+
// Estimated size of the node barrier in number of C2 Ideal nodes.
282+
// This is used to guide heuristics in C2, e.g. whether to unroll a loop.
283+
virtual uint estimated_barrier_size(const Node* node) const { return 0; }
281284

282285
enum CompilePhase {
283286
BeforeOptimize,

‎src/hotspot/share/gc/z/c2/zBarrierSetC2.cpp

+15-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2015, 2023, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2015, 2024, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -320,6 +320,20 @@ void ZStoreBarrierStubC2::emit_code(MacroAssembler& masm) {
320320
ZBarrierSet::assembler()->generate_c2_store_barrier_stub(&masm, static_cast<ZStoreBarrierStubC2*>(this));
321321
}
322322

323+
uint ZBarrierSetC2::estimated_barrier_size(const Node* node) const {
324+
uint8_t barrier_data = MemNode::barrier_data(node);
325+
assert(barrier_data != 0, "should be a barrier node");
326+
uint uncolor_or_color_size = node->is_Load() ? 1 : 2;
327+
if ((barrier_data & ZBarrierElided) != 0) {
328+
return uncolor_or_color_size;
329+
}
330+
// A compare and branch corresponds to approximately four fast-path Ideal
331+
// nodes (Cmp, Bool, If, If projection). The slow path (If projection and
332+
// runtime call) is excluded since the corresponding code is laid out
333+
// separately and does not directly affect performance.
334+
return uncolor_or_color_size + 4;
335+
}
336+
323337
void* ZBarrierSetC2::create_barrier_state(Arena* comp_arena) const {
324338
return new (comp_arena) ZBarrierSetC2State(comp_arena);
325339
}

‎src/hotspot/share/gc/z/c2/zBarrierSetC2.hpp

+1
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ class ZBarrierSetC2 : public BarrierSetC2 {
128128
const Type* val_type) const;
129129

130130
public:
131+
virtual uint estimated_barrier_size(const Node* node) const;
131132
virtual void* create_barrier_state(Arena* comp_arena) const;
132133
virtual bool array_copy_requires_gc_barriers(bool tightly_coupled_alloc,
133134
BasicType type,

‎src/hotspot/share/opto/loopTransform.cpp

+7-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2000, 2023, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2000, 2024, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -24,6 +24,8 @@
2424

2525
#include "precompiled.hpp"
2626
#include "compiler/compileLog.hpp"
27+
#include "gc/shared/barrierSet.hpp"
28+
#include "gc/shared/c2/barrierSetC2.hpp"
2729
#include "memory/allocation.inline.hpp"
2830
#include "opto/addnode.hpp"
2931
#include "opto/callnode.hpp"
@@ -996,9 +998,12 @@ bool IdealLoopTree::policy_unroll(PhaseIdealLoop *phase) {
996998
uint body_size = _body.size();
997999
// Key test to unroll loop in CRC32 java code
9981000
int xors_in_loop = 0;
999-
// Also count ModL, DivL and MulL which expand mightly
1001+
// Also count ModL, DivL, MulL, and other nodes that expand mightly
10001002
for (uint k = 0; k < _body.size(); k++) {
10011003
Node* n = _body.at(k);
1004+
if (MemNode::barrier_data(n) != 0) {
1005+
body_size += BarrierSet::barrier_set()->barrier_set_c2()->estimated_barrier_size(n);
1006+
}
10021007
switch (n->Opcode()) {
10031008
case Op_XorI: xors_in_loop++; break; // CRC32 java code
10041009
case Op_ModL: body_size += 30; break;

‎src/hotspot/share/opto/memnode.cpp

+9
Original file line numberDiff line numberDiff line change
@@ -839,6 +839,15 @@ const TypePtr* MemNode::calculate_adr_type(const Type* t, const TypePtr* cross_c
839839
}
840840
}
841841

842+
uint8_t MemNode::barrier_data(const Node* n) {
843+
if (n->is_LoadStore()) {
844+
return n->as_LoadStore()->barrier_data();
845+
} else if (n->is_Mem()) {
846+
return n->as_Mem()->barrier_data();
847+
}
848+
return 0;
849+
}
850+
842851
//=============================================================================
843852
// Should LoadNode::Ideal() attempt to remove control edges?
844853
bool LoadNode::can_remove_control() const {

‎src/hotspot/share/opto/memnode.hpp

+3
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,9 @@ class MemNode : public Node {
126126
#endif
127127
}
128128

129+
// Return the barrier data of n, if available, or 0 otherwise.
130+
static uint8_t barrier_data(const Node* n);
131+
129132
// Map a load or store opcode to its corresponding store opcode.
130133
// (Return -1 if unknown.)
131134
virtual int store_Opcode() const { return -1; }
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
/*
2+
* Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*/
23+
24+
package compiler.gcbarriers;
25+
26+
import compiler.lib.ir_framework.*;
27+
import java.lang.invoke.VarHandle;
28+
import java.lang.invoke.MethodHandles;
29+
30+
/**
31+
* @test
32+
* @summary Test that the expanded size of ZGC barriers is taken into account in
33+
* C2's loop unrolling heuristics so that over-unrolling is avoided.
34+
* The tests use volatile memory accesses to prevent C2 from simply
35+
* optimizing them away.
36+
* @library /test/lib /
37+
* @requires vm.gc.ZGenerational
38+
* @run driver compiler.gcbarriers.TestZGCUnrolling
39+
*/
40+
41+
public class TestZGCUnrolling {
42+
43+
static class Outer {
44+
Object f;
45+
}
46+
47+
static final VarHandle fVarHandle;
48+
static {
49+
MethodHandles.Lookup l = MethodHandles.lookup();
50+
try {
51+
fVarHandle = l.findVarHandle(Outer.class, "f", Object.class);
52+
} catch (Exception e) {
53+
throw new Error(e);
54+
}
55+
}
56+
57+
public static void main(String[] args) {
58+
TestFramework.runWithFlags("-XX:+UseZGC", "-XX:+ZGenerational",
59+
"-XX:LoopUnrollLimit=24");
60+
}
61+
62+
@Test
63+
@IR(counts = {IRNode.STORE_P, "1"})
64+
public static void testNoUnrolling(Outer o, Object o1) {
65+
for (int i = 0; i < 64; i++) {
66+
fVarHandle.setVolatile(o, o1);
67+
}
68+
}
69+
70+
@Run(test = {"testNoUnrolling"})
71+
void run() {
72+
testNoUnrolling(new Outer(), new Object());
73+
}
74+
}

0 commit comments

Comments
 (0)
Please sign in to comment.