8152773: C2: LoadNode properties aren't preserved when converting between signed/unsigned variants

Reviewed-by: jrose, kvn

hotspot/src/share/vm/opto/memnode.cpp

@@ -742,7 +742,7 @@ void LoadNode::dump_spec(outputStream *st) const {
     // standard dump does this in Verbose and WizardMode
     st->print(" #"); _type->dump_on(st);
   }
-  if (!_depends_only_on_test) {
+  if (!depends_only_on_test()) {
     st->print(" (does not depend only on test)");
   }
 }
@@ -914,7 +914,7 @@ Node* LoadNode::can_see_arraycopy_value(Node* st, PhaseTransform* phase) const {
       }
     }
     // load depends on the tests that validate the arraycopy
-    ld->as_Load()->_depends_only_on_test = Pinned;
+    ld->as_Load()->_control_dependency = Pinned;
     return ld;
   }
   return NULL;
@@ -1118,6 +1118,44 @@ Node* LoadNode::Identity(PhaseGVN* phase) {
   return this;
 }
 
+// Construct an equivalent unsigned load.
+Node* LoadNode::convert_to_unsigned_load(PhaseGVN& gvn) {
+  BasicType bt = T_ILLEGAL;
+  const Type* rt = NULL;
+  switch (Opcode()) {
+    case Op_LoadUB: return this;
+    case Op_LoadUS: return this;
+    case Op_LoadB: bt = T_BOOLEAN; rt = TypeInt::UBYTE; break;
+    case Op_LoadS: bt = T_CHAR;    rt = TypeInt::CHAR;  break;
+    default:
+      assert(false, "no unsigned variant: %s", Name());
+      return NULL;
+  }
+  return LoadNode::make(gvn, in(MemNode::Control), in(MemNode::Memory), in(MemNode::Address),
+                        adr_type(), rt, bt, _mo, _control_dependency,
+                        is_unaligned_access(), is_mismatched_access());
+}
+
+// Construct an equivalent signed load.
+Node* LoadNode::convert_to_signed_load(PhaseGVN& gvn) {
+  BasicType bt = T_ILLEGAL;
+  const Type* rt = NULL;
+  switch (Opcode()) {
+    case Op_LoadUB: bt = T_BYTE;  rt = TypeInt::BYTE;  break;
+    case Op_LoadUS: bt = T_SHORT; rt = TypeInt::SHORT; break;
+    case Op_LoadB: // fall through
+    case Op_LoadS: // fall through
+    case Op_LoadI: // fall through
+    case Op_LoadL: return this;
+    default:
+      assert(false, "no signed variant: %s", Name());
+      return NULL;
+  }
+  return LoadNode::make(gvn, in(MemNode::Control), in(MemNode::Memory), in(MemNode::Address),
+                        adr_type(), rt, bt, _mo, _control_dependency,
+                        is_unaligned_access(), is_mismatched_access());
+}
+
 // We're loading from an object which has autobox behaviour.
 // If this object is result of a valueOf call we'll have a phi
 // merging a newly allocated object and a load from the cache.

hotspot/src/share/vm/opto/memnode.hpp

@@ -148,9 +148,8 @@ public:
 class LoadNode : public MemNode {
 public:
   // Some loads (from unsafe) should be pinned: they don't depend only
-  // on the dominating test.  The boolean field _depends_only_on_test
-  // below records whether that node depends only on the dominating
-  // test.
+  // on the dominating test.  The field _control_dependency below records
+  // whether that node depends only on the dominating test.
   // Methods used to build LoadNodes pass an argument of type enum
   // ControlDependency instead of a boolean because those methods
   // typically have multiple boolean parameters with default values:
@@ -162,7 +161,7 @@ public:
     DependsOnlyOnTest
   };
 private:
-  // LoadNode::hash() doesn't take the _depends_only_on_test field
+  // LoadNode::hash() doesn't take the _control_dependency field
   // into account: If the graph already has a non-pinned LoadNode and
   // we add a pinned LoadNode with the same inputs, it's safe for GVN
   // to replace the pinned LoadNode with the non-pinned LoadNode,
@@ -171,7 +170,7 @@ private:
   // pinned LoadNode and we add a non pinned LoadNode with the same
   // inputs, it's safe (but suboptimal) for GVN to replace the
   // non-pinned LoadNode by the pinned LoadNode.
