From bffdc70d4b6dd994adf48b015e7818094f30938f Mon Sep 17 00:00:00 2001 From: Scott Wakeling Date: Wed, 7 Dec 2016 17:46:03 +0000 Subject: [PATCH] Revert "Revert "ARM: VIXL32: Use DontCare for SetFlags + fix for GenerateFrameEntry."" Override Add in ArmVIXLMacroAssembler to improve 16-bit encodings. This reverts commit 2f34995469e20a1ac342975856155f69995997ce. Test: m test-art-host Change-Id: Ief9f7576cd805104fd517a76b96d8a92f2208dfd --- compiler/optimizing/code_generator_arm_vixl.cc | 24 +++++++-- compiler/utils/arm/assembler_arm_vixl.cc | 22 ++++++++ compiler/utils/arm/assembler_arm_vixl.h | 71 ++++++++++++++++++++++++++ test/538-checker-embed-constants/src/Main.java | 6 +-- 4 files changed, 115 insertions(+), 8 deletions(-) diff --git a/compiler/optimizing/code_generator_arm_vixl.cc b/compiler/optimizing/code_generator_arm_vixl.cc index 4b24ac345..c617a6af0 100644 --- a/compiler/optimizing/code_generator_arm_vixl.cc +++ b/compiler/optimizing/code_generator_arm_vixl.cc @@ -897,10 +897,25 @@ void CodeGeneratorARMVIXL::GenerateFrameEntry() { GetAssembler()->cfi().AdjustCFAOffset(kArmWordSize * POPCOUNT(fpu_spill_mask_)); GetAssembler()->cfi().RelOffsetForMany(DWARFReg(s0), 0, fpu_spill_mask_, kArmWordSize); } + + if (GetGraph()->HasShouldDeoptimizeFlag()) { + UseScratchRegisterScope temps(GetVIXLAssembler()); + vixl32::Register temp = temps.Acquire(); + // Initialize should_deoptimize flag to 0. + __ Mov(temp, 0); + GetAssembler()->StoreToOffset(kStoreWord, temp, sp, -kShouldDeoptimizeFlagSize); + } + int adjust = GetFrameSize() - FrameEntrySpillSize(); __ Sub(sp, sp, adjust); GetAssembler()->cfi().AdjustCFAOffset(adjust); - GetAssembler()->StoreToOffset(kStoreWord, kMethodRegister, sp, 0); + + // Save the current method if we need it. Note that we do not + // do this in HCurrentMethod, as the instruction might have been removed + // in the SSA graph. + if (RequiresCurrentMethod()) { + GetAssembler()->StoreToOffset(kStoreWord, kMethodRegister, sp, 0); + } } void CodeGeneratorARMVIXL::GenerateFrameExit() { @@ -3103,8 +3118,7 @@ void InstructionCodeGeneratorARMVIXL::HandleLongRotate(HRor* ror) { __ And(shift_right, RegisterFrom(rhs), 0x1F); __ Lsrs(shift_left, RegisterFrom(rhs), 6); - // TODO(VIXL): Check that flags are kept after "vixl32::LeaveFlags" enabled. - __ Rsb(shift_left, shift_right, Operand::From(kArmBitsPerWord)); + __ Rsb(LeaveFlags, shift_left, shift_right, Operand::From(kArmBitsPerWord)); __ B(cc, &shift_by_32_plus_shift_right); // out_reg_hi = (reg_hi << shift_left) | (reg_lo >> shift_right). @@ -6249,9 +6263,9 @@ void InstructionCodeGeneratorARMVIXL::GenerateAddLongConst(Location out, return; } __ Adds(out_low, first_low, value_low); - if (GetAssembler()->ShifterOperandCanHold(ADC, value_high, kCcKeep)) { + if (GetAssembler()->ShifterOperandCanHold(ADC, value_high, kCcDontCare)) { __ Adc(out_high, first_high, value_high); - } else if (GetAssembler()->ShifterOperandCanHold(SBC, ~value_high, kCcKeep)) { + } else if (GetAssembler()->ShifterOperandCanHold(SBC, ~value_high, kCcDontCare)) { __ Sbc(out_high, first_high, ~value_high); } else { LOG(FATAL) << "Unexpected constant " << value_high; diff --git a/compiler/utils/arm/assembler_arm_vixl.cc b/compiler/utils/arm/assembler_arm_vixl.cc index c35c39328..1614d04a9 100644 --- a/compiler/utils/arm/assembler_arm_vixl.cc +++ b/compiler/utils/arm/assembler_arm_vixl.cc @@ -455,5 +455,27 @@ void ArmVIXLMacroAssembler::CompareAndBranchIfNonZero(vixl32::Register rn, B(ne, label); } +void ArmVIXLMacroAssembler::B(vixl32::Label* label) { + if (!label->IsBound()) { + // Try to use 16-bit T2 encoding of B instruction. + DCHECK(OutsideITBlock()); + AssemblerAccurateScope ass(this, + kMaxInstructionSizeInBytes, + CodeBufferCheckScope::kMaximumSize); + b(al, Narrow, label); + AddBranchLabel(label); + return; + } + MacroAssembler::B(label); +} + +void ArmVIXLMacroAssembler::B(vixl32::Condition cond, vixl32::Label* label) { + // To further reduce the Bcc encoding size and use 16-bit T1 encoding, + // we can provide a hint to this function: i.e. far_target=false. + // By default this function uses 'EncodingSizeType::Best' which generates 32-bit T3 encoding. + MacroAssembler::B(cond, label); +} + + } // namespace arm } // namespace art diff --git a/compiler/utils/arm/assembler_arm_vixl.h b/compiler/utils/arm/assembler_arm_vixl.h index b4a4abc87..17cf1064b 100644 --- a/compiler/utils/arm/assembler_arm_vixl.h +++ b/compiler/utils/arm/assembler_arm_vixl.h @@ -54,6 +54,77 @@ class ArmVIXLMacroAssembler FINAL : public vixl32::MacroAssembler { void CompareAndBranchIfNonZero(vixl32::Register rn, vixl32::Label* label, bool is_far_target = true); + + // In T32 some of the instructions (add, mov, etc) outside an IT block + // have only 32-bit encodings. But there are 16-bit flag setting + // versions of these instructions (adds, movs, etc). In most of the + // cases in ART we don't care if the instructions keep flags or not; + // thus we can benefit from smaller code size. + // VIXL will never generate flag setting versions (for example, adds + // for Add macro instruction) unless vixl32::DontCare option is + // explicitly specified. That's why we introduce wrappers to use + // DontCare option by default. +#define WITH_FLAGS_DONT_CARE_RD_RN_OP(func_name) \ + void (func_name)(vixl32::Register rd, vixl32::Register rn, const vixl32::Operand& operand) { \ + MacroAssembler::func_name(vixl32::DontCare, rd, rn, operand); \ + } \ + using MacroAssembler::func_name + + WITH_FLAGS_DONT_CARE_RD_RN_OP(Adc); + WITH_FLAGS_DONT_CARE_RD_RN_OP(Sub); + WITH_FLAGS_DONT_CARE_RD_RN_OP(Sbc); + WITH_FLAGS_DONT_CARE_RD_RN_OP(Rsb); + WITH_FLAGS_DONT_CARE_RD_RN_OP(Rsc); + + WITH_FLAGS_DONT_CARE_RD_RN_OP(Eor); + WITH_FLAGS_DONT_CARE_RD_RN_OP(Orr); + WITH_FLAGS_DONT_CARE_RD_RN_OP(Orn); + WITH_FLAGS_DONT_CARE_RD_RN_OP(And); + WITH_FLAGS_DONT_CARE_RD_RN_OP(Bic); + + WITH_FLAGS_DONT_CARE_RD_RN_OP(Asr); + WITH_FLAGS_DONT_CARE_RD_RN_OP(Lsr); + WITH_FLAGS_DONT_CARE_RD_RN_OP(Lsl); + WITH_FLAGS_DONT_CARE_RD_RN_OP(Ror); + +#undef WITH_FLAGS_DONT_CARE_RD_RN_OP + +#define WITH_FLAGS_DONT_CARE_RD_OP(func_name) \ + void (func_name)(vixl32::Register rd, const vixl32::Operand& operand) { \ + MacroAssembler::func_name(vixl32::DontCare, rd, operand); \ + } \ + using MacroAssembler::func_name + + WITH_FLAGS_DONT_CARE_RD_OP(Mvn); + WITH_FLAGS_DONT_CARE_RD_OP(Mov); + +#undef WITH_FLAGS_DONT_CARE_RD_OP + + // The following two functions don't fall into above categories. Overload them separately. + void Rrx(vixl32::Register rd, vixl32::Register rn) { + MacroAssembler::Rrx(vixl32::DontCare, rd, rn); + } + using MacroAssembler::Rrx; + + void Mul(vixl32::Register rd, vixl32::Register rn, vixl32::Register rm) { + MacroAssembler::Mul(vixl32::DontCare, rd, rn, rm); + } + using MacroAssembler::Mul; + + // TODO: Remove when MacroAssembler::Add(FlagsUpdate, Condition, Register, Register, Operand) + // makes the right decision about 16-bit encodings. + void Add(vixl32::Register rd, vixl32::Register rn, const vixl32::Operand& operand) { + if (rd.Is(rn)) { + MacroAssembler::Add(rd, rn, operand); + } else { + MacroAssembler::Add(vixl32::DontCare, rd, rn, operand); + } + } + using MacroAssembler::Add; + + // These interfaces try to use 16-bit T2 encoding of B instruction. + void B(vixl32::Label* label); + void B(vixl32::Condition cond, vixl32::Label* label); }; class ArmVIXLAssembler FINAL : public Assembler { diff --git a/test/538-checker-embed-constants/src/Main.java b/test/538-checker-embed-constants/src/Main.java index 0329e63ff..4f34ec90e 100644 --- a/test/538-checker-embed-constants/src/Main.java +++ b/test/538-checker-embed-constants/src/Main.java @@ -308,7 +308,7 @@ public class Main { } /// CHECK-START-ARM: long Main.shl32(long) disassembly (after) - /// CHECK-DAG: mov {{r\d+}}, {{r\d+}} + /// CHECK-DAG: mov{{s?}} {{r\d+}}, {{r\d+}} /// CHECK-DAG: mov{{s?|\.w}} {{r\d+}}, #0 /// CHECK-START-ARM: long Main.shl32(long) disassembly (after) @@ -377,7 +377,7 @@ public class Main { /// CHECK-START-ARM: long Main.shr32(long) disassembly (after) /// CHECK-DAG: asr{{s?|\.w}} {{r\d+}}, <>, #31 - /// CHECK-DAG: mov {{r\d+}}, <> + /// CHECK-DAG: mov{{s?}} {{r\d+}}, <> /// CHECK-START-ARM: long Main.shr32(long) disassembly (after) /// CHECK-NOT: asr{{s?|\.w}} {{r\d+}}, {{r\d+}}, {{r\d+}} @@ -445,7 +445,7 @@ public class Main { } /// CHECK-START-ARM: long Main.ushr32(long) disassembly (after) - /// CHECK-DAG: mov {{r\d+}}, {{r\d+}} + /// CHECK-DAG: mov{{s?}} {{r\d+}}, {{r\d+}} /// CHECK-DAG: mov{{s?|\.w}} {{r\d+}}, #0 /// CHECK-START-ARM: long Main.ushr32(long) disassembly (after) -- 2.11.0