RFR(M): 8212243: More gc interface tweaks for arraycopy and some other smaller changes in preparation for Shenandoah

[Erik Österlund]

Hi Roland,

One goal I have had in the barrier set classes is to not introduce the 
concepts of pre/post (write) barriers until the ModRefBarrierSet layer, 
as it is not a concept that is at all shared for our GCs. Therefore, I 
would prefer if this:

1141 if (clone_needs_postbarrier(ac)) {
1142 const TypePtr* raw_adr_type = TypeRawPtr::BOTTOM;
1143 Node* c = new ProjNode(call,TypeFunc::Control);
1144 transform_later(c);
1145 Node* m = new ProjNode(call, TypeFunc::Memory);
1146 transform_later(m);
1147 BarrierSetC2* bs = BarrierSet::barrier_set()->barrier_set_c2();
1148 bs->array_copy_post_barrier_at_expansion(ac, c, m, _igvn);
1149 Node* out_c = ac->proj_out(TypeFunc::Control);
1150 Node* out_m = ac->proj_out(TypeFunc::Memory);
1151 _igvn.replace_node(out_c, c);
1152 _igvn.replace_node(out_m, m);
1153 } else {
1154       _igvn.replace_node(ac, call);
1155 } and the code it calls: 1085 bool 
PhaseMacroExpand::clone_needs_postbarrier(ArrayCopyNode *ac) {
1086 BarrierSetC2* bs = BarrierSet::barrier_set()->barrier_set_c2();
1087 if (!bs->array_copy_requires_gc_barriers(true, T_OBJECT, true, 
BarrierSetC2::Expansion)) {
1088 return false;
1089 }
1090
1091 Node* src = ac->in(ArrayCopyNode::Src);
1092 const TypeOopPtr* src_type = _igvn.type(src)->is_oopptr();
1093 if (src_type->isa_instptr() != NULL) {
1094 ciInstanceKlass* ik = src_type->klass()->as_instance_klass();
1095 if ((src_type->klass_is_exact() || (!ik->is_interface() && 
!ik->has_subklass())) && !ik->has_injected_fields()) {
1096 if (ik->has_object_fields()) {
1097 return true;
1098 } else {
1099 if (!src_type->klass_is_exact()) {
1100 C->dependencies()->assert_leaf_type(ik);
1101 }
1102 }
1103 } else {
1104 return true;
1105 }
1106 } else if (src_type->isa_aryptr()) {
1107 BasicType src_elem = 
src_type->klass()->as_array_klass()->element_type()->basic_type();
1108 if (src_elem == T_OBJECT || src_elem == T_ARRAY) {
1109 return true;
1110 }
1111 } else {
1112 return true;
1113 }
1114 return false;
1115 } ...and the code that calls:
virtual void array_copy_post_barrier_at_expansion(ArrayCopyNode* ac, 
Node*& c, Node*& m, PhaseIterGVN& igvn) const { } ...to all be moved 
into ModRefBarrierSetC2. I'm imagining to on BarrierSetC2 only expose 
something like clone_barrier_at_expansion() that gets overridden by 
ModRefBarrierSetC2 to do the check if post_barriers are needed, and then 
call the "array_copy_post_barrier_at_expansion" member function of 
ModRefBarrierSetC2, that looks like it should also be renamed to 
clone_post_barrier_at_expansion.

As for

virtual Node* array_copy_load_store_barrier(PhaseGVN *phase, bool 
can_reshape, Node* v, MergeMemNode* mem, Node*& ctl) const { return v; } 
...I would prefer to pass in the BasicType, and perform the T_OBJECT 
filtering inside, for symmetry with other similar hooks. Also it beats 
me that this is strictly speaking a load barrier for loads performed in 
arraycopy. Would it be possible to use something like access_load_at 
instead? That would have been fantastic. Or is there a mis-fit, such as 
not having a GraphKit with memory state? If so, it would be interesting 
to see what it would take to fix that. I believe that hook could be 
useful for ZGC cloning as well. Thanks, /Erik

On 2018-10-17 17:44, Roland Westrelin wrote:
> Hi Vlaimir,
>
> Thanks for looking at this. Here is a new webrev that isolates the array
> copy related changes and implements your suggestions:
>
> http://cr.openjdk.java.net/~roland/8212243/webrev.01/
>
> Roland.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20181017/e3833256/attachment-0001.html>