Skip to content

Commit dc9fe9a

Browse files
committed
8301769: Generational ZGC: Indirect access barriers are never elided
Reviewed-by: eosterlund
1 parent 6600cff commit dc9fe9a

File tree

2 files changed

+70
-17
lines changed

2 files changed

+70
-17
lines changed

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

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -606,10 +606,24 @@ static bool is_concrete(intptr_t offset) {
606606
static const Node* get_base_and_offset(const MachNode* mach, intptr_t& offset) {
607607
const TypePtr* adr_type = NULL;
608608
offset = 0;
609-
const Node* const base = mach->get_base_and_disp(offset, adr_type);
609+
const Node* base = mach->get_base_and_disp(offset, adr_type);
610610

611-
if (base == NULL || base == NodeSentinel ||
612-
is_undefined(offset) || (is_concrete(offset) && offset < 0)) {
611+
if (base == NULL || base == NodeSentinel) {
612+
return NULL;
613+
}
614+
615+
if (offset == 0 && base->is_Mach() && base->as_Mach()->ideal_Opcode() == Op_AddP) {
616+
// The memory address is computed by 'base' and fed to 'mach' via an
617+
// indirect memory operand (indicated by offset == 0). The ultimate base and
618+
// offset can be fetched directly from the inputs and Ideal type of 'base'.
619+
offset = base->bottom_type()->isa_oopptr()->offset();
620+
// Even if 'base' is not an Ideal AddP node anymore, Matcher::ReduceInst()
621+
// guarantees that the base address is still available at the same slot.
622+
base = base->in(AddPNode::Base);
623+
assert(base != NULL, "");
624+
}
625+
626+
if (is_undefined(offset) || (is_concrete(offset) && offset < 0)) {
613627
return NULL;
614628
}
615629

test/hotspot/jtreg/compiler/gcbarriers/TestZGCBarrierElision.java

Lines changed: 53 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
* volatile memory accesses and blackholes to prevent C2 from simply
3535
* optimizing them away.
3636
* @library /test/lib /
37-
* @requires vm.gc.Z & os.simpleArch == "x64"
37+
* @requires vm.gc.Z & (vm.simpleArch == "x64" | vm.simpleArch == "aarch64")
3838
* @run driver compiler.gcbarriers.TestZGCBarrierElision
3939
*/
4040

@@ -303,30 +303,31 @@ static void testLoadThenAtomic(Outer o, Inner i) {
303303

304304
@Test
305305
@IR(counts = { IRNode.Z_STORE_P_WITH_BARRIER_FLAG, REMAINING, "1" }, phase = CompilePhase.FINAL_CODE)
306-
// The atomic access barrier should be elided, but is not.
306+
@IR(counts = { IRNode.Z_GET_AND_SET_P_WITH_BARRIER_FLAG, ELIDED, "1" }, phase = CompilePhase.FINAL_CODE)
307307
static void testStoreThenAtomic(Outer o, Inner i) {
308308
o.field1 = i;
309309
field1VarHandle.getAndSet​(o, i);
310310
}
311311

312312
@Test
313313
@IR(counts = { IRNode.Z_GET_AND_SET_P_WITH_BARRIER_FLAG, REMAINING, "1" }, phase = CompilePhase.FINAL_CODE)
314-
// The load barrier should be elided, but is not.
314+
@IR(counts = { IRNode.Z_LOAD_P_WITH_BARRIER_FLAG, ELIDED, "1" }, phase = CompilePhase.FINAL_CODE)
315315
static void testAtomicThenLoad(Outer o, Inner i) {
316316
field1VarHandle.getAndSet​(o, i);
317317
blackhole(o.field1);
318318
}
319319

320320
@Test
321321
@IR(counts = { IRNode.Z_GET_AND_SET_P_WITH_BARRIER_FLAG, REMAINING, "1" }, phase = CompilePhase.FINAL_CODE)
322-
// The store barrier should be elided, but is not.
322+
@IR(counts = { IRNode.Z_STORE_P_WITH_BARRIER_FLAG, ELIDED, "1" }, phase = CompilePhase.FINAL_CODE)
323323
static void testAtomicThenStore(Outer o, Inner i) {
324324
field1VarHandle.getAndSet​(o, i);
325325
o.field1 = i;
326326
}
327327

328328
@Test
329-
// The second atomic access barrier should be elided, but is not.
329+
@IR(counts = { IRNode.Z_GET_AND_SET_P_WITH_BARRIER_FLAG, REMAINING, "1" }, phase = CompilePhase.FINAL_CODE)
330+
@IR(counts = { IRNode.Z_GET_AND_SET_P_WITH_BARRIER_FLAG, ELIDED, "1" }, phase = CompilePhase.FINAL_CODE)
330331
static void testAtomicThenAtomic(Outer o, Inner i) {
331332
field1VarHandle.getAndSet​(o, i);
332333
field1VarHandle.getAndSet​(o, i);
@@ -339,21 +340,59 @@ static void testAtomicThenAtomicAnotherField(Outer o, Inner i) {
339340
field2VarHandle.getAndSet​(o, i);
340341
}
341342

343+
@Test
344+
@IR(counts = { IRNode.Z_GET_AND_SET_P_WITH_BARRIER_FLAG, REMAINING, "1" }, phase = CompilePhase.FINAL_CODE)
345+
static void testAllocateArrayThenAtomicAtKnownIndex(Outer o) {
346+
Outer[] a = new Outer[42];
347+
blackhole(a);
348+
outerArrayVarHandle.getAndSet(a, 2, o);
349+
}
350+
351+
@Test
352+
@IR(counts = { IRNode.Z_GET_AND_SET_P_WITH_BARRIER_FLAG, REMAINING, "1" }, phase = CompilePhase.FINAL_CODE)
353+
static void testAllocateArrayThenAtomicAtUnknownIndex(Outer o, int index) {
354+
Outer[] a = new Outer[42];
355+
blackhole(a);
356+
outerArrayVarHandle.getAndSet(a, index, o);
357+
}
358+
359+
@Test
360+
@IR(counts = { IRNode.Z_GET_AND_SET_P_WITH_BARRIER_FLAG, REMAINING, "1" }, phase = CompilePhase.FINAL_CODE)
361+
@IR(counts = { IRNode.Z_GET_AND_SET_P_WITH_BARRIER_FLAG, ELIDED, "1" }, phase = CompilePhase.FINAL_CODE)
362+
static void testArrayAtomicThenAtomic(Outer[] a, Outer o) {
363+
outerArrayVarHandle.getAndSet(a, 0, o);
364+
outerArrayVarHandle.getAndSet(a, 0, o);
365+
}
366+
367+
@Test
368+
@IR(counts = { IRNode.Z_GET_AND_SET_P_WITH_BARRIER_FLAG, REMAINING, "2" }, phase = CompilePhase.FINAL_CODE)
369+
static void testArrayAtomicThenAtomicAtUnknownIndices(Outer[] a, Outer o, int index1, int index2) {
370+
outerArrayVarHandle.getAndSet(a, index1, o);
371+
outerArrayVarHandle.getAndSet(a, index2, o);
372+
}
373+
342374
@Run(test = {"testAllocateThenAtomic",
343375
"testLoadThenAtomic",
344376
"testStoreThenAtomic",
345377
"testAtomicThenLoad",
346378
"testAtomicThenStore",
347379
"testAtomicThenAtomic",
348-
"testAtomicThenAtomicAnotherField"})
380+
"testAtomicThenAtomicAnotherField",
381+
"testAllocateArrayThenAtomicAtKnownIndex",
382+
"testAllocateArrayThenAtomicAtUnknownIndex",
383+
"testArrayAtomicThenAtomic",
384+
"testArrayAtomicThenAtomicAtUnknownIndices"})
349385
void runAtomicOperationTests() {
350-
testAllocateThenAtomic(inner);
351-
testLoadThenAtomic(outer, inner);
352-
testStoreThenAtomic(outer, inner);
353-
testAtomicThenLoad(outer, inner);
354-
testAtomicThenStore(outer, inner);
355-
testAtomicThenAtomic(outer, inner);
356-
testAtomicThenAtomicAnotherField(outer, inner);
386+
testAllocateThenAtomic(inner);
387+
testLoadThenAtomic(outer, inner);
388+
testStoreThenAtomic(outer, inner);
389+
testAtomicThenLoad(outer, inner);
390+
testAtomicThenStore(outer, inner);
391+
testAtomicThenAtomic(outer, inner);
392+
testAtomicThenAtomicAnotherField(outer, inner);
393+
testAllocateArrayThenAtomicAtKnownIndex(outer);
394+
testAllocateArrayThenAtomicAtUnknownIndex(outer, 10);
395+
testArrayAtomicThenAtomic(outerArray, outer);
396+
testArrayAtomicThenAtomicAtUnknownIndices(outerArray, outer, 10, 20);
357397
}
358-
359398
}

0 commit comments

Comments
 (0)