8152773: C2: LoadNode properties aren't preserved when converting between signed/unsigned variants
Reviewed-by: jrose, kvn
--- a/hotspot/src/share/vm/opto/memnode.cpp Mon Mar 28 13:49:34 2016 +0300
+++ b/hotspot/src/share/vm/opto/memnode.cpp Mon Mar 28 13:49:34 2016 +0300
@@ -742,7 +742,7 @@
// 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 @@
}
}
// 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 @@
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.
--- a/hotspot/src/share/vm/opto/memnode.hpp Mon Mar 28 13:49:34 2016 +0300
+++ b/hotspot/src/share/vm/opto/memnode.hpp Mon Mar 28 13:49:34 2016 +0300
@@ -148,9 +148,8 @@
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 @@
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 @@
// 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 @@
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 @@
// 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 @@
// 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--------------------------------------
--- a/hotspot/src/share/vm/opto/mulnode.cpp Mon Mar 28 13:49:34 2016 +0300
+++ b/hotspot/src/share/vm/opto/mulnode.cpp Mon Mar 28 13:49:34 2016 +0300
@@ -483,11 +483,7 @@
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 @@
// 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 @@
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