From f969a209c30e3af636342d2fb7851d82a2529bf7 Mon Sep 17 00:00:00 2001 From: Roland Levillain Date: Wed, 9 Mar 2016 16:14:00 +0000 Subject: [PATCH] Fix and enable java.lang.StringFactory intrinsics. The following intrinsics were not considered by the intrinsics recognizer: - StringNewStringFromBytes - StringNewStringFromChars - StringNewStringFromString This CL enables them and add tests for them. This CL also: - Fixes the locations of the ARM64 & MIPS64 StringNewStringFromString intrinsics. - Fixes the definitions of the FOUR_ARG_DOWNCALL macros on ARM and x86, which are used to implement the art_quick_alloc_string_from_bytes* runtime entry points. - Fixes PC info (stack maps) recording in the StringNewStringFromBytes, StringNewStringFromChars and StringNewStringFromString ARM, ARM64 & MIPS64 intrinsics. Bug: 27425743 Change-Id: I38c00d3f0b2e6b64f7d3fe9146743493bef9e45c --- compiler/dex/quick/dex_file_method_inliner.cc | 7 ++ compiler/optimizing/intrinsics_arm.cc | 9 ++- compiler/optimizing/intrinsics_arm64.cc | 12 ++-- compiler/optimizing/intrinsics_mips64.cc | 13 ++-- compiler/optimizing/intrinsics_x86.cc | 3 + compiler/optimizing/intrinsics_x86_64.cc | 3 + runtime/arch/arm/quick_entrypoints_arm.S | 13 +++- runtime/arch/x86/asm_support_x86.S | 4 ++ runtime/arch/x86/quick_entrypoints_x86.S | 10 +++ .../expected.txt | 3 + .../580-checker-string-factory-intrinsics/info.txt | 1 + .../src/Main.java | 83 ++++++++++++++++++++++ 12 files changed, 144 insertions(+), 17 deletions(-) create mode 100644 test/580-checker-string-factory-intrinsics/expected.txt create mode 100644 test/580-checker-string-factory-intrinsics/info.txt create mode 100644 test/580-checker-string-factory-intrinsics/src/Main.java diff --git a/compiler/dex/quick/dex_file_method_inliner.cc b/compiler/dex/quick/dex_file_method_inliner.cc index 8f5d3ae4b..48c4356cf 100644 --- a/compiler/dex/quick/dex_file_method_inliner.cc +++ b/compiler/dex/quick/dex_file_method_inliner.cc @@ -605,6 +605,13 @@ const DexFileMethodInliner::IntrinsicDef DexFileMethodInliner::kIntrinsicMethods INTRINSIC(JavaLangString, IndexOf, I_I, kIntrinsicIndexOf, kIntrinsicFlagBase0), INTRINSIC(JavaLangString, Length, _I, kIntrinsicIsEmptyOrLength, kIntrinsicFlagLength), + INTRINSIC(JavaLangStringFactory, NewStringFromBytes, ByteArrayIII_String, + kIntrinsicNewStringFromBytes, kIntrinsicFlagNone), + INTRINSIC(JavaLangStringFactory, NewStringFromChars, IICharArray_String, + kIntrinsicNewStringFromChars, kIntrinsicFlagNone), + INTRINSIC(JavaLangStringFactory, NewStringFromString, String_String, + kIntrinsicNewStringFromString, kIntrinsicFlagNone), + INTRINSIC(JavaLangThread, CurrentThread, _Thread, kIntrinsicCurrentThread, 0), INTRINSIC(LibcoreIoMemory, PeekByte, J_B, kIntrinsicPeek, kSignedByte), diff --git a/compiler/optimizing/intrinsics_arm.cc b/compiler/optimizing/intrinsics_arm.cc index b599d42c4..34ed575bd 100644 --- a/compiler/optimizing/intrinsics_arm.cc +++ b/compiler/optimizing/intrinsics_arm.cc @@ -1224,8 +1224,9 @@ void IntrinsicCodeGeneratorARM::VisitStringNewStringFromBytes(HInvoke* invoke) { __ LoadFromOffset( kLoadWord, LR, TR, QUICK_ENTRYPOINT_OFFSET(kArmWordSize, pAllocStringFromBytes).Int32Value()); - codegen_->RecordPcInfo(invoke, invoke->GetDexPc()); + CheckEntrypointTypes(); __ blx(LR); + codegen_->RecordPcInfo(invoke, invoke->GetDexPc()); __ Bind(slow_path->GetExitLabel()); } @@ -1251,8 +1252,9 @@ void IntrinsicCodeGeneratorARM::VisitStringNewStringFromChars(HInvoke* invoke) { // all include a null check on `data` before calling that method. __ LoadFromOffset( kLoadWord, LR, TR, QUICK_ENTRYPOINT_OFFSET(kArmWordSize, pAllocStringFromChars).Int32Value()); - codegen_->RecordPcInfo(invoke, invoke->GetDexPc()); + CheckEntrypointTypes(); __ blx(LR); + codegen_->RecordPcInfo(invoke, invoke->GetDexPc()); } void IntrinsicLocationsBuilderARM::VisitStringNewStringFromString(HInvoke* invoke) { @@ -1276,8 +1278,9 @@ void IntrinsicCodeGeneratorARM::VisitStringNewStringFromString(HInvoke* invoke) __ LoadFromOffset(kLoadWord, LR, TR, QUICK_ENTRYPOINT_OFFSET(kArmWordSize, pAllocStringFromString).Int32Value()); - codegen_->RecordPcInfo(invoke, invoke->GetDexPc()); + CheckEntrypointTypes(); __ blx(LR); + codegen_->RecordPcInfo(invoke, invoke->GetDexPc()); __ Bind(slow_path->GetExitLabel()); } diff --git a/compiler/optimizing/intrinsics_arm64.cc b/compiler/optimizing/intrinsics_arm64.cc index ccbbd4325..b65445766 100644 --- a/compiler/optimizing/intrinsics_arm64.cc +++ b/compiler/optimizing/intrinsics_arm64.cc @@ -1409,8 +1409,9 @@ void IntrinsicCodeGeneratorARM64::VisitStringNewStringFromBytes(HInvoke* invoke) __ Ldr(lr, MemOperand(tr, QUICK_ENTRYPOINT_OFFSET(kArm64WordSize, pAllocStringFromBytes).Int32Value())); - codegen_->RecordPcInfo(invoke, invoke->GetDexPc()); + CheckEntrypointTypes(); __ Blr(lr); + codegen_->RecordPcInfo(invoke, invoke->GetDexPc()); __ Bind(slow_path->GetExitLabel()); } @@ -1436,19 +1437,17 @@ void IntrinsicCodeGeneratorARM64::VisitStringNewStringFromChars(HInvoke* invoke) // all include a null check on `data` before calling that method. __ Ldr(lr, MemOperand(tr, QUICK_ENTRYPOINT_OFFSET(kArm64WordSize, pAllocStringFromChars).Int32Value())); - codegen_->RecordPcInfo(invoke, invoke->GetDexPc()); + CheckEntrypointTypes(); __ Blr(lr); + codegen_->RecordPcInfo(invoke, invoke->GetDexPc()); } void IntrinsicLocationsBuilderARM64::VisitStringNewStringFromString(HInvoke* invoke) { - // The inputs plus one temp. LocationSummary* locations = new (arena_) LocationSummary(invoke, LocationSummary::kCall, kIntrinsified); InvokeRuntimeCallingConvention calling_convention; locations->SetInAt(0, LocationFrom(calling_convention.GetRegisterAt(0))); - locations->SetInAt(1, LocationFrom(calling_convention.GetRegisterAt(1))); - locations->SetInAt(2, LocationFrom(calling_convention.GetRegisterAt(2))); locations->SetOut(calling_convention.GetReturnLocation(Primitive::kPrimNot)); } @@ -1464,8 +1463,9 @@ void IntrinsicCodeGeneratorARM64::VisitStringNewStringFromString(HInvoke* invoke __ Ldr(lr, MemOperand(tr, QUICK_ENTRYPOINT_OFFSET(kArm64WordSize, pAllocStringFromString).Int32Value())); - codegen_->RecordPcInfo(invoke, invoke->GetDexPc()); + CheckEntrypointTypes(); __ Blr(lr); + codegen_->RecordPcInfo(invoke, invoke->GetDexPc()); __ Bind(slow_path->GetExitLabel()); } diff --git a/compiler/optimizing/intrinsics_mips64.cc b/compiler/optimizing/intrinsics_mips64.cc index 83dff33ec..33096cac0 100644 --- a/compiler/optimizing/intrinsics_mips64.cc +++ b/compiler/optimizing/intrinsics_mips64.cc @@ -1590,9 +1590,10 @@ void IntrinsicCodeGeneratorMIPS64::VisitStringNewStringFromBytes(HInvoke* invoke TR, QUICK_ENTRYPOINT_OFFSET(kMips64DoublewordSize, pAllocStringFromBytes).Int32Value()); - codegen_->RecordPcInfo(invoke, invoke->GetDexPc()); + CheckEntrypointTypes(); __ Jalr(TMP); __ Nop(); + codegen_->RecordPcInfo(invoke, invoke->GetDexPc()); __ Bind(slow_path->GetExitLabel()); } @@ -1623,20 +1624,19 @@ void IntrinsicCodeGeneratorMIPS64::VisitStringNewStringFromChars(HInvoke* invoke TR, QUICK_ENTRYPOINT_OFFSET(kMips64DoublewordSize, pAllocStringFromChars).Int32Value()); - codegen_->RecordPcInfo(invoke, invoke->GetDexPc()); + CheckEntrypointTypes(); __ Jalr(TMP); __ Nop(); + codegen_->RecordPcInfo(invoke, invoke->GetDexPc()); } -// java.lang.String.String(String original) +// java.lang.StringFactory.newStringFromString(String toCopy) void IntrinsicLocationsBuilderMIPS64::VisitStringNewStringFromString(HInvoke* invoke) { LocationSummary* locations = new (arena_) LocationSummary(invoke, LocationSummary::kCall, kIntrinsified); InvokeRuntimeCallingConvention calling_convention; locations->SetInAt(0, Location::RegisterLocation(calling_convention.GetRegisterAt(0))); - locations->SetInAt(1, Location::RegisterLocation(calling_convention.GetRegisterAt(1))); - locations->SetInAt(2, Location::RegisterLocation(calling_convention.GetRegisterAt(2))); Location outLocation = calling_convention.GetReturnLocation(Primitive::kPrimInt); locations->SetOut(Location::RegisterLocation(outLocation.AsRegister())); } @@ -1655,9 +1655,10 @@ void IntrinsicCodeGeneratorMIPS64::VisitStringNewStringFromString(HInvoke* invok TR, QUICK_ENTRYPOINT_OFFSET(kMips64DoublewordSize, pAllocStringFromString).Int32Value()); - codegen_->RecordPcInfo(invoke, invoke->GetDexPc()); + CheckEntrypointTypes(); __ Jalr(TMP); __ Nop(); + codegen_->RecordPcInfo(invoke, invoke->GetDexPc()); __ Bind(slow_path->GetExitLabel()); } diff --git a/compiler/optimizing/intrinsics_x86.cc b/compiler/optimizing/intrinsics_x86.cc index 048590e1b..065866396 100644 --- a/compiler/optimizing/intrinsics_x86.cc +++ b/compiler/optimizing/intrinsics_x86.cc @@ -1546,6 +1546,7 @@ void IntrinsicCodeGeneratorX86::VisitStringNewStringFromBytes(HInvoke* invoke) { __ j(kEqual, slow_path->GetEntryLabel()); __ fs()->call(Address::Absolute(QUICK_ENTRYPOINT_OFFSET(kX86WordSize, pAllocStringFromBytes))); + CheckEntrypointTypes(); codegen_->RecordPcInfo(invoke, invoke->GetDexPc()); __ Bind(slow_path->GetExitLabel()); } @@ -1571,6 +1572,7 @@ void IntrinsicCodeGeneratorX86::VisitStringNewStringFromChars(HInvoke* invoke) { // // all include a null check on `data` before calling that method. __ fs()->call(Address::Absolute(QUICK_ENTRYPOINT_OFFSET(kX86WordSize, pAllocStringFromChars))); + CheckEntrypointTypes(); codegen_->RecordPcInfo(invoke, invoke->GetDexPc()); } @@ -1594,6 +1596,7 @@ void IntrinsicCodeGeneratorX86::VisitStringNewStringFromString(HInvoke* invoke) __ j(kEqual, slow_path->GetEntryLabel()); __ fs()->call(Address::Absolute(QUICK_ENTRYPOINT_OFFSET(kX86WordSize, pAllocStringFromString))); + CheckEntrypointTypes(); codegen_->RecordPcInfo(invoke, invoke->GetDexPc()); __ Bind(slow_path->GetExitLabel()); } diff --git a/compiler/optimizing/intrinsics_x86_64.cc b/compiler/optimizing/intrinsics_x86_64.cc index 35e13a654..13a4bd08a 100644 --- a/compiler/optimizing/intrinsics_x86_64.cc +++ b/compiler/optimizing/intrinsics_x86_64.cc @@ -1641,6 +1641,7 @@ void IntrinsicCodeGeneratorX86_64::VisitStringNewStringFromBytes(HInvoke* invoke __ gs()->call(Address::Absolute(QUICK_ENTRYPOINT_OFFSET(kX86_64WordSize, pAllocStringFromBytes), /* no_rip */ true)); + CheckEntrypointTypes(); codegen_->RecordPcInfo(invoke, invoke->GetDexPc()); __ Bind(slow_path->GetExitLabel()); } @@ -1667,6 +1668,7 @@ void IntrinsicCodeGeneratorX86_64::VisitStringNewStringFromChars(HInvoke* invoke // all include a null check on `data` before calling that method. __ gs()->call(Address::Absolute(QUICK_ENTRYPOINT_OFFSET(kX86_64WordSize, pAllocStringFromChars), /* no_rip */ true)); + CheckEntrypointTypes(); codegen_->RecordPcInfo(invoke, invoke->GetDexPc()); } @@ -1691,6 +1693,7 @@ void IntrinsicCodeGeneratorX86_64::VisitStringNewStringFromString(HInvoke* invok __ gs()->call(Address::Absolute(QUICK_ENTRYPOINT_OFFSET(kX86_64WordSize, pAllocStringFromString), /* no_rip */ true)); + CheckEntrypointTypes(); codegen_->RecordPcInfo(invoke, invoke->GetDexPc()); __ Bind(slow_path->GetExitLabel()); } diff --git a/runtime/arch/arm/quick_entrypoints_arm.S b/runtime/arch/arm/quick_entrypoints_arm.S index cfcef4908..f33eebed9 100644 --- a/runtime/arch/arm/quick_entrypoints_arm.S +++ b/runtime/arch/arm/quick_entrypoints_arm.S @@ -276,7 +276,7 @@ ENTRY \name bl \entrypoint @ (field_idx, Object*, new_val, referrer, Thread*) add sp, #16 @ release out args .cfi_adjust_cfa_offset -16 - RESTORE_REFS_ONLY_CALLEE_SAVE_FRAME @ TODO: we can clearly save an add here + RESTORE_REFS_ONLY_CALLEE_SAVE_FRAME @ TODO: we can clearly save an add here \return END \name .endm @@ -812,14 +812,23 @@ END \name .macro FOUR_ARG_DOWNCALL name, entrypoint, return .extern \entrypoint ENTRY \name + sub sp, #12 @ alignment padding + .cfi_adjust_cfa_offset 12 + push {r3} @ Save r3 as is it used as a temp register in the + .cfi_adjust_cfa_offset 4 @ expansion of the SETUP_REFS_ONLY_CALLEE_SAVE_FRAME + .cfi_rel_offset r3, 0 @ macro below, which clobbers its arguments. SETUP_REFS_ONLY_CALLEE_SAVE_FRAME r3, r12 @ save callee saves in case of GC + ldr r3, [sp, 32] @ restore r3 + .cfi_restore r3 + str r9, [sp, #-16]! @ expand the frame and pass Thread::Current - .pad #16 .cfi_adjust_cfa_offset 16 bl \entrypoint add sp, #16 @ strip the extra frame .cfi_adjust_cfa_offset -16 RESTORE_REFS_ONLY_CALLEE_SAVE_FRAME + add sp, #16 @ pop r3 + padding + .cfi_adjust_cfa_offset -16 \return END \name .endm diff --git a/runtime/arch/x86/asm_support_x86.S b/runtime/arch/x86/asm_support_x86.S index 77b8e87c9..3e47209af 100644 --- a/runtime/arch/x86/asm_support_x86.S +++ b/runtime/arch/x86/asm_support_x86.S @@ -142,6 +142,10 @@ MACRO1(POP, reg) CFI_RESTORE(REG_VAR(reg)) END_MACRO +MACRO1(CFI_RESTORE_REG, reg) + CFI_RESTORE(REG_VAR(reg)) +END_MACRO + #define UNREACHABLE int3 MACRO1(UNIMPLEMENTED,name) diff --git a/runtime/arch/x86/quick_entrypoints_x86.S b/runtime/arch/x86/quick_entrypoints_x86.S index fbee5d772..4be00ce52 100644 --- a/runtime/arch/x86/quick_entrypoints_x86.S +++ b/runtime/arch/x86/quick_entrypoints_x86.S @@ -686,7 +686,15 @@ END_MACRO MACRO3(FOUR_ARG_DOWNCALL, c_name, cxx_name, return_macro) DEFINE_FUNCTION VAR(c_name) + subl MACRO_LITERAL(12), %esp // alignment padding + CFI_ADJUST_CFA_OFFSET(12) + PUSH ebx // Save ebx as the expansion of the + // SETUP_REFS_ONLY_CALLEE_SAVE_FRAME + // macro below clobbers it. SETUP_REFS_ONLY_CALLEE_SAVE_FRAME ebx, ebx // save ref containing registers for GC + movl 28(%esp), %ebx // restore ebx + CFI_RESTORE_REG ebx + // Outgoing argument set up subl MACRO_LITERAL(12), %esp // alignment padding CFI_ADJUST_CFA_OFFSET(12) @@ -700,6 +708,8 @@ MACRO3(FOUR_ARG_DOWNCALL, c_name, cxx_name, return_macro) addl MACRO_LITERAL(32), %esp // pop arguments CFI_ADJUST_CFA_OFFSET(-32) RESTORE_REFS_ONLY_CALLEE_SAVE_FRAME // restore frame up to return address + addl MACRO_LITERAL(16), %esp // pop ebx + padding + CFI_ADJUST_CFA_OFFSET(-16) CALL_MACRO(return_macro) // return or deliver exception END_FUNCTION VAR(c_name) END_MACRO diff --git a/test/580-checker-string-factory-intrinsics/expected.txt b/test/580-checker-string-factory-intrinsics/expected.txt new file mode 100644 index 000000000..86e041dad --- /dev/null +++ b/test/580-checker-string-factory-intrinsics/expected.txt @@ -0,0 +1,3 @@ +foo +bar +baz diff --git a/test/580-checker-string-factory-intrinsics/info.txt b/test/580-checker-string-factory-intrinsics/info.txt new file mode 100644 index 000000000..3d01a1964 --- /dev/null +++ b/test/580-checker-string-factory-intrinsics/info.txt @@ -0,0 +1 @@ +Ensure java.lang.StringFactory intrinsics are recognized and used. diff --git a/test/580-checker-string-factory-intrinsics/src/Main.java b/test/580-checker-string-factory-intrinsics/src/Main.java new file mode 100644 index 000000000..a2e34bffd --- /dev/null +++ b/test/580-checker-string-factory-intrinsics/src/Main.java @@ -0,0 +1,83 @@ +/* + * Copyright (C) 2016 The Android Open Source Project + * + * 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. + */ + +public class Main { + + /// CHECK-START: void Main.testNewStringFromBytes() builder (after) + /// CHECK-DAG: InvokeStaticOrDirect method_name:java.lang.StringFactory.newStringFromBytes intrinsic:None + + /// CHECK-START: void Main.testNewStringFromBytes() intrinsics_recognition (after) + /// CHECK-DAG: InvokeStaticOrDirect method_name:java.lang.StringFactory.newStringFromBytes intrinsic:StringNewStringFromBytes + + public static void testNewStringFromBytes() { + byte[] bytes = { 'f', 'o', 'o' }; + String s = StringFactory.newStringFromBytes(bytes, 0, 0, 3); + System.out.println(s); + } + + // The (native) method + // + // java.lang.StringFactory.newStringFromChars(int offset, int charCount, char[] data) + // + // is recognized as intrinsic StringNewStringFromChars. However, + // because this method is not public, we cannot call it and check + // that the compiler actually intrinsifies it (as it does for the + // StringNewStringFromBytes and StringNewStringFromString + // intrinsics) with Checker. + // + // We can call a public method such as + // + // java.lang.StringFactory.newStringFromChars(char[] data) + // + // which contains a call to the former (non-public) native method. + // However, this call will not be inlined (because it is a method in + // another Dex file and which contains a call, which needs an + // environment), so we cannot use Checker here to ensure the native + // call was intrinsified either. + + /// CHECK-START: void Main.testNewStringFromChars() builder (after) + /// CHECK-DAG: InvokeStaticOrDirect method_name:java.lang.StringFactory.newStringFromChars intrinsic:None + + /// CHECK-START: void Main.testNewStringFromChars() intrinsics_recognition (after) + /// CHECK-DAG: InvokeStaticOrDirect method_name:java.lang.StringFactory.newStringFromChars intrinsic:None + + /// CHECK-START: void Main.testNewStringFromChars() inliner (after) + /// CHECK-DAG: InvokeStaticOrDirect method_name:java.lang.StringFactory.newStringFromChars intrinsic:None + + public static void testNewStringFromChars() { + char[] chars = { 'b', 'a', 'r' }; + String s = StringFactory.newStringFromChars(chars); + System.out.println(s); + } + + /// CHECK-START: void Main.testNewStringFromString() builder (after) + /// CHECK-DAG: InvokeStaticOrDirect method_name:java.lang.StringFactory.newStringFromString intrinsic:None + + /// CHECK-START: void Main.testNewStringFromString() intrinsics_recognition (after) + /// CHECK-DAG: InvokeStaticOrDirect method_name:java.lang.StringFactory.newStringFromString intrinsic:StringNewStringFromString + + public static void testNewStringFromString() { + String s1 = "baz"; + String s2 = StringFactory.newStringFromString(s1); + System.out.println(s2); + } + + public static void main(String[] args) throws Exception { + testNewStringFromBytes(); + testNewStringFromChars(); + testNewStringFromString(); + } +} -- 2.11.0