-  bool _depends_only_on_test;
+  ControlDependency _control_dependency;
 
   // On platforms with weak memory ordering (e.g., PPC, Ia64) we distinguish
   // loads that can be reordered, and such requiring acquire semantics to
@@ -190,7 +189,7 @@ protected:
 public:
 
   LoadNode(Node *c, Node *mem, Node *adr, const TypePtr* at, const Type *rt, MemOrd mo, ControlDependency control_dependency)
-    : MemNode(c,mem,adr,at), _type(rt), _mo(mo), _depends_only_on_test(control_dependency == DependsOnlyOnTest) {
+    : MemNode(c,mem,adr,at), _type(rt), _mo(mo), _control_dependency(control_dependency) {
     init_class_id(Class_Load);
   }
   inline bool is_unordered() const { return !is_acquire(); }
@@ -252,6 +251,9 @@ public:
   // Check if the load's memory input is a Phi node with the same control.
   bool is_instance_field_load_with_local_phi(Node* ctrl);
 
+  Node* convert_to_unsigned_load(PhaseGVN& gvn);
+  Node* convert_to_signed_load(PhaseGVN& gvn);
+
 #ifndef PRODUCT
   virtual void dump_spec(outputStream *st) const;
 #endif
@@ -274,7 +276,9 @@ protected:
   // which produce results (new raw memory state) inside of loops preventing all
   // manner of other optimizations).  Basically, it's ugly but so is the alternative.
   // See comment in macro.cpp, around line 125 expand_allocate_common().
-  virtual bool depends_only_on_test() const { return adr_type() != TypeRawPtr::BOTTOM && _depends_only_on_test; }
+  virtual bool depends_only_on_test() const {
+    return adr_type() != TypeRawPtr::BOTTOM && _control_dependency == DependsOnlyOnTest;
+  }
 };
 
 //------------------------------LoadBNode--------------------------------------

hotspot/src/share/vm/opto/mulnode.cpp

@@ -483,11 +483,7 @@ Node *AndINode::Ideal(PhaseGVN *phase, bool can_reshape) {
   if (can_reshape &&
       load->outcnt() == 1 && load->unique_out() == this) {
     if (lop == Op_LoadS && (mask & 0xFFFF0000) == 0 ) {
-      Node *ldus = new LoadUSNode(load->in(MemNode::Control),
-                                  load->in(MemNode::Memory),
-                                  load->in(MemNode::Address),
-                                  load->adr_type(),
-                                  TypeInt::CHAR, MemNode::unordered);
+      Node* ldus = load->as_Load()->convert_to_unsigned_load(*phase);
       ldus = phase->transform(ldus);
       return new AndINode(ldus, phase->intcon(mask & 0xFFFF));
     }
@@ -495,11 +491,7 @@ Node *AndINode::Ideal(PhaseGVN *phase, bool can_reshape) {
     // Masking sign bits off of a Byte?  Do an unsigned byte load plus
     // an and.
     if (lop == Op_LoadB && (mask & 0xFFFFFF00) == 0) {
-      Node* ldub = new LoadUBNode(load->in(MemNode::Control),
-                                  load->in(MemNode::Memory),
-                                  load->in(MemNode::Address),
-                                  load->adr_type(),
-                                  TypeInt::UBYTE, MemNode::unordered);
+      Node* ldub = load->as_Load()->convert_to_unsigned_load(*phase);
       ldub = phase->transform(ldub);
       return new AndINode(ldub, phase->intcon(mask));
     }
@@ -934,11 +926,7 @@ Node *RShiftINode::Ideal(PhaseGVN *phase, bool can_reshape) {
              ld->Opcode() == Op_LoadUS &&
              ld->outcnt() == 1 && ld->unique_out() == shl)
       // Replace zero-extension-load with sign-extension-load
-      return new LoadSNode( ld->in(MemNode::Control),
-                            ld->in(MemNode::Memory),
-                            ld->in(MemNode::Address),
-                            ld->adr_type(), TypeInt::SHORT,
-                            MemNode::unordered);
+      return ld->as_Load()->convert_to_signed_load(*phase);
   }
 
   // Check for "(byte[i] <<24)>>24" which simply sign-extends