Revert "[VM] Inline ClassID.getID() eagerly, extend pattern matching logic to recognize it, use it to special case ascii decoding"
This reverts commit d7cf6959bb823879390b56d270f8592a8ddf32b6.
Reason for revert: This causes crashes on the vm-obfuscate builder at https://ci.chromium.org/p/dart/builders/luci.dart.ci.sandbox/vm-kernel-precomp-obfuscate-linux-release-x64
like
FAILED: dartkp-dart_precompiled release_x64 corelib_2/linked_hash_map_test
Expected: Pass
Actual: Crash
Unexpected compile error.
--- Command "vm_compile_to_kernel" (took 11.000189s):
DART_CONFIGURATION=ReleaseX64 /b/s/w/ir/cache/builder/sdk/pkg/vm/tool/gen_kernel --aot --platform=out/ReleaseX64/vm_platform_strong.dill -o /b/s/w/ir/cache/builder/sdk/out/ReleaseX64/generated_compilations/dartkp/tests_corelib_2_linked_hash_map_test/out.dill /b/s/w/ir/cache/builder/sdk/tests/corelib_2/linked_hash_map_test.dart --packages=/b/s/w/ir/cache/builder/sdk/.packages -Ddart.developer.causal_async_stacks=true
exit code:
0
--- Command "precompiler" (took 583ms):
DART_CONFIGURATION=ReleaseX64 out/ReleaseX64/gen_snapshot --snapshot-kind=app-aot-assembly --assembly=/b/s/w/ir/cache/builder/sdk/out/ReleaseX64/generated_compilations/dartkp/tests_corelib_2_linked_hash_map_test/out.S --obfuscate --sync-async --ignore-unrecognized-flags --packages=/b/s/w/ir/cache/builder/sdk/.packages /b/s/w/ir/cache/builder/sdk/out/ReleaseX64/generated_compilations/dartkp/tests_corelib_2_linked_hash_map_test/out.dill
exit code:
-6
stderr:
Warning: This VM has been configured to obfuscate symbol information which violates the Dart standard.
See dartbug.com/30524 for more information.
===== CRASH =====
version=2.2.0-edge.83712405657ff736033380cb24b0bd5a62fc3692 (Tue Nov 27 16:56:07 2018 +0000) on "linux_x64"
si_signo=Segmentation fault(11), si_code=1, si_addr=0xd1400050e5f
Dumping native stack trace for thread 2b71
[0x000056001fc426c7] Unknown symbol
[0x000056001fc426c7] Unknown symbol
[0x000056001fe5763d] Unknown symbol
[0x000056001fe4c99c] Unknown symbol
[0x000056001fe6166a] Unknown symbol
[0x000056001fe59c0c] Unknown symbol
Original change's description:
> [VM] Inline ClassID.getID() eagerly, extend pattern matching logic to recognize it, use it to special case ascii decoding
>
> This change extends/fixes the exiting "pattern recognition" which tries
> to recognize the pattern
>
> v2 <- LoadClassIdInstr(v1)
> BranchIf v2 == IntegerConstant(cid)
>
> Furthermore we start inlining the recognized `ClassID.getID` method very
> early in the pipeline. This allows the VM to recognize the above
> pattern and insert redefinitions before the actual inlining pass.
>
> Furthermore we special-case two very hot methods in utf8 decoding by
> manually having two loops, one of which is guarded by a class-id check
> against the _Uint8ArrayView class, which is most common. (In the future
> we would like to unify the typed data layouts so we no longer need to
> use `ClassId.getID`, thereby also allowing non core library code to use
> this).
>
> This improves dart-aot by
> * 31%+ for a protobuf decoding benchmark we care about
>
>
> Issue https://github.com/dart-lang/sdk/issues/31954
>
> Change-Id: Ia567b92b7e76ff28eda1726deaafda32732ed8f5
> Reviewed-on: https://dart-review.googlesource.com/c/85281
> Reviewed-by: Johnni Winther <johnniwinther@google.com>
> Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
> Reviewed-by: Jenny Messerly <jmesserly@google.com>
> Commit-Queue: Martin Kustermann <kustermann@google.com>
TBR=vegorov@google.com,kustermann@google.com,jmesserly@google.com,johnniwinther@google.com,sra@google.com
Change-Id: I912b0768c32cbb00297ce48db29dbdbea44c14fa
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/85441
Reviewed-by: William Hesse <whesse@google.com>
Commit-Queue: William Hesse <whesse@google.com>
diff --git a/pkg/dev_compiler/tool/input_sdk/patch/convert_patch.dart b/pkg/dev_compiler/tool/input_sdk/patch/convert_patch.dart
index 787c677..cfaa3fc 100644
--- a/pkg/dev_compiler/tool/input_sdk/patch/convert_patch.dart
+++ b/pkg/dev_compiler/tool/input_sdk/patch/convert_patch.dart
@@ -495,13 +495,3 @@
return null;
}();
}
-
-@patch
-int _scanOneByteCharacters(List<int> units, int from, int endIndex) {
- final to = endIndex;
- for (var i = from; i < to; i++) {
- final unit = units[i];
- if ((unit & _ONE_BYTE_LIMIT) != unit) return i - from;
- }
- return to - from;
-}
diff --git a/runtime/bin/BUILD.gn b/runtime/bin/BUILD.gn
index a5c2317..d3bc4df 100644
--- a/runtime/bin/BUILD.gn
+++ b/runtime/bin/BUILD.gn
@@ -342,7 +342,9 @@
"..:dart_config",
"..:dart_os_config",
] + extra_configs
- public_configs = [ "..:dart_public_config" ]
+ public_configs = [
+ "..:dart_public_config",
+ ]
if (is_fuchsia) {
configs -= [ "//build/config:symbol_visibility_hidden" ]
}
diff --git a/runtime/bin/process_fuchsia.cc b/runtime/bin/process_fuchsia.cc
index 1608bc9..f24027e2 100644
--- a/runtime/bin/process_fuchsia.cc
+++ b/runtime/bin/process_fuchsia.cc
@@ -615,9 +615,9 @@
char err_msg[FDIO_SPAWN_ERR_MSG_MAX_LENGTH];
uint32_t flags = FDIO_SPAWN_CLONE_JOB | FDIO_SPAWN_CLONE_LDSVC |
FDIO_SPAWN_CLONE_NAMESPACE;
- status =
- fdio_spawn_vmo(ZX_HANDLE_INVALID, flags, vmo, program_arguments_,
- program_environment_, 4, actions, &process, err_msg);
+ status = fdio_spawn_vmo(ZX_HANDLE_INVALID, flags, vmo, program_arguments_,
+ program_environment_, 4, actions, &process,
+ err_msg);
if (status != ZX_OK) {
LOG_ERR("ProcessStarter: Start() fdio_spawn_vmo failed\n");
diff --git a/runtime/lib/class_id_fasta.dart b/runtime/lib/class_id_fasta.dart
index c2f4ae0..0a89db4 100644
--- a/runtime/lib/class_id_fasta.dart
+++ b/runtime/lib/class_id_fasta.dart
@@ -14,5 +14,4 @@
static final int cidImmutableArray = 0;
static final int cidOneByteString = 0;
static final int cidTwoByteString = 0;
- static final int cidUint8ArrayView = 0;
}
diff --git a/runtime/lib/convert_patch.dart b/runtime/lib/convert_patch.dart
index 3d99c0d..0802cbb 100644
--- a/runtime/lib/convert_patch.dart
+++ b/runtime/lib/convert_patch.dart
@@ -7,7 +7,7 @@
/// used by patches of that library. We plan to change this when we have a
/// shared front end and simply use parts.
-import "dart:_internal" show POWERS_OF_TEN, patch, ClassID;
+import "dart:_internal" show POWERS_OF_TEN, patch;
import "dart:typed_data" show Uint8List, Uint16List;
@@ -1816,27 +1816,3 @@
_sink.close();
}
}
-
-@patch
-int _scanOneByteCharacters(List<int> units, int from, int endIndex) {
- final to = endIndex;
-
- // Special case for _Uint8ArrayView.
- final cid = ClassID.getID(units);
- if (identical(cid, ClassID.cidUint8ArrayView)) {
- if (from >= 0 && to >= 0 && to <= units.length) {
- for (int i = from; i < to; i++) {
- final unit = units[i];
- if ((unit & _ONE_BYTE_LIMIT) != unit) return i - from;
- }
- return to - from;
- }
- }
-
- // Fall through to normal case.
- for (var i = from; i < to; i++) {
- final unit = units[i];
- if ((unit & _ONE_BYTE_LIMIT) != unit) return i - from;
- }
- return to - from;
-}
diff --git a/runtime/lib/string_patch.dart b/runtime/lib/string_patch.dart
index e90667a..103959b 100644
--- a/runtime/lib/string_patch.dart
+++ b/runtime/lib/string_patch.dart
@@ -222,19 +222,6 @@
static String _createOneByteString(List<int> charCodes, int start, int len) {
// It's always faster to do this in Dart than to call into the runtime.
var s = _OneByteString._allocate(len);
-
- // Special case for _Uint8ArrayView.
- final cid = ClassID.getID(charCodes);
- if (identical(cid, ClassID.cidUint8ArrayView)) {
- if (start >= 0 && len >= 0) {
- for (int i = 0; i < len; i++) {
- s._setAt(i, charCodes[start + i]);
- }
- return s;
- }
- }
-
- // Fall through to normal case.
for (int i = 0; i < len; i++) {
s._setAt(i, charCodes[start + i]);
}
diff --git a/runtime/vm/compiler/backend/inliner.cc b/runtime/vm/compiler/backend/inliner.cc
index 61a6a51..bb5778a 100644
--- a/runtime/vm/compiler/backend/inliner.cc
+++ b/runtime/vm/compiler/backend/inliner.cc
@@ -2692,22 +2692,6 @@
return true;
}
-static bool InlineLoadClassId(FlowGraph* flow_graph,
- Instruction* call,
- GraphEntryInstr* graph_entry,
- FunctionEntryInstr** entry,
- Instruction** last) {
- *entry =
- new (Z) FunctionEntryInstr(graph_entry, flow_graph->allocate_block_id(),
- call->GetBlock()->try_index(), DeoptId::kNone);
- (*entry)->InheritDeoptTarget(Z, call);
- auto load_cid = new (Z)
- LoadClassIdInstr(call->PushArgumentAt(0)->value()->CopyWithType(Z));
- flow_graph->InsertBefore(call, load_cid, nullptr, FlowGraph::kValue);
- *last = load_cid;
- return true;
-}
-
// Adds an explicit bounds check for a typed getter/setter.
static void PrepareInlineTypedArrayBoundsCheck(FlowGraph* flow_graph,
Instruction* call,
@@ -3621,8 +3605,6 @@
}
return InlineGetIndexed(flow_graph, kind, call, receiver, graph_entry,
entry, last, can_speculate);
- case MethodRecognizer::kClassIDgetID:
- return InlineLoadClassId(flow_graph, call, graph_entry, entry, last);
default:
break;
}
diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc
index c81689d..80b2c50 100644
--- a/runtime/vm/object.cc
+++ b/runtime/vm/object.cc
@@ -3707,18 +3707,6 @@
CLASS_LIST_WITH_NULL(ADD_SET_FIELD)
#undef ADD_SET_FIELD
-
-#define ADD_SET_FIELD(clazz) \
- field_name = Symbols::New(thread, "cid" #clazz "View"); \
- field = Field::New(field_name, true, false, true, false, *this, \
- Type::Handle(Type::IntType()), TokenPosition::kMinSource, \
- TokenPosition::kMinSource); \
- value = Smi::New(kTypedData##clazz##ViewCid); \
- field.SetStaticValue(value, true); \
- AddField(field);
-
- CLASS_LIST_TYPED_DATA(ADD_SET_FIELD)
-#undef ADD_SET_FIELD
#undef CLASS_LIST_WITH_NULL
}
diff --git a/runtime/vm/symbols.h b/runtime/vm/symbols.h
index 38bfdf5..23e3fde 100644
--- a/runtime/vm/symbols.h
+++ b/runtime/vm/symbols.h
@@ -412,7 +412,6 @@
V(_UserTag, "_UserTag") \
V(Default, "Default") \
V(ClassID, "ClassID") \
- V(getID, "getID") \
V(DartIsVM, "dart.isVM") \
V(stack, ":stack") \
V(stack_pointer, ":stack_pointer") \
diff --git a/sdk/lib/_internal/js_runtime/lib/convert_patch.dart b/sdk/lib/_internal/js_runtime/lib/convert_patch.dart
index afb5062..2d234a4 100644
--- a/sdk/lib/_internal/js_runtime/lib/convert_patch.dart
+++ b/sdk/lib/_internal/js_runtime/lib/convert_patch.dart
@@ -496,13 +496,3 @@
return null;
}
}
-
-@patch
-int _scanOneByteCharacters(List<int> units, int from, int endIndex) {
- final to = endIndex;
- for (var i = from; i < to; i++) {
- final unit = units[i];
- if ((unit & _ONE_BYTE_LIMIT) != unit) return i - from;
- }
- return to - from;
-}
diff --git a/sdk/lib/convert/utf.dart b/sdk/lib/convert/utf.dart
index eb92e2b..567c8b2 100644
--- a/sdk/lib/convert/utf.dart
+++ b/sdk/lib/convert/utf.dart
@@ -412,6 +412,16 @@
_expectedUnits = 0;
_extraUnits = 0;
+ int scanOneByteCharacters(List<int> units, int from) {
+ final to = endIndex;
+ final mask = _ONE_BYTE_LIMIT;
+ for (var i = from; i < to; i++) {
+ final unit = units[i];
+ if ((unit & mask) != unit) return i - from;
+ }
+ return to - from;
+ }
+
void addSingleBytes(int from, int to) {
assert(from >= startIndex && from <= endIndex);
assert(to >= startIndex && to <= endIndex);
@@ -474,7 +484,7 @@
}
while (i < endIndex) {
- var oneBytes = _scanOneByteCharacters(codeUnits, i, endIndex);
+ var oneBytes = scanOneByteCharacters(codeUnits, i);
if (oneBytes > 0) {
_isFirstCharacter = false;
addSingleBytes(i, i + oneBytes);
@@ -535,10 +545,3 @@
}
}
}
-
-// Returns the number of bytes in [units] starting at offset [from] which have
-// the leftmost bit set to 0.
-//
-// To increase performance of this critical method we have a special variant of
-// it implemented in the VM's patch files, which is why we make it external.
-external int _scanOneByteCharacters(List<int> units, int from, int endIndex);
