From b3cb6babfbd4bf70b9df692d9cd0013733e2f632 Mon Sep 17 00:00:00 2001 From: Mikhail Kaskov Date: Thu, 27 Jul 2023 20:59:56 +0300 Subject: [PATCH] Added Peephole for pair MultiArray and LenArray Signed-off-by: Mikhail Kaskov --- compiler/optimizer/ir/ir_constructor.h | 2 +- .../optimizer/optimizations/peepholes.cpp | 62 +++++++ compiler/optimizer/optimizations/peepholes.h | 1 + compiler/tests/peepholes_test.cpp | 165 ++++++++++++++++++ plugins/ets/tests/checked/CMakeLists.txt | 1 + plugins/ets/tests/checked/multiarray.ets | 64 +++++++ 6 files changed, 294 insertions(+), 1 deletion(-) create mode 100644 plugins/ets/tests/checked/multiarray.ets diff --git a/compiler/optimizer/ir/ir_constructor.h b/compiler/optimizer/ir/ir_constructor.h index 1b6ea40f6..bc0b0b4c6 100644 --- a/compiler/optimizer/ir/ir_constructor.h +++ b/compiler/optimizer/ir/ir_constructor.h @@ -261,7 +261,7 @@ public: template IrConstructor &InputsAutoType(Args... inputs) { - ASSERT(CurrentInst()->IsCall()); + ASSERT(CurrentInst()->IsCall() || CurrentInst()->GetOpcode() == Opcode::MultiArray); auto *call_inst = static_cast(CurrentInst()); call_inst->AllocateInputTypes(graph_->GetAllocator(), sizeof...(inputs)); ((call_inst->AddInputType(GetInst(inputs).GetType())), ...); diff --git a/compiler/optimizer/optimizations/peepholes.cpp b/compiler/optimizer/optimizations/peepholes.cpp index 27efa99bf..5a48c4bf9 100644 --- a/compiler/optimizer/optimizations/peepholes.cpp +++ b/compiler/optimizer/optimizations/peepholes.cpp @@ -1232,6 +1232,9 @@ void Peepholes::VisitLenArray(GraphVisitor *v, Inst *inst) inst->ReplaceUsers(array_size); PEEPHOLE_IS_APPLIED(static_cast(v), inst); } + if (static_cast(v)->OptimizeLenArrayForMultiArray(inst, input, 0)) { + PEEPHOLE_IS_APPLIED(static_cast(v), inst); + } } void Peepholes::VisitPhi([[maybe_unused]] GraphVisitor *v, Inst *inst) @@ -2635,4 +2638,63 @@ void Peepholes::VisitLoadFromConstantPool(GraphVisitor *v, Inst *inst) PEEPHOLE_IS_APPLIED(static_cast(v), inst); } +// Move users from LenArray to constant which used in MultiArray +// Case 1 +// 1.s64 ... ->{v3} +// 2.s64 ... ->{v3} +// 3.ref MultiArray ... , v1, v2, ... ->{v4,..} +// 4.s32 LenArray v3 ->{v5, ...} +// 5. USE v5 +// ===> +// 1.s64 ... ->{v3, v5, ...} +// 2.s64 ... ->{v3} +// 3.ref MultiArray ... , v1, v2, ... ->{v4,..} +// 4.s32 LenArray v3 +// 5. USE v1 + +// Case 2 +// 1.s64 ... ->{v3} +// 2.s64 ... ->{v3} +// 3.ref MultiArray ... , v1, v2, ... ->{v4,..} +// 4.ref LoadArray v3, ... +// 5.ref NullCheck v4, ... +// 6.s32 LenArray v5 ->{v7, ...} +// 7. USE v6 +// ===> +// 1.s64 ... ->{v3} +// 2.s64 ... ->{v3, v7, ...} +// 3.ref MultiArray ... , v1, v2, ... ->{v4,..} +// 4.ref LoadArray v3, ... +// 5.ref NullCheck v4, ... +// 6.s32 LenArray v5 +// 7. USE v2 +bool Peepholes::OptimizeLenArrayForMultiArray(Inst *len_array, Inst *inst, size_t index_size) +{ + if (inst->GetOpcode() == Opcode::MultiArray) { + // Arguments of MultiArray look like : class, size_0, size_1, ..., size_N, SaveState + // When element type of multyarray is array-like object (LenArray can be applyed to it), we can get case when + // number sequential LoadArray with LenArray more than dimension of MultiArrays. So limiting the index_size. + // Example in unittest PeepholesTest.MultiArrayWithLenArrayOfString + + auto multiarr_dimension = inst->GetInputsCount() - 2; + if (!(index_size < multiarr_dimension)) { + return false; + } + // MultiArray's sizes starts from index "1", so need add "1" to get absolute index + auto value = inst->GetDataFlowInput(index_size + 1); + for (auto &it : len_array->GetUsers()) { + if (SkipThisPeepholeInOSR(it.GetInst(), value)) { + return false; + } + } + len_array->ReplaceUsers(value); + return true; + } + if (inst->GetOpcode() == Opcode::LoadArray) { + auto input = inst->GetDataFlowInput(0); + return OptimizeLenArrayForMultiArray(len_array, input, index_size + 1); + } + return false; +} + } // namespace panda::compiler diff --git a/compiler/optimizer/optimizations/peepholes.h b/compiler/optimizer/optimizations/peepholes.h index 89818f345..f71ab1302 100644 --- a/compiler/optimizer/optimizations/peepholes.h +++ b/compiler/optimizer/optimizations/peepholes.h @@ -163,6 +163,7 @@ private: static bool GetInputsOfCompareWithConst(const Inst *inst, Inst **input, ConstantInst **const_input, bool *inputs_swapped); + bool OptimizeLenArrayForMultiArray(Inst *len_array, Inst *inst, size_t index_size); static bool TryEliminatePhi(PhiInst *phi); // It is check can we use this peephole in OSR or not static bool SkipThisPeepholeInOSR(Inst *inst, Inst *new_input); diff --git a/compiler/tests/peepholes_test.cpp b/compiler/tests/peepholes_test.cpp index 8b5731fb4..3d4d9a845 100644 --- a/compiler/tests/peepholes_test.cpp +++ b/compiler/tests/peepholes_test.cpp @@ -5936,6 +5936,171 @@ TEST_F(PeepholesTest, OverflowCheckAsBitwiseAndSSInput) ASSERT_TRUE(GraphComparator().Compare(GetGraph(), clone)); } +TEST_F(PeepholesTest, MultiArrayWithLenArrayFirstLayer) +{ + auto class1 = reinterpret_cast(1); + GRAPH(GetGraph()) + { + CONSTANT(0, 0x3); + CONSTANT(1, 0x2); + CONSTANT(7, 0x1); + + BASIC_BLOCK(2, -1) + { + INST(2, Opcode::SaveState).Inputs(0, 1).SrcVregs({0, 1}); + INST(3, Opcode::LoadImmediate).ref().Class(class1); + INST(4, Opcode::MultiArray) + .ref() + .InputsAutoType(3, 0, 1, 2); // Will be create [ [ [][] ], [ [][] ], [ [][] ] ] + INST(5, Opcode::LenArray).Inputs(4).s32(); + INST(10, Opcode::Return).s32().Inputs(5); + } + } + + auto graph = CreateEmptyGraph(); + GRAPH(graph) + { + CONSTANT(0, 0x3); + CONSTANT(1, 0x2); + CONSTANT(7, 0x1); + + BASIC_BLOCK(2, -1) + { + INST(2, Opcode::SaveState).Inputs(0, 1).SrcVregs({0, 1}); + INST(3, Opcode::LoadImmediate).ref().Class(class1); + INST(4, Opcode::MultiArray) + .ref() + .InputsAutoType(3, 0, 1, 2); // Will be create [ [ [][] ], [ [][] ], [ [][] ] ] + INST(5, Opcode::LenArray).Inputs(4).s32(); + INST(10, Opcode::Return).s32().Inputs(0); + } + } + ASSERT_TRUE(GetGraph()->RunPass()); + GraphChecker(GetGraph()).Check(); + ASSERT_TRUE(GraphComparator().Compare(GetGraph(), graph)); +} + +TEST_F(PeepholesTest, MultiArrayWithLenArraySecondLayer) +{ + auto class1 = reinterpret_cast(1); + GRAPH(GetGraph()) + { + PARAMETER(0, 0x3).s32(); + CONSTANT(1, 0x2); + CONSTANT(7, 0x1); + + BASIC_BLOCK(2, -1) + { + INST(2, Opcode::SaveState).Inputs(0, 1).SrcVregs({0, 1}); + INST(3, Opcode::LoadImmediate).ref().Class(class1); + INST(4, Opcode::MultiArray) + .ref() + .InputsAutoType(3, 0, 1, 2); // Will be create [ [ [][] ], [ [][] ], [ [][] ] ] + INST(5, Opcode::LenArray).Inputs(4).s32(); + INST(11, Opcode::LoadArray).ref().Inputs(4, 7); + INST(12, Opcode::NullCheck).ref().Inputs(11, 2); + INST(13, Opcode::LenArray).Inputs(12).s32(); + INST(14, Opcode::Add).Inputs(5, 13).s32(); + INST(10, Opcode::Return).s32().Inputs(14); + } + } + + auto graph = CreateEmptyGraph(); + GRAPH(graph) + { + PARAMETER(0, 0x3).s32(); + CONSTANT(1, 0x2); + CONSTANT(7, 0x1); + + BASIC_BLOCK(2, -1) + { + INST(2, Opcode::SaveState).Inputs(0, 1).SrcVregs({0, 1}); + INST(3, Opcode::LoadImmediate).ref().Class(class1); + INST(4, Opcode::MultiArray) + .ref() + .InputsAutoType(3, 0, 1, 2); // Will be create [ [ [][] ], [ [][] ], [ [][] ] ] + INST(5, Opcode::LenArray).Inputs(4).s32(); + INST(11, Opcode::LoadArray).ref().Inputs(4, 7); + INST(12, Opcode::NullCheck).ref().Inputs(11, 2); + INST(13, Opcode::LenArray).Inputs(12).s32(); + INST(14, Opcode::Add).Inputs(0, 1).s32(); + INST(10, Opcode::Return).s32().Inputs(14); + } + } + ASSERT_TRUE(GetGraph()->RunPass()); + GraphChecker(GetGraph()).Check(); + ASSERT_TRUE(GraphComparator().Compare(GetGraph(), graph)); +} + +TEST_F(PeepholesTest, MultiArrayWithLenArrayOfString) +{ + auto class1 = reinterpret_cast(1); + GRAPH(GetGraph()) + { + PARAMETER(0, 0x1).s32(); + CONSTANT(1, 0x3); + PARAMETER(2, 0x2).s32(); // Will think, that value is 0x2 + + BASIC_BLOCK(2, -1) + { + INST(3, Opcode::SaveState); + INST(4, Opcode::LoadImmediate).ref().Class(class1); + INST(5, Opcode::MultiArray) + .ref() + .InputsAutoType(4, 1, 2, + 3); // Will be create [ [ String, String ], [ String, String ], [ String, String ] ] + INST(6, Opcode::LenArray).Inputs(5).s32(); + + INST(7, Opcode::LoadArray).ref().Inputs(5, 0); // Loaded: [ String, String ] + INST(8, Opcode::NullCheck).ref().Inputs(7, 3); + INST(9, Opcode::LenArray).Inputs(8).s32(); + + INST(10, Opcode::LoadArray).ref().Inputs(8, 0); // Loaded: String + INST(11, Opcode::NullCheck).ref().Inputs(10, 3); + INST(12, Opcode::LenArray).Inputs(11).s32(); // Length of String + + INST(13, Opcode::Add).Inputs(9, 6).s32(); + INST(14, Opcode::Add).Inputs(13, 12).s32(); + INST(15, Opcode::Return).s32().Inputs(14); + } + } + + auto graph = CreateEmptyGraph(); + GRAPH(graph) + { + PARAMETER(0, 0x1).s32(); + CONSTANT(1, 0x3); + PARAMETER(2, 0x2).s32(); // Will think, that value is 0x2 + + BASIC_BLOCK(2, -1) + { + INST(3, Opcode::SaveState); + INST(4, Opcode::LoadImmediate).ref().Class(class1); + INST(5, Opcode::MultiArray) + .ref() + .InputsAutoType(4, 1, 2, + 3); // Will be create [ [ String, String ], [ String, String ], [ String, String ] ] + INST(6, Opcode::LenArray).Inputs(5).s32(); + + INST(7, Opcode::LoadArray).ref().Inputs(5, 0); // Loaded: [ String, String ] + INST(8, Opcode::NullCheck).ref().Inputs(7, 3); + INST(9, Opcode::LenArray).Inputs(8).s32(); + + INST(10, Opcode::LoadArray).ref().Inputs(8, 0); // Loaded: String + INST(11, Opcode::NullCheck).ref().Inputs(10, 3); + INST(12, Opcode::LenArray).Inputs(11).s32(); // Length of String + + INST(13, Opcode::Add).Inputs(2, 1).s32(); + INST(14, Opcode::Add).Inputs(13, 12).s32(); + INST(15, Opcode::Return).s32().Inputs(14); + } + } + + ASSERT_TRUE(GetGraph()->RunPass()); + GraphChecker(GetGraph()).Check(); + ASSERT_TRUE(GraphComparator().Compare(GetGraph(), graph)); +} + // NOLINTEND(readability-magic-numbers) } // namespace panda::compiler diff --git a/plugins/ets/tests/checked/CMakeLists.txt b/plugins/ets/tests/checked/CMakeLists.txt index cd3cd78f8..763e7e448 100644 --- a/plugins/ets/tests/checked/CMakeLists.txt +++ b/plugins/ets/tests/checked/CMakeLists.txt @@ -142,4 +142,5 @@ if (PANDA_TARGET_AMD64 OR NOT PANDA_ARM64_TESTS_WITH_SANITIZER) panda_add_checked_test_ets(FILE ${CMAKE_CURRENT_SOURCE_DIR}/hoist_loop_inv_bounds_check.ets) panda_add_checked_test_ets(FILE ${CMAKE_CURRENT_SOURCE_DIR}/ets_string_length.ets) panda_add_checked_test_ets(FILE ${CMAKE_CURRENT_SOURCE_DIR}/ets_string_isempty.ets) + panda_add_checked_test_ets(FILE ${CMAKE_CURRENT_SOURCE_DIR}/multiarray.ets) endif() diff --git a/plugins/ets/tests/checked/multiarray.ets b/plugins/ets/tests/checked/multiarray.ets new file mode 100644 index 000000000..404d84052 --- /dev/null +++ b/plugins/ets/tests/checked/multiarray.ets @@ -0,0 +1,64 @@ +/* + * Copyright (c) 2023 Huawei Device Co., Ltd. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +//! CHECKER Test check optimize LenArray and MultiArray two layers +//! RUN force_jit: true, options: "", entry: "ETSGLOBAL::main1" +//! EVENT /Compilation,ETSGLOBAL::main1,.*,COMPILED/ +//! METHOD "ETSGLOBAL::main1" +//! PASS_BEFORE "Peepholes" +//! INST_COUNT "LenArray", 2 +//! INST_COUNT "BoundsCheck", 2 +//! PASS_AFTER_NEXT "Cleanup" +//! INST_COUNT "LenArray", 0 +//! PASS_AFTER "Codegen" +//! INST_COUNT "BoundsCheck", 0 +//! INST_COUNT "BoundsCheckI", 0 + +//! CHECKER Test check optimize LenArray and MultiArray checking work BoundCheck +//! SKIP_IF @architecture == "arm32" +//! RUN_PAOC options: "--compiler-regex='.*main2.*'" +//! EVENT /Compilation,ETSGLOBAL::main2,.*,COMPILED/ +//! METHOD "ETSGLOBAL::main2" +//! PASS_BEFORE "Peepholes" +//! INST_COUNT "LenArray", 3 +//! PASS_AFTER_NEXT "Cleanup" +//! INST_COUNT "LenArray", 0 +//! RUN options: "--compiler-enable-jit=false", entry: "ETSGLOBAL::main2", result: 1 +//! EVENT /DeoptimizationReason,ETSGLOBAL::main2,BOUNDS_CHECK/ + +//! CHECKER Test check don't optimize LenArray of String in MultiArray +//! RUN force_jit: true, options: "--compiler-regex='.*main3.*'", entry: "ETSGLOBAL::main3" +//! EVENT /Compilation,ETSGLOBAL::main3,.*,COMPILED/ +//! METHOD "ETSGLOBAL::main3" +//! PASS_AFTER "Codegen" +//! INST_COUNT "LenArray", 1 + +function main1(): int { + let M1: int[][] = new int[4][4]; + M1[0][0] = 1; + return 0; +} + +function main2(): int { + let M1: int[][][] = new int[2][2][2]; + M1[0][0][3] = 1; + return 0; +} + +function main3(): int { + let arr = new String[2][3]; + arr[0][0] = new String(); + return arr[0][0].length(); // Should be "0" +} -- Gitee