Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
8267726: ZGC: array_copy_requires_gc_barriers too strict
Reviewed-by: thartmann, vlivanov
  • Loading branch information
Nils Eliasson committed Jun 2, 2021
1 parent d47a77d commit bba3728
Show file tree
Hide file tree
Showing 10 changed files with 21 additions and 12 deletions.
2 changes: 1 addition & 1 deletion src/hotspot/share/gc/shared/c2/barrierSetC2.hpp
Expand Up @@ -254,7 +254,7 @@ class BarrierSetC2: public CHeapObj<mtGC> {
Expansion
};

virtual bool array_copy_requires_gc_barriers(bool tightly_coupled_alloc, BasicType type, bool is_clone, ArrayCopyPhase phase) const { return false; }
virtual bool array_copy_requires_gc_barriers(bool tightly_coupled_alloc, BasicType type, bool is_clone, bool is_clone_instance, ArrayCopyPhase phase) const { return false; }
virtual void clone_at_expansion(PhaseMacroExpand* phase, ArrayCopyNode* ac) const;

// Support for GC barriers emitted during parsing
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/gc/shared/c2/cardTableBarrierSetC2.cpp
Expand Up @@ -176,7 +176,7 @@ void CardTableBarrierSetC2::eliminate_gc_barrier(PhaseMacroExpand* macro, Node*
}
}

