Skip to content

Commit 0106db0

Browse files
TheRealMDoerrTheRealMDoerr
committed
8320807: [PPC64][ZGC] C1 generates wrong code for atomics
Backport-of: 3087e14cde9257680f0406b11942f9cb7739cb7b
1 parent 0e0e897 commit 0106db0

File tree

4 files changed

+51
-79
lines changed

4 files changed

+51
-79
lines changed

‎src/hotspot/cpu/ppc/c1_LIRAssembler_ppc.cpp‎

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2635,6 +2635,13 @@ void LIR_Assembler::emit_compare_and_swap(LIR_OpCompareAndSwap* op) {
26352635
Unimplemented();
26362636
}
26372637

2638+
// There might be a volatile load before this Unsafe CAS.
2639+
if (support_IRIW_for_not_multiple_copy_atomic_cpu) {
2640+
__ sync();
2641+
} else {
2642+
__ lwsync();
2643+
}
2644+
26382645
if (is_64bit) {
26392646
__ cmpxchgd(BOOL_RESULT, /*current_value=*/R0, cmp_value, new_value, addr,
26402647
MacroAssembler::MemBarNone,
@@ -2996,9 +3003,24 @@ void LIR_Assembler::atomic_op(LIR_Code code, LIR_Opr src, LIR_Opr data, LIR_Opr
29963003
assert(addr->disp() == 0 && addr->index()->is_illegal(), "use leal!");
29973004
const Register Rptr = addr->base()->as_pointer_register(),
29983005
Rtmp = tmp->as_register();
2999-
Register Rco = noreg;
3000-
if (UseCompressedOops && data->is_oop()) {
3001-
Rco = __ encode_heap_oop(Rtmp, data->as_register());
3006+
Register Robj = noreg;
3007+
if (data->is_oop()) {
3008+
if (UseCompressedOops) {
3009+
Robj = __ encode_heap_oop(Rtmp, data->as_register());
3010+
} else {
3011+
Robj = data->as_register();
3012+
if (Robj == dest->as_register()) { // May happen with ZGC.
3013+
__ mr(Rtmp, Robj);
3014+
Robj = Rtmp;
3015+
}
3016+
}
3017+
}
3018+
3019+
// There might be a volatile load before this Unsafe OP.
3020+
if (support_IRIW_for_not_multiple_copy_atomic_cpu) {
3021+
__ sync();
3022+
} else {
3023+
__ lwsync();
30023024
}
30033025

30043026
Label Lretry;
@@ -3018,18 +3040,11 @@ void LIR_Assembler::atomic_op(LIR_Code code, LIR_Opr src, LIR_Opr data, LIR_Opr
30183040
} else if (data->is_oop()) {
30193041
assert(code == lir_xchg, "xadd for oops");
30203042
const Register Rold = dest->as_register();
3043+
assert_different_registers(Rptr, Rold, Robj);
30213044
if (UseCompressedOops) {
3022-
assert_different_registers(Rptr, Rold, Rco);
30233045
__ lwarx(Rold, Rptr, MacroAssembler::cmpxchgx_hint_atomic_update());
3024-
__ stwcx_(Rco, Rptr);
3046+
__ stwcx_(Robj, Rptr);
30253047
} else {
3026-
Register Robj = data->as_register();
3027-
assert_different_registers(Rptr, Rold, Rtmp);
3028-
assert_different_registers(Rptr, Robj, Rtmp);
3029-
if (Robj == Rold) { // May happen with ZGC.
3030-
__ mr(Rtmp, Robj);
3031-
Robj = Rtmp;
3032-
}
30333048
__ ldarx(Rold, Rptr, MacroAssembler::cmpxchgx_hint_atomic_update());
30343049
__ stdcx_(Robj, Rptr);
30353050
}
@@ -3057,6 +3072,12 @@ void LIR_Assembler::atomic_op(LIR_Code code, LIR_Opr src, LIR_Opr data, LIR_Opr
30573072
if (UseCompressedOops && data->is_oop()) {
30583073
__ decode_heap_oop(dest->as_register());
30593074
}
3075+
3076+
if (support_IRIW_for_not_multiple_copy_atomic_cpu) {
3077+
__ isync();
3078+
} else {
3079+
__ sync();
3080+
}
30603081
}
30613082

30623083

‎src/hotspot/cpu/ppc/c1_LIRGenerator_ppc.cpp‎

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -639,13 +639,6 @@ LIR_Opr LIRGenerator::atomic_cmpxchg(BasicType type, LIR_Opr addr, LIRItem& cmp_
639639
cmp_value.load_item();
640640
new_value.load_item();
641641

642-
// Volatile load may be followed by Unsafe CAS.
643-
if (support_IRIW_for_not_multiple_copy_atomic_cpu) {
644-
__ membar();
645-
} else {
646-
__ membar_release();
647-
}
648-
649642
if (is_reference_type(type)) {
650643
if (UseCompressedOops) {
651644
t1 = new_register(T_OBJECT);
@@ -670,21 +663,7 @@ LIR_Opr LIRGenerator::atomic_xchg(BasicType type, LIR_Opr addr, LIRItem& value)
670663
LIR_Opr tmp = FrameMap::R0_opr;
671664

672665
value.load_item();
673-
674-
// Volatile load may be followed by Unsafe CAS.
675-
if (support_IRIW_for_not_multiple_copy_atomic_cpu) {
676-
__ membar();
677-
} else {
678-
__ membar_release();
679-
}
680-
681666
__ xchg(addr, value.result(), result, tmp);
682-
683-
if (support_IRIW_for_not_multiple_copy_atomic_cpu) {
684-
__ membar_acquire();
685-
} else {
686-
__ membar();
687-
}
688667
return result;
689668
}
690669

@@ -694,21 +673,7 @@ LIR_Opr LIRGenerator::atomic_add(BasicType type, LIR_Opr addr, LIRItem& value) {
694673
LIR_Opr tmp = FrameMap::R0_opr;
695674

696675
value.load_item();
697-
698-
// Volatile load may be followed by Unsafe CAS.
699-
if (support_IRIW_for_not_multiple_copy_atomic_cpu) {
700-
__ membar(); // To be safe. Unsafe semantics are unclear.
701-
} else {
702-
__ membar_release();
703-
}
704-
705676
__ xadd(addr, value.result(), result, tmp);
706-
707-
if (support_IRIW_for_not_multiple_copy_atomic_cpu) {
708-
__ membar_acquire();
709-
} else {
710-
__ membar();
711-
}
712677
return result;
713678
}
714679

‎src/hotspot/cpu/ppc/gc/shenandoah/c1/shenandoahBarrierSetC1_ppc.cpp‎

Lines changed: 15 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/*
2-
* Copyright (c) 2018, 2021, Red Hat, Inc. All rights reserved.
3-
* Copyright (c) 2012, 2021 SAP SE. All rights reserved.
2+
* Copyright (c) 2018, 2023, Red Hat, Inc. All rights reserved.
3+
* Copyright (c) 2012, 2023 SAP SE. All rights reserved.
44
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
55
*
66
* This code is free software; you can redistribute it and/or modify it
@@ -53,8 +53,13 @@ void LIR_OpShenandoahCompareAndSwap::emit_code(LIR_Assembler *masm) {
5353
__ encode_heap_oop(new_val, new_val);
5454
}
5555

56-
// Due to the memory barriers emitted in ShenandoahBarrierSetC1::atomic_cmpxchg_at_resolved,
57-
// there is no need to specify stronger memory semantics.
56+
// There might be a volatile load before this Unsafe CAS.
57+
if (support_IRIW_for_not_multiple_copy_atomic_cpu) {
58+
__ sync();
59+
} else {
60+
__ lwsync();
61+
}
62+
5863
ShenandoahBarrierSet::assembler()->cmpxchg_oop(masm->masm(), addr, cmp_val, new_val, tmp1, tmp2,
5964
false, result);
6065

@@ -63,6 +68,12 @@ void LIR_OpShenandoahCompareAndSwap::emit_code(LIR_Assembler *masm) {
6368
__ decode_heap_oop(new_val);
6469
}
6570

71+
if (support_IRIW_for_not_multiple_copy_atomic_cpu) {
72+
__ isync();
73+
} else {
74+
__ sync();
75+
}
76+
6677
__ block_comment("} LIR_OpShenandoahCompareAndSwap (shenandaohgc)");
6778
}
6879

@@ -80,14 +91,6 @@ LIR_Opr ShenandoahBarrierSetC1::atomic_cmpxchg_at_resolved(LIRAccess &access, LI
8091
if (access.is_oop()) {
8192
LIRGenerator* gen = access.gen();
8293

83-
if (ShenandoahCASBarrier) {
84-
if (support_IRIW_for_not_multiple_copy_atomic_cpu) {
85-
__ membar();
86-
} else {
87-
__ membar_release();
88-
}
89-
}
90-
9194
if (ShenandoahSATBBarrier) {
9295
pre_barrier(gen, access.access_emit_info(), access.decorators(), access.resolved_addr(),
9396
LIR_OprFact::illegalOpr);
@@ -104,12 +107,6 @@ LIR_Opr ShenandoahBarrierSetC1::atomic_cmpxchg_at_resolved(LIRAccess &access, LI
104107

105108
__ append(new LIR_OpShenandoahCompareAndSwap(addr, cmp_value.result(), new_value.result(), t1, t2, result));
106109

107-
if (support_IRIW_for_not_multiple_copy_atomic_cpu) {
108-
__ membar_acquire();
109-
} else {
110-
__ membar();
111-
}
112-
113110
return result;
114111
}
115112
}
@@ -125,12 +122,6 @@ LIR_Opr ShenandoahBarrierSetC1::atomic_xchg_at_resolved(LIRAccess &access, LIRIt
125122
value.load_item();
126123
LIR_Opr value_opr = value.result();
127124

128-
if (support_IRIW_for_not_multiple_copy_atomic_cpu) {
129-
__ membar();
130-
} else {
131-
__ membar_release();
132-
}
133-
134125
if (access.is_oop()) {
135126
value_opr = iu_barrier(access.gen(), value_opr, access.access_emit_info(), access.decorators());
136127
}
@@ -152,11 +143,5 @@ LIR_Opr ShenandoahBarrierSetC1::atomic_xchg_at_resolved(LIRAccess &access, LIRIt
152143
}
153144
}
154145

155-
if (support_IRIW_for_not_multiple_copy_atomic_cpu) {
156-
__ membar_acquire();
157-
} else {
158-
__ membar();
159-
}
160-
161146
return result;
162147
}

‎src/hotspot/cpu/ppc/gc/z/zBarrierSetAssembler_ppc.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -340,11 +340,12 @@ void ZBarrierSetAssembler::store_barrier_medium(MacroAssembler* masm,
340340
}
341341
__ ld(R0, in_bytes(ZThreadLocalData::store_good_mask_offset()), R16_thread);
342342
__ cmpxchgd(CCR0, tmp, (intptr_t)0, R0, ref_base,
343-
MacroAssembler::MemBarNone, MacroAssembler::cmpxchgx_hint_atomic_update());
343+
MacroAssembler::MemBarNone, MacroAssembler::cmpxchgx_hint_atomic_update(),
344+
noreg, need_restore ? nullptr : &slow_path);
344345
if (need_restore) {
345346
__ subf(ref_base, ind_or_offs, ref_base);
347+
__ bne(CCR0, slow_path);
346348
}
347-
__ bne(CCR0, slow_path);
348349
} else {
349350
// A non-atomic relocatable object won't get to the medium fast path due to a
350351
// raw null in the young generation. We only get here because the field is bad.

0 commit comments

Comments
 (0)