RFR: 8260473: [vector] ZGC: VectorReshape test produces incorrect results with ZGC enabled [v4]

[王超]

On Thu, 28 Jan 2021 21:36:39 GMT, Vladimir Ivanov <vlivanov at openjdk.org> wrote:

> > ArrayCopyNode::load performs the same work as it does here in PhaseVector::optimize_vector_boxes .
> > Is there a need to provide a similar function in PhaseVector or GraphKit?
> 
> My point is since PhaseVector effectively enters the parsing phase (by signaling about the possibility of post-parse inlining), technically I don't see why `GraphKit::access_load_at` won't work. But I need to spend more time looking into the details.
> 
> So far, I took a look at the review thread of 8212243 (which introduced `ArrayCopyNode::load`) and found the following discussion between Roland and Erik:
> https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2018-October/030971.html
> 
> ```
> > ... 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? ...
> ...
> GraphKit is a parse time only thing. So the existing gc interface
> doesn't offer any way to add barriers once parsing is over. This code
> runs after parsing in optimization phases.
> ...
> ```
> 
> Considering `PhaseVector::optimize_vector_boxes()` already has access to a usable `GraphKit` instance, it is possible that `GraphKit::access_load_at` will "just work".

`kit.access_load_at` can be called here and make the test pass, but will lead to another test(test/hotspot/jtreg/compiler/vectorapi/VectorReinterpretTest.java) fail, I'm trying to find out why.

diff --git a/src/hotspot/share/opto/vector.cpp b/src/hotspot/share/opto/vector.cpp
index 671083e..69c00c5 100644
--- a/src/hotspot/share/opto/vector.cpp
+++ b/src/hotspot/share/opto/vector.cpp
@@ -414,17 +414,12 @@ void PhaseVector::expand_vunbox_node(VectorUnboxNode* vec_unbox) {
 
     Node* mem = vec_unbox->mem();
     Node* ctrl = vec_unbox->in(0);
-    Node* vec_field_ld;
-    {
-      DecoratorSet decorators = C2_READ_ACCESS | C2_CONTROL_DEPENDENT_LOAD | IN_HEAP;
-      C2AccessValuePtr addr(vec_adr, vec_adr->bottom_type()->is_ptr());
-      MergeMemNode* local_mem = MergeMemNode::make(mem);
-      gvn.record_for_igvn(local_mem);
-      BarrierSetC2* bs = BarrierSet::barrier_set()->barrier_set_c2();
-      C2OptAccess access(gvn, ctrl, local_mem, decorators, T_OBJECT, obj, addr);
-      const Type* type = TypeOopPtr::make_from_klass(field->type()->as_klass());
-      vec_field_ld = bs->load_at(access, type);
-    }
+    Node* vec_field_ld = kit.access_load_at(obj,
+                                            vec_adr,
+                                            vec_adr->bottom_type()->is_ptr(),
+                                            TypeOopPtr::make_from_klass(field->type()->as_klass()),
+                                            T_OBJECT,
+                                            C2_READ_ACCESS | C2_CONTROL_DEPENDENT_LOAD | IN_HEAP);
 
     // For proper aliasing, attach concrete payload type.
     ciKlass* payload_klass = ciTypeArrayKlass::make(bt);

-------------

PR: https://git.openjdk.java.net/jdk/pull/2253