bool CardTableBarrierSetC2::array_copy_requires_gc_barriers(bool tightly_coupled_alloc, BasicType type, bool is_clone, ArrayCopyPhase phase) const {
bool CardTableBarrierSetC2::array_copy_requires_gc_barriers(bool tightly_coupled_alloc, BasicType type, bool is_clone, bool is_clone_instance, ArrayCopyPhase phase) const {
bool is_oop = is_reference_type(type);
return is_oop && (!tightly_coupled_alloc || !use_ReduceInitialCardMarks());
}
2 changes: 1 addition & 1 deletion src/hotspot/share/gc/shared/c2/cardTableBarrierSetC2.hpp
Expand Up @@ -45,7 +45,7 @@ class CardTableBarrierSetC2: public ModRefBarrierSetC2 {
virtual void clone(GraphKit* kit, Node* src, Node* dst, Node* size, bool is_array) const;
virtual bool is_gc_barrier_node(Node* node) const;
virtual void eliminate_gc_barrier(PhaseMacroExpand* macro, Node* node) const;
virtual bool array_copy_requires_gc_barriers(bool tightly_coupled_alloc, BasicType type, bool is_clone, ArrayCopyPhase phase) const;
virtual bool array_copy_requires_gc_barriers(bool tightly_coupled_alloc, BasicType type, bool is_clone, bool is_clone_instance, ArrayCopyPhase phase) const;

bool use_ReduceInitialCardMarks() const;
};
Expand Down
Expand Up @@ -763,7 +763,7 @@ bool ShenandoahBarrierSetC2::optimize_loops(PhaseIdealLoop* phase, LoopOptsMode
return false;
}

bool ShenandoahBarrierSetC2::array_copy_requires_gc_barriers(bool tightly_coupled_alloc, BasicType type, bool is_clone, ArrayCopyPhase phase) const {
bool ShenandoahBarrierSetC2::array_copy_requires_gc_barriers(bool tightly_coupled_alloc, BasicType type, bool is_clone, bool is_clone_instance, ArrayCopyPhase phase) const {
bool is_oop = is_reference_type(type);
if (!is_oop) {
return false;
Expand Down
Expand Up @@ -109,7 +109,7 @@ class ShenandoahBarrierSetC2 : public BarrierSetC2 {
virtual void clone_at_expansion(PhaseMacroExpand* phase, ArrayCopyNode* ac) const;

// These are general helper methods used by C2
virtual bool array_copy_requires_gc_barriers(bool tightly_coupled_alloc, BasicType type, bool is_clone, ArrayCopyPhase phase) const;
virtual bool array_copy_requires_gc_barriers(bool tightly_coupled_alloc, BasicType type, bool is_clone, bool is_clone_instance, ArrayCopyPhase phase) const;

// Support for GC barriers emitted during parsing
virtual bool is_gc_barrier_node(Node* node) const;
Expand Down
10 changes: 9 additions & 1 deletion src/hotspot/share/gc/z/c2/zBarrierSetC2.cpp
Expand Up @@ -224,7 +224,15 @@ Node* ZBarrierSetC2::atomic_xchg_at_resolved(C2AtomicParseAccess& access, Node*
}

bool ZBarrierSetC2::array_copy_requires_gc_barriers(bool tightly_coupled_alloc, BasicType type,
bool is_clone, ArrayCopyPhase phase) const {
bool is_clone, bool is_clone_instance,
ArrayCopyPhase phase) const {
if (phase == ArrayCopyPhase::Parsing) {
return false;
}
if (phase == ArrayCopyPhase::Optimization) {
return is_clone_instance;
}
// else ArrayCopyPhase::Expansion
return type == T_OBJECT || type == T_ARRAY;
}

Expand Down
1 change: 1 addition & 0 deletions src/hotspot/share/gc/z/c2/zBarrierSetC2.hpp
Expand Up @@ -83,6 +83,7 @@ class ZBarrierSetC2 : public BarrierSetC2 {
virtual bool array_copy_requires_gc_barriers(bool tightly_coupled_alloc,
BasicType type,
bool is_clone,
bool is_clone_instance,
ArrayCopyPhase phase) const;
virtual void clone_at_expansion(PhaseMacroExpand* phase,
ArrayCopyNode* ac) const;
Expand Down
8 changes: 4 additions & 4 deletions src/hotspot/share/opto/arraycopynode.cpp
Expand Up @@ -276,7 +276,7 @@ bool ArrayCopyNode::prepare_array_copy(PhaseGVN *phase, bool can_reshape,
}

BarrierSetC2* bs = BarrierSet::barrier_set()->barrier_set_c2();
if (bs->array_copy_requires_gc_barriers(is_alloc_tightly_coupled(), dest_elem, false, BarrierSetC2::Optimization)) {
if (bs->array_copy_requires_gc_barriers(is_alloc_tightly_coupled(), dest_elem, false, false, BarrierSetC2::Optimization)) {
// It's an object array copy but we can't emit the card marking
// that is needed
return false;
Expand Down Expand Up @@ -319,7 +319,7 @@ bool ArrayCopyNode::prepare_array_copy(PhaseGVN *phase, bool can_reshape,
}

BarrierSetC2* bs = BarrierSet::barrier_set()->barrier_set_c2();
if (bs->array_copy_requires_gc_barriers(true, elem, true, BarrierSetC2::Optimization)) {
if (bs->array_copy_requires_gc_barriers(true, elem, true, is_clone_inst(), BarrierSetC2::Optimization)) {
return false;
}

Expand Down Expand Up @@ -423,7 +423,7 @@ Node* ArrayCopyNode::array_copy_backward(PhaseGVN *phase,
MergeMemNode* mm = MergeMemNode::make(mem);

BarrierSetC2* bs = BarrierSet::barrier_set()->barrier_set_c2();
assert(copy_type != T_OBJECT || !bs->array_copy_requires_gc_barriers(false, T_OBJECT, false, BarrierSetC2::Optimization), "only tightly coupled allocations for object arrays");
assert(copy_type != T_OBJECT || !bs->array_copy_requires_gc_barriers(false, T_OBJECT, false, false, BarrierSetC2::Optimization), "only tightly coupled allocations for object arrays");

if (count > 0) {
for (int i = count-1; i >= 1; i--) {
Expand Down Expand Up @@ -456,7 +456,7 @@ bool ArrayCopyNode::finish_transform(PhaseGVN *phase, bool can_reshape,
BarrierSetC2* bs = BarrierSet::barrier_set()->barrier_set_c2();
if (out_mem->outcnt() != 1 || !out_mem->raw_out(0)->is_MergeMem() ||
out_mem->raw_out(0)->outcnt() != 1 || !out_mem->raw_out(0)->raw_out(0)->is_MemBar()) {
assert(bs->array_copy_requires_gc_barriers(true, T_OBJECT, true, BarrierSetC2::Optimization), "can only happen with card marking");
assert(bs->array_copy_requires_gc_barriers(true, T_OBJECT, true, is_clone_inst(), BarrierSetC2::Optimization), "can only happen with card marking");
return false;
}

Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/opto/library_call.cpp
Expand Up @@ -4233,7 +4233,7 @@ bool LibraryCallKit::inline_native_clone(bool is_virtual) {
Node* alloc_obj = new_array(obj_klass, obj_length, 0, &obj_size, /*deoptimize_on_exception=*/true);

BarrierSetC2* bs = BarrierSet::barrier_set()->barrier_set_c2();
if (bs->array_copy_requires_gc_barriers(true, T_OBJECT, true, BarrierSetC2::Parsing)) {
if (bs->array_copy_requires_gc_barriers(true, T_OBJECT, true, false, BarrierSetC2::Parsing)) {
// If it is an oop array, it requires very special treatment,
// because gc barriers are required when accessing the array.
Node* is_obja = generate_objArray_guard(obj_klass, (RegionNode*)NULL);
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/opto/macroArrayCopy.cpp
Expand Up @@ -656,7 +656,7 @@ Node* PhaseMacroExpand::generate_arraycopy(ArrayCopyNode *ac, AllocateArrayNode*
// At this point we know we do not need type checks on oop stores.

BarrierSetC2* bs = BarrierSet::barrier_set()->barrier_set_c2();
if (!bs->array_copy_requires_gc_barriers(alloc != NULL, copy_type, false, BarrierSetC2::Expansion)) {
if (!bs->array_copy_requires_gc_barriers(alloc != NULL, copy_type, false, false, BarrierSetC2::Expansion)) {
// If we do not need gc barriers, copy using the jint or jlong stub.
copy_type = LP64_ONLY(UseCompressedOops ? T_INT : T_LONG) NOT_LP64(T_INT);
assert(type2aelembytes(basic_elem_type) == type2aelembytes(copy_type),
Expand Down

1 comment on commit bba3728

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.