From 26911a6176ed84eb4e4cc4b4e3fdcceb130c592b Mon Sep 17 00:00:00 2001 From: Tess Strickland Date: Tue, 21 Nov 2023 11:40:01 +0000 Subject: [PATCH] [vm/compiler] On 32-bit archs, no specific inline for index checks. Replacing index checks with a single GenericCheckBound instruction currently causes performance regressions on 32-bit ARM. Undo that optimization there for now, instead inlining the original Dart code into the indexing operations. TEST=ci Issue: https://github.com/flutter/flutter/issues/138689 Change-Id: I7e67966b576a9d1b5f510e17b44f853f23c98a91 Cq-Include-Trybots: luci.dart.try:vm-linux-release-simarm-try,vm-ffi-qemu-linux-release-arm-try,vm-aot-linux-release-simarm_x64-try Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/337480 Reviewed-by: Daco Harkes Commit-Queue: Tess Strickland --- .../dart/typed_list_index_checkbound_il_test.dart | 15 ++++++++++++--- runtime/vm/compiler/backend/inliner.cc | 8 ++++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/runtime/tests/vm/dart/typed_list_index_checkbound_il_test.dart b/runtime/tests/vm/dart/typed_list_index_checkbound_il_test.dart index 59f8df2f8b6..794dabf0a46 100644 --- a/runtime/tests/vm/dart/typed_list_index_checkbound_il_test.dart +++ b/runtime/tests/vm/dart/typed_list_index_checkbound_il_test.dart @@ -24,7 +24,10 @@ int retrieveFromBase(Int8List src, int n) => src[n]; int retrieveFromExternal(Int8List src, int n) => src[n]; void matchIL$retrieveFromView(FlowGraph graph) { - graph.dump(); + // TODO(https://github.com/flutter/flutter/issues/138689): Once the regression + // for doing the replacement with the GenericCheckBound instruction is fixed + // on 32-bit archs, remove this. + if (is32BitConfiguration) return; graph.match([ match.block('Graph'), match.block('Function', [ @@ -57,7 +60,10 @@ void matchIL$retrieveFromView(FlowGraph graph) { } void matchIL$retrieveFromBase(FlowGraph graph) { - graph.dump(); + // TODO(https://github.com/flutter/flutter/issues/138689): Once the regression + // for doing the replacement with the GenericCheckBound instruction is fixed + // on 32-bit archs, remove this. + if (is32BitConfiguration) return; graph.match([ match.block('Graph'), match.block('Function', [ @@ -80,7 +86,10 @@ void matchIL$retrieveFromBase(FlowGraph graph) { } void matchIL$retrieveFromExternal(FlowGraph graph) { - graph.dump(); + // TODO(https://github.com/flutter/flutter/issues/138689): Once the regression + // for doing the replacement with the GenericCheckBound instruction is fixed + // on 32-bit archs, remove this. + if (is32BitConfiguration) return; graph.match([ match.block('Graph'), match.block('Function', [ diff --git a/runtime/vm/compiler/backend/inliner.cc b/runtime/vm/compiler/backend/inliner.cc index 7d6c669ebd9..b56922d49c7 100644 --- a/runtime/vm/compiler/backend/inliner.cc +++ b/runtime/vm/compiler/backend/inliner.cc @@ -2753,6 +2753,13 @@ static bool InlineTypedDataIndexCheck(FlowGraph* flow_graph, Instruction** last, Definition** result, const String& symbol) { +#if defined(TARGET_ARCH_IS_32_BIT) + // TODO(https://github.com/flutter/flutter/issues/138689): We only convert + // the index check to a GenericCheckBound instruction on 64-bit architectures, + // where the inputs are always unboxed. Once the regressions on 32-bit + // architectures has been identified and fixed, remove the #ifdef. + return false; +#else *entry = new (Z) FunctionEntryInstr(graph_entry, flow_graph->allocate_block_id(), call->GetBlock()->try_index(), DeoptId::kNone); @@ -2779,6 +2786,7 @@ static bool InlineTypedDataIndexCheck(FlowGraph* flow_graph, *last = cursor; *result = index; return true; +#endif } static intptr_t PrepareInlineIndexedOp(FlowGraph* flow_graph,