Reviewers: ulan, jochen,
Message:
There remains a similar tag/untag sequence caused by the forced
representation
change for BuildUncheckedMonomorphicElementAccess(). However, I can't find a
good way to remove this, as the representation is unknown at the point that
the
forced change is introduced.
Description:
Remove forced type changes when they can't deopt
Hydrogen attempts to force representation changes on certain operations in
order
to deoptimise on the change rather than the operation. However, these forced
changes are often unnecessary on 64-bit platforms, and cause poor code
generation, so this patch makes some of them conditional on whether it's
possible for deoptimisation to occur in the change.
On ARM64, this prevents sequences like:
;;; <@46,#89> smi-tag
0x7ff282c7f050 144 lsl x4, x4, #32
;;; <@48,#90> smi-untag
0x7ff282c7f054 148 asr x5, x4, #32
;;; <@50,#31> mul-const-i-s
0x7ff282c7f058 152 lsl w6, w5, #3
BUG=
Please review this at https://codereview.chromium.org/303263010/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+34, -9 lines):
M src/hydrogen.cc
Index: src/hydrogen.cc
diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index
c836dc21653129de5d7255a3c8d8b2e9c63e71c0..779b22470280bf9dba59575bd29d5fd63acff38a
100644
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -2121,6 +2121,18 @@ HValue* HGraphBuilder::BuildStringAdd(
}
+static bool RepresentationChangeToSmiCanDeopt(HValue* val) {
+ // A representation change to smi can deopt if the value:
+ // - is not an int32 or smi
+ // - is an int32, and smis are 31 bits
+ // - is a uint32.
+ return !val->representation().IsSmiOrInteger32() ||
+ (val->representation().IsInteger32() &&
+ ((kSmiValueSize < (sizeof(int32_t) * 8)) || //
NOLINT(runtime/sizeof)
+ val->CheckFlag(HValue::kUint32)));
+}
+
+
HInstruction* HGraphBuilder::BuildUncheckedMonomorphicElementAccess(
HValue* checked_object,
HValue* key,
@@ -2323,7 +2335,10 @@ HValue*
HGraphBuilder::BuildAllocateElementsAndInitializeElementsHeader(
HValue* capacity) {
// The HForceRepresentation is to prevent possible deopt on int-smi
// conversion after allocation but before the new object fields are set.
- capacity = AddUncasted<HForceRepresentation>(capacity,
Representation::Smi());
+ if (RepresentationChangeToSmiCanDeopt(capacity)) {
+ capacity = AddUncasted<HForceRepresentation>(capacity,
+ Representation::Smi());
+ }
HValue* new_elements = BuildAllocateElements(kind, capacity);
BuildInitializeElementsHeader(new_elements, kind, capacity);
return new_elements;
@@ -3001,12 +3016,17 @@ HValue*
HGraphBuilder::JSArrayBuilder::AllocateArray(HValue* size_in_bytes,
// These HForceRepresentations are because we store these as fields in
the
// objects we construct, and an int32-to-smi HChange could deopt. Accept
// the deopt possibility now, before allocation occurs.
- capacity =
- builder()->AddUncasted<HForceRepresentation>(capacity,
- Representation::Smi());
- length_field =
- builder()->AddUncasted<HForceRepresentation>(length_field,
- Representation::Smi());
+ if (RepresentationChangeToSmiCanDeopt(capacity)) {
+ capacity =
+ builder()->AddUncasted<HForceRepresentation>(capacity,
+
Representation::Smi());
+ }
+ if (RepresentationChangeToSmiCanDeopt(length_field)) {
+ length_field =
+ builder()->AddUncasted<HForceRepresentation>(length_field,
+
Representation::Smi());
+ }
+
// Allocate (dealing with failure appropriately)
HAllocate* new_object = builder()->Add<HAllocate>(size_in_bytes,
HType::JSArray(), NOT_TENURED, JS_ARRAY_TYPE);
@@ -8869,7 +8889,9 @@ HValue*
HOptimizedGraphBuilder::BuildAllocateExternalElements(
// The HForceRepresentation is to prevent possible deopt on int-smi
// conversion after allocation but before the new object fields are set.
- length = AddUncasted<HForceRepresentation>(length,
Representation::Smi());
+ if (RepresentationChangeToSmiCanDeopt(length)) {
+ length = AddUncasted<HForceRepresentation>(length,
Representation::Smi());
+ }
HValue* elements =
Add<HAllocate>(
Add<HConstant>(ExternalArray::kAlignedSize),
@@ -8926,7 +8948,10 @@ HValue*
HOptimizedGraphBuilder::BuildAllocateFixedTypedArray(
// The HForceRepresentation is to prevent possible deopt on int-smi
// conversion after allocation but before the new object fields are set.
- length = AddUncasted<HForceRepresentation>(length,
Representation::Smi());
+ if (RepresentationChangeToSmiCanDeopt(length)) {
+ length = AddUncasted<HForceRepresentation>(length,
Representation::Smi());
+ }
+
Handle<Map> fixed_typed_array_map(
isolate()->heap()->MapForFixedTypedArray(array_type));
HValue* elements =
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
