8371065: C2 SuperWord: VTransformLoopPhiNode::apply setting type leads to assert/wrong result

Co-authored-by: Roland Westrelin <roland@openjdk.org>
Reviewed-by: qamai, chagedorn

Author: eme64

src/hotspot/share/opto/superwordVTransformBuilder.cpp

@@ -64,7 +64,7 @@ void SuperWordVTransformBuilder::build_scalar_vtnodes_for_non_packed_nodes() {
       const VPointer& mem_p = _vloop_analyzer.vpointers().vpointer(mem);
       vtn = new (_vtransform.arena()) VTransformMemopScalarNode(_vtransform, mem, mem_p);
     } else if (n->is_Phi()) {
-      vtn = new (_vtransform.arena()) VTransformLoopPhiNode(_vtransform, n->as_Phi());
+      vtn = new (_vtransform.arena()) VTransformPhiScalarNode(_vtransform, n->as_Phi());
     } else if (n->is_CountedLoop()) {
       vtn = new (_vtransform.arena()) VTransformCountedLoopNode(_vtransform, n->as_CountedLoop());
     } else if (n->is_CFG()) {

src/hotspot/share/opto/vtransform.cpp

@@ -62,7 +62,7 @@ void VTransformGraph::optimize(VTransform& vtransform) {
           // 1. Memory phi uses are not modeled, so they appear to have no use here, but must be kept alive.
           // 2. Similarly, some stores may not have their memory uses modeled, but need to be kept alive.
           // 3. Outer node with strong inputs: is a use after the loop that we must keep alive.
-          !(vtn->isa_LoopPhi() != nullptr ||
+          !(vtn->isa_PhiScalar() != nullptr ||
             vtn->is_load_or_store_in_loop() ||
             (vtn->isa_Outer() != nullptr && vtn->has_strong_in_edge()))) {
         vtn->mark_dead();
@@ -123,8 +123,10 @@ bool VTransformGraph::schedule() {
         // Skip dead nodes
         if (!use->is_alive()) { continue; }
 
-        // Skip LoopPhi backedge.
-        if ((use->isa_LoopPhi() != nullptr || use->isa_CountedLoop() != nullptr) && use->in_req(2) == vtn) { continue; }
+        // Skip backedges.
+        if ((use->is_loop_head_phi() || use->isa_CountedLoop() != nullptr) && use->in_req(2) == vtn) {
+          continue;
+        }
 
         if (post_visited.test(use->_idx)) { continue; }
         if (pre_visited.test(use->_idx)) {
@@ -205,7 +207,7 @@ void VTransformGraph::mark_vtnodes_in_loop(VectorSet& in_loop) const {
   for (int i = 0; i < _schedule.length(); i++) {
     VTransformNode* vtn = _schedule.at(i);
     // Is vtn a loop-phi?
-    if (vtn->isa_LoopPhi() != nullptr ||
+    if (vtn->is_loop_head_phi() ||
         vtn->is_load_or_store_in_loop()) {
       is_not_before_loop.set(vtn->_idx);
       continue;
@@ -232,8 +234,7 @@ void VTransformGraph::mark_vtnodes_in_loop(VectorSet& in_loop) const {
     for (uint i = 0; i < vtn->out_strong_edges(); i++) {
       VTransformNode* use = vtn->out_strong_edge(i);
       // Or is vtn a backedge or one of its transitive defs?
-      if (in_loop.test(use->_idx) ||
-          use->isa_LoopPhi() != nullptr) {
+      if (in_loop.test(use->_idx) || use->is_loop_head_phi()) {
         in_loop.set(vtn->_idx);
         break;
       }
@@ -957,27 +958,22 @@ VTransformApplyResult VTransformDataScalarNode::apply(VTransformApplyState& appl
   return VTransformApplyResult::make_scalar(_node);
 }
 
-VTransformApplyResult VTransformLoopPhiNode::apply(VTransformApplyState& apply_state) const {
+VTransformApplyResult VTransformPhiScalarNode::apply(VTransformApplyState& apply_state) const {
   PhaseIdealLoop* phase = apply_state.phase();
   Node* in0 = apply_state.transformed_node(in_req(0));
   Node* in1 = apply_state.transformed_node(in_req(1));
   phase->igvn().replace_input_of(_node, 0, in0);
   phase->igvn().replace_input_of(_node, 1, in1);
   // Note: the backedge is hooked up later.
 
-  // The Phi's inputs may have been modified, and the types changes,
-  // e.g. from scalar to vector.
-  const Type* t = in1->bottom_type();
-  _node->as_Type()->set_type(t);
-  phase->igvn().set_type(_node, t);
-
   return VTransformApplyResult::make_scalar(_node);
 }
 
 // Cleanup backedges. In the schedule, the backedges come after their phis. Hence,
 // we only have the transformed backedges after the phis are already transformed.
 // We hook the backedges into the phis now, during cleanup.
-void VTransformLoopPhiNode::apply_backedge(VTransformApplyState& apply_state) const {
+void VTransformPhiScalarNode::apply_backedge(VTransformApplyState& apply_state) const {
+  assert(_node == apply_state.transformed_node(this), "sanity");
   PhaseIdealLoop* phase = apply_state.phase();
   if (_node->is_memory_phi()) {
     // Memory phi/backedge
@@ -1219,7 +1215,7 @@ bool VTransformReductionVectorNode::requires_strict_order() const {
 //       are performed inside the loop.
 bool VTransformReductionVectorNode::optimize_move_non_strict_order_reductions_out_of_loop_preconditions(VTransform& vtransform) {
   // We have a phi with a single use.
-  VTransformLoopPhiNode* phi = in_req(1)->isa_LoopPhi();
+  VTransformPhiScalarNode* phi = in_req(1)->isa_PhiScalar();
   if (phi == nullptr) {
     return false;
   }
@@ -1284,7 +1280,7 @@ bool VTransformReductionVectorNode::optimize_move_non_strict_order_reductions_ou
       // All uses must be outside loop body, except for the phi.
       for (uint i = 0; i < current_red->out_strong_edges(); i++) {
         VTransformNode* use = current_red->out_strong_edge(i);
-        if (use->isa_LoopPhi() == nullptr &&
+        if (use->isa_PhiScalar() == nullptr &&
             use->isa_Outer() == nullptr) {
           // Should not be allowed by SuperWord::mark_reductions
           assert(false, "reduction has use inside loop");
@@ -1339,16 +1335,28 @@ bool VTransformReductionVectorNode::optimize_move_non_strict_order_reductions_ou
   VTransformNode* vtn_identity_vector = new (vtransform.arena()) VTransformReplicateNode(vtransform, vlen, bt);
   vtn_identity_vector->init_req(1, vtn_identity);
 
-  // Turn the scalar phi into a vector phi.
-  VTransformLoopPhiNode* phi = in_req(1)->isa_LoopPhi();
-  VTransformNode* init = phi->in_req(1);
-  phi->set_req(1, vtn_identity_vector);
+  // Look at old scalar phi.
+  VTransformPhiScalarNode* phi_scalar = in_req(1)->isa_PhiScalar();
+  PhiNode* old_phi = phi_scalar->node();
+  VTransformNode* init = phi_scalar->in_req(1);
+
+  TRACE_OPTIMIZE(
+    tty->print("  phi_scalar ");
+    phi_scalar->print();
+  )
+
+  // Create new vector phi
+  const VTransformVectorNodeProperties properties = VTransformVectorNodeProperties::make_for_phi_vector(old_phi, vlen, bt);
+  VTransformPhiVectorNode* phi_vector = new (vtransform.arena()) VTransformPhiVectorNode(vtransform, 3, properties);
+  phi_vector->init_req(0, phi_scalar->in_req(0));
+  phi_vector->init_req(1, vtn_identity_vector);
+  // Note: backedge comes later
 
   // Traverse down the chain of reductions, and replace them with vector_accumulators.
   VTransformReductionVectorNode* first_red   = this;
-  VTransformReductionVectorNode* last_red    = phi->in_req(2)->isa_ReductionVector();
+  VTransformReductionVectorNode* last_red    = phi_scalar->in_req(2)->isa_ReductionVector();
   VTransformReductionVectorNode* current_red = first_red;
-  VTransformNode* current_vector_accumulator = phi;
+  VTransformNode* current_vector_accumulator = phi_vector;
   while (true) {
     VTransformNode* vector_input = current_red->in_req(2);
     VTransformVectorNode* vector_accumulator = new (vtransform.arena()) VTransformElementWiseVectorNode(vtransform, 3, current_red->properties(), vopc);
@@ -1366,15 +1374,15 @@ bool VTransformReductionVectorNode::optimize_move_non_strict_order_reductions_ou
   }
 
   // Feed vector accumulator into the backedge.
-  phi->set_req(2, current_vector_accumulator);
+  phi_vector->set_req(2, current_vector_accumulator);
 
   // Create post-loop reduction. last_red keeps all uses outside the loop.
   last_red->set_req(1, init);
   last_red->set_req(2, current_vector_accumulator);
 
   TRACE_OPTIMIZE(
-    tty->print("  phi        ");
-    phi->print();
+    tty->print("  phi_scalar ");
+    phi_scalar->print();
     tty->print("  after loop ");
     last_red->print();
   )
@@ -1398,6 +1406,37 @@ VTransformApplyResult VTransformReductionVectorNode::apply(VTransformApplyState&
   return VTransformApplyResult::make_vector(vn, vn->vect_type());
 }
 
+VTransformApplyResult VTransformPhiVectorNode::apply(VTransformApplyState& apply_state) const {
+  PhaseIdealLoop* phase = apply_state.phase();
+  Node* in0 = apply_state.transformed_node(in_req(0));
+  Node* in1 = apply_state.transformed_node(in_req(1));
+
+  // We create a new phi node, because the type is different to the scalar phi.
+  PhiNode* old_phi = approximate_origin()->as_Phi();
+  PhiNode* new_phi = old_phi->clone()->as_Phi();
+
+  phase->igvn().replace_input_of(new_phi, 0, in0);
+  phase->igvn().replace_input_of(new_phi, 1, in1);
+  // Note: the backedge is hooked up later.
+
+  // Give the new phi node the correct vector type.
+  const TypeVect* vt = TypeVect::make(element_basic_type(), vector_length());
+  new_phi->as_Type()->set_type(vt);
+  phase->igvn().set_type(new_phi, vt);
+
+  return VTransformApplyResult::make_vector(new_phi, vt);
+}
+
+// Cleanup backedges. In the schedule, the backedges come after their phis. Hence,
+// we only have the transformed backedges after the phis are already transformed.
+// We hook the backedges into the phis now, during cleanup.
+void VTransformPhiVectorNode::apply_backedge(VTransformApplyState& apply_state) const {
+  PhaseIdealLoop* phase = apply_state.phase();
+  PhiNode* new_phi = apply_state.transformed_node(this)->as_Phi();
+  Node* in2 = apply_state.transformed_node(in_req(2));
+  phase->igvn().replace_input_of(new_phi, 2, in2);
+}
+
 float VTransformLoadVectorNode::cost(const VLoopAnalyzer& vloop_analyzer) const {
   uint vlen    = vector_length();
   BasicType bt = element_basic_type();
@@ -1535,7 +1574,7 @@ void VTransformDataScalarNode::print_spec() const {
   tty->print("node[%d %s]", _node->_idx, _node->Name());
 }
 
-void VTransformLoopPhiNode::print_spec() const {
+void VTransformPhiScalarNode::print_spec() const {
   tty->print("node[%d %s]", _node->_idx, _node->Name());
 }
 

src/hotspot/share/opto/vtransform.hpp

@@ -79,7 +79,7 @@ class VTransform;
 class VTransformNode;
 class VTransformMemopScalarNode;
 class VTransformDataScalarNode;
-class VTransformLoopPhiNode;
+class VTransformPhiScalarNode;
 class VTransformCFGNode;
 class VTransformCountedLoopNode;
 class VTransformOuterNode;
@@ -88,6 +88,7 @@ class VTransformElementWiseVectorNode;
 class VTransformCmpVectorNode;
 class VTransformBoolVectorNode;
 class VTransformReductionVectorNode;
+class VTransformPhiVectorNode;
 class VTransformMemVectorNode;
 class VTransformLoadVectorNode;
 class VTransformStoreVectorNode;
@@ -539,21 +540,23 @@ class VTransformNode : public ArenaObj {
   }
 
   virtual VTransformMemopScalarNode* isa_MemopScalar() { return nullptr; }
-  virtual VTransformLoopPhiNode* isa_LoopPhi() { return nullptr; }
+  virtual VTransformPhiScalarNode* isa_PhiScalar() { return nullptr; }
   virtual VTransformCountedLoopNode* isa_CountedLoop() { return nullptr; }
   virtual VTransformOuterNode* isa_Outer() { return nullptr; }
   virtual VTransformVectorNode* isa_Vector() { return nullptr; }
   virtual VTransformElementWiseVectorNode* isa_ElementWiseVector() { return nullptr; }
   virtual VTransformCmpVectorNode* isa_CmpVector() { return nullptr; }
   virtual VTransformBoolVectorNode* isa_BoolVector() { return nullptr; }
   virtual VTransformReductionVectorNode* isa_ReductionVector() { return nullptr; }
+  virtual VTransformPhiVectorNode* isa_PhiVector() { return nullptr; }
   virtual VTransformMemVectorNode* isa_MemVector() { return nullptr; }
   virtual VTransformLoadVectorNode* isa_LoadVector() { return nullptr; }
   virtual VTransformStoreVectorNode* isa_StoreVector() { return nullptr; }
 
   virtual bool is_load_in_loop() const { return false; }
   virtual bool is_load_or_store_in_loop() const { return false; }
   virtual const VPointer& vpointer() const { ShouldNotReachHere(); }
+  virtual bool is_loop_head_phi() const { return false; }
 
   virtual bool optimize(const VLoopAnalyzer& vloop_analyzer, VTransform& vtransform) { return false; }
 
@@ -613,21 +616,24 @@ class VTransformDataScalarNode : public VTransformNode {
 };
 
 // Identity transform for loop head phi nodes.
-class VTransformLoopPhiNode : public VTransformNode {
+class VTransformPhiScalarNode : public VTransformNode {
 private:
   PhiNode* _node;
 public:
-  VTransformLoopPhiNode(VTransform& vtransform, PhiNode* n) :
+  VTransformPhiScalarNode(VTransform& vtransform, PhiNode* n) :
     VTransformNode(vtransform, n->req()), _node(n)
   {
     assert(_node->in(0)->is_Loop(), "phi ctrl must be Loop: %s", _node->in(0)->Name());
   }
 
-  virtual VTransformLoopPhiNode* isa_LoopPhi() override { return this; }
+  PhiNode* node() const { return _node; }
+
+  virtual VTransformPhiScalarNode* isa_PhiScalar() override { return this; }
+  virtual bool is_loop_head_phi() const override { return in_req(0)->isa_CountedLoop() != nullptr; }
   virtual float cost(const VLoopAnalyzer& vloop_analyzer) const override { return 0; }
   virtual VTransformApplyResult apply(VTransformApplyState& apply_state) const override;
   virtual void apply_backedge(VTransformApplyState& apply_state) const override;
-  NOT_PRODUCT(virtual const char* name() const override { return "LoopPhi"; };)
+  NOT_PRODUCT(virtual const char* name() const override { return "PhiScalar"; };)
   NOT_PRODUCT(virtual void print_spec() const override;)
 };
 
@@ -754,6 +760,10 @@ class VTransformVectorNodeProperties : public StackObj {
     return VTransformVectorNodeProperties(first, opc, vlen, bt);
   }
 
+  static VTransformVectorNodeProperties make_for_phi_vector(PhiNode* phi, int vlen, BasicType bt) {
+    return VTransformVectorNodeProperties(phi, phi->Opcode(), vlen, bt);
+  }
+
   Node* approximate_origin()     const { return _approximate_origin; }
   int scalar_opcode()            const { return _scalar_opcode; }
   uint vector_length()           const { return _vector_length; }
@@ -870,6 +880,18 @@ class VTransformReductionVectorNode : public VTransformVectorNode {
   bool optimize_move_non_strict_order_reductions_out_of_loop(const VLoopAnalyzer& vloop_analyzer, VTransform& vtransform);
 };
 
+class VTransformPhiVectorNode : public VTransformVectorNode {
+public:
+  VTransformPhiVectorNode(VTransform& vtransform, uint req, const VTransformVectorNodeProperties properties) :
+    VTransformVectorNode(vtransform, req, properties) {}
+  virtual VTransformPhiVectorNode* isa_PhiVector() override { return this; }
+  virtual bool is_loop_head_phi() const override { return in_req(0)->isa_CountedLoop() != nullptr; }
+  virtual float cost(const VLoopAnalyzer& vloop_analyzer) const override { return 0; }
+  virtual VTransformApplyResult apply(VTransformApplyState& apply_state) const override;
+  virtual void apply_backedge(VTransformApplyState& apply_state) const override;
+  NOT_PRODUCT(virtual const char* name() const override { return "PhiVector"; };)
+};
+
 class VTransformMemVectorNode : public VTransformVectorNode {
 private:
   const VPointer _vpointer; // with size of the vector