summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Vladimir Marko <vmarko@google.com> 2023-09-07 11:37:44 +0000
committer VladimĂ­r Marko <vmarko@google.com> 2023-09-08 13:45:02 +0000
commit1dc27c9f2b132aeccb55f45cc17fbc4dc4a35fa0 (patch)
treee7f3e602eb05794ded4733d72dd5239a3d38b364
parent8aa54bd205ae14d523f243bfe6892c65d8e65527 (diff)
riscv64: [codegen] Add VisitNullCheck
Also enable codegen for NullCheck and fix related entrypoint issues. For now, this is using explicit null checks. Change the default value of the implicit null checks flag in the `CompilerOptions`. In `dex2oat` we expect the default value to be false and override it to true when needed by the selected architecture. This aligns with the behaviour in `Runtime` which is the source of this information for JIT. We do not change the default value for implicit stack overflow checks flag yet because it requires additional adjustments to avoid breaking certain gtests. Test: m test-art-host-gtest Test: aosp_cf_riscv64_phone-userdebug boots. Test: run-gtests.sh # Ignore pre-existing timeout in `TestImageLayout`. Test: testrunner.py --target --64 --optimizing # Ignore 49 pre-existing failures. Bug: 283082089 Change-Id: If663d3279da5e6c53669860cefa7185c53e7e146
-rw-r--r--compiler/driver/compiler_options.cc2
-rw-r--r--compiler/driver/compiler_options.h2
-rw-r--r--compiler/optimizing/code_generator_riscv64.cc7
-rw-r--r--compiler/optimizing/optimizing_compiler.cc1
-rw-r--r--dex2oat/linker/arm/relative_patcher_thumb2_test.cc47
-rw-r--r--dex2oat/linker/arm64/relative_patcher_arm64_test.cc27
-rw-r--r--dex2oat/linker/relative_patcher_test.h2
-rw-r--r--runtime/arch/riscv64/asm_support_riscv64.S8
-rw-r--r--runtime/arch/riscv64/quick_entrypoints_riscv64.S3
9 files changed, 75 insertions, 24 deletions
diff --git a/compiler/driver/compiler_options.cc b/compiler/driver/compiler_options.cc
index 3ed5f88875..d0770e952b 100644
--- a/compiler/driver/compiler_options.cc
+++ b/compiler/driver/compiler_options.cc
@@ -57,7 +57,7 @@ CompilerOptions::CompilerOptions()
generate_debug_info_(kDefaultGenerateDebugInfo),
generate_mini_debug_info_(kDefaultGenerateMiniDebugInfo),
generate_build_id_(false),
- implicit_null_checks_(true),
+ implicit_null_checks_(false),
implicit_so_checks_(true),
implicit_suspend_checks_(false),
compile_pic_(false),
diff --git a/compiler/driver/compiler_options.h b/compiler/driver/compiler_options.h
index 74d081d29e..a5b3ae17d0 100644
--- a/compiler/driver/compiler_options.h
+++ b/compiler/driver/compiler_options.h
@@ -42,6 +42,7 @@ class VerifierDepsTest;
namespace linker {
class Arm64RelativePatcherTest;
+class Thumb2RelativePatcherTest;
} // namespace linker
class ArtMethod;
@@ -502,6 +503,7 @@ class CompilerOptions final {
friend class jit::JitCompiler;
friend class verifier::VerifierDepsTest;
friend class linker::Arm64RelativePatcherTest;
+ friend class linker::Thumb2RelativePatcherTest;
template <class Base>
friend bool ReadCompilerOptions(Base& map, CompilerOptions* options, std::string* error_msg);
diff --git a/compiler/optimizing/code_generator_riscv64.cc b/compiler/optimizing/code_generator_riscv64.cc
index b4f0caf607..ce0fdf20b5 100644
--- a/compiler/optimizing/code_generator_riscv64.cc
+++ b/compiler/optimizing/code_generator_riscv64.cc
@@ -2775,13 +2775,12 @@ void InstructionCodeGeneratorRISCV64::VisitNullConstant(HNullConstant* instructi
}
void LocationsBuilderRISCV64::VisitNullCheck(HNullCheck* instruction) {
- UNUSED(instruction);
- LOG(FATAL) << "Unimplemented";
+ LocationSummary* locations = codegen_->CreateThrowingSlowPathLocations(instruction);
+ locations->SetInAt(0, Location::RequiresRegister());
}
void InstructionCodeGeneratorRISCV64::VisitNullCheck(HNullCheck* instruction) {
- UNUSED(instruction);
- LOG(FATAL) << "Unimplemented";
+ codegen_->GenerateNullCheck(instruction);
}
void LocationsBuilderRISCV64::VisitOr(HOr* instruction) {
diff --git a/compiler/optimizing/optimizing_compiler.cc b/compiler/optimizing/optimizing_compiler.cc
index 0829bc2912..cca11e0c7e 100644
--- a/compiler/optimizing/optimizing_compiler.cc
+++ b/compiler/optimizing/optimizing_compiler.cc
@@ -782,6 +782,7 @@ static bool CanAssembleGraphForRiscv64(HGraph* graph) {
case HInstruction::kInvokeVirtual:
case HInstruction::kInvokeInterface:
case HInstruction::kCurrentMethod:
+ case HInstruction::kNullCheck:
break;
case HInstruction::kInvokeStaticOrDirect:
if (it.Current()->AsInvokeStaticOrDirect()->GetCodePtrLocation() ==
diff --git a/dex2oat/linker/arm/relative_patcher_thumb2_test.cc b/dex2oat/linker/arm/relative_patcher_thumb2_test.cc
index f7abd6b9eb..4885cfc6db 100644
--- a/dex2oat/linker/arm/relative_patcher_thumb2_test.cc
+++ b/dex2oat/linker/arm/relative_patcher_thumb2_test.cc
@@ -211,6 +211,19 @@ class Thumb2RelativePatcherTest : public RelativePatcherTest {
OptimizingUnitTestHelper helper;
HGraph* graph = helper.CreateGraph();
CompilerOptions compiler_options;
+
+ // Set isa to Thumb2.
+ compiler_options.instruction_set_ = instruction_set_;
+ compiler_options.instruction_set_features_ =
+ InstructionSetFeatures::FromBitmap(instruction_set_, instruction_set_features_->AsBitmap());
+ CHECK(compiler_options.instruction_set_features_->Equals(instruction_set_features_.get()));
+
+ // If a test requests that implicit null checks are enabled or disabled,
+ // apply that option, otherwise use the default from `CompilerOptions`.
+ if (implicit_null_checks_.has_value()) {
+ compiler_options.implicit_null_checks_ = implicit_null_checks_.value();
+ }
+
arm::CodeGeneratorARMVIXL codegen(graph, compiler_options);
ArenaVector<uint8_t> code(helper.GetAllocator()->Adapter());
codegen.EmitThunkCode(patch, &code, debug_name);
@@ -326,8 +339,16 @@ class Thumb2RelativePatcherTest : public RelativePatcherTest {
(static_cast<uint32_t>(output_[offset + 1]) << 8);
}
- void TestBakerFieldWide(uint32_t offset, uint32_t ref_reg);
- void TestBakerFieldNarrow(uint32_t offset, uint32_t ref_reg);
+ void TestBakerFieldWide(uint32_t offset, uint32_t ref_reg, bool implicit_null_checks);
+ void TestBakerFieldNarrow(uint32_t offset, uint32_t ref_reg, bool implicit_null_checks);
+
+ void Reset() final {
+ RelativePatcherTest::Reset();
+ implicit_null_checks_ = std::nullopt;
+ }
+
+ private:
+ std::optional<bool> implicit_null_checks_ = std::nullopt;
};
const uint8_t Thumb2RelativePatcherTest::kCallRawCode[] = {
@@ -702,7 +723,10 @@ const uint32_t kBakerValidRegsNarrow[] = {
0, 1, 2, 3, 4, 5, 6, 7,
};
-void Thumb2RelativePatcherTest::TestBakerFieldWide(uint32_t offset, uint32_t ref_reg) {
+void Thumb2RelativePatcherTest::TestBakerFieldWide(uint32_t offset,
+ uint32_t ref_reg,
+ bool implicit_null_checks) {
+ implicit_null_checks_ = implicit_null_checks;
DCHECK_ALIGNED(offset, 4u);
DCHECK_LT(offset, 4 * KB);
constexpr size_t kMethodCodeSize = 8u;
@@ -750,7 +774,7 @@ void Thumb2RelativePatcherTest::TestBakerFieldWide(uint32_t offset, uint32_t ref
}
size_t gray_check_offset = thunk_offset;
- if (holder_reg == base_reg) {
+ if (implicit_null_checks && holder_reg == base_reg) {
// Verify that the null-check uses the correct register, i.e. holder_reg.
if (holder_reg < 8) {
ASSERT_GE(output_.size() - gray_check_offset, 2u);
@@ -797,7 +821,10 @@ void Thumb2RelativePatcherTest::TestBakerFieldWide(uint32_t offset, uint32_t ref
}
}
-void Thumb2RelativePatcherTest::TestBakerFieldNarrow(uint32_t offset, uint32_t ref_reg) {
+void Thumb2RelativePatcherTest::TestBakerFieldNarrow(uint32_t offset,
+ uint32_t ref_reg,
+ bool implicit_null_checks) {
+ implicit_null_checks_ = implicit_null_checks;
DCHECK_ALIGNED(offset, 4u);
DCHECK_LT(offset, 32u);
constexpr size_t kMethodCodeSize = 6u;
@@ -851,7 +878,7 @@ void Thumb2RelativePatcherTest::TestBakerFieldNarrow(uint32_t offset, uint32_t r
}
size_t gray_check_offset = thunk_offset;
- if (holder_reg == base_reg) {
+ if (implicit_null_checks && holder_reg == base_reg) {
// Verify that the null-check uses the correct register, i.e. holder_reg.
if (holder_reg < 8) {
ASSERT_GE(output_.size() - gray_check_offset, 2u);
@@ -911,7 +938,9 @@ TEST_F(Thumb2RelativePatcherTest, BakerOffsetWide) {
};
for (const TestCase& test_case : test_cases) {
Reset();
- TestBakerFieldWide(test_case.offset, test_case.ref_reg);
+ TestBakerFieldWide(test_case.offset, test_case.ref_reg, /*implicit_null_checks=*/ true);
+ Reset();
+ TestBakerFieldWide(test_case.offset, test_case.ref_reg, /*implicit_null_checks=*/ false);
}
}
@@ -927,7 +956,9 @@ TEST_F(Thumb2RelativePatcherTest, BakerOffsetNarrow) {
};
for (const TestCase& test_case : test_cases) {
Reset();
- TestBakerFieldNarrow(test_case.offset, test_case.ref_reg);
+ TestBakerFieldNarrow(test_case.offset, test_case.ref_reg, /*implicit_null_checks=*/ true);
+ Reset();
+ TestBakerFieldNarrow(test_case.offset, test_case.ref_reg, /*implicit_null_checks=*/ false);
}
}
diff --git a/dex2oat/linker/arm64/relative_patcher_arm64_test.cc b/dex2oat/linker/arm64/relative_patcher_arm64_test.cc
index 09fbfc7f48..b140a5e41b 100644
--- a/dex2oat/linker/arm64/relative_patcher_arm64_test.cc
+++ b/dex2oat/linker/arm64/relative_patcher_arm64_test.cc
@@ -184,6 +184,12 @@ class Arm64RelativePatcherTest : public RelativePatcherTest {
InstructionSetFeatures::FromBitmap(instruction_set_, instruction_set_features_->AsBitmap());
CHECK(compiler_options.instruction_set_features_->Equals(instruction_set_features_.get()));
+ // If a test requests that implicit null checks are enabled or disabled,
+ // apply that option, otherwise use the default from `CompilerOptions`.
+ if (implicit_null_checks_.has_value()) {
+ compiler_options.implicit_null_checks_ = implicit_null_checks_.value();
+ }
+
arm64::CodeGeneratorARM64 codegen(graph, compiler_options);
ArenaVector<uint8_t> code(helper.GetAllocator()->Adapter());
codegen.EmitThunkCode(patch, &code, debug_name);
@@ -556,7 +562,15 @@ class Arm64RelativePatcherTest : public RelativePatcherTest {
(static_cast<uint32_t>(output_[offset + 3]) << 24);
}
- void TestBakerField(uint32_t offset, uint32_t ref_reg);
+ void TestBakerField(uint32_t offset, uint32_t ref_reg, bool implicit_null_checks);
+
+ void Reset() final {
+ RelativePatcherTest::Reset();
+ implicit_null_checks_ = std::nullopt;
+ }
+
+ private:
+ std::optional<bool> implicit_null_checks_ = std::nullopt;
};
const uint8_t Arm64RelativePatcherTest::kCallRawCode[] = {
@@ -1038,7 +1052,10 @@ TEST_F(Arm64RelativePatcherTestDefault, EntrypointCall) {
EXPECT_EQ(br_ip0, GetOutputInsn(thunk_offset + 4u));
}
-void Arm64RelativePatcherTest::TestBakerField(uint32_t offset, uint32_t ref_reg) {
+void Arm64RelativePatcherTest::TestBakerField(uint32_t offset,
+ uint32_t ref_reg,
+ bool implicit_null_checks) {
+ implicit_null_checks_ = implicit_null_checks;
uint32_t valid_regs[] = {
0, 1, 2, 3, 4, 5, 6, 7, 8, 9,
10, 11, 12, 13, 14, 15, 18, 19, // IP0 and IP1 are reserved.
@@ -1092,7 +1109,7 @@ void Arm64RelativePatcherTest::TestBakerField(uint32_t offset, uint32_t ref_reg)
}
size_t gray_check_offset = thunk_offset;
- if (holder_reg == base_reg) {
+ if (implicit_null_checks && holder_reg == base_reg) {
// Verify that the null-check CBZ uses the correct register, i.e. holder_reg.
ASSERT_GE(output_.size() - gray_check_offset, 4u);
ASSERT_EQ(0x34000000u | holder_reg, GetOutputInsn(thunk_offset) & 0xff00001fu);
@@ -1138,7 +1155,9 @@ TEST_F(Arm64RelativePatcherTestDefault, BakerOffset) {
};
for (const TestCase& test_case : test_cases) {
Reset();
- TestBakerField(test_case.offset, test_case.ref_reg);
+ TestBakerField(test_case.offset, test_case.ref_reg, /*implicit_null_checks=*/ true);
+ Reset();
+ TestBakerField(test_case.offset, test_case.ref_reg, /*implicit_null_checks=*/ false);
}
}
diff --git a/dex2oat/linker/relative_patcher_test.h b/dex2oat/linker/relative_patcher_test.h
index 1e9b0e1307..7a8497d1ea 100644
--- a/dex2oat/linker/relative_patcher_test.h
+++ b/dex2oat/linker/relative_patcher_test.h
@@ -78,7 +78,7 @@ class RelativePatcherTest : public testing::Test {
// Reset the helper to start another test. Creating and tearing down the Runtime is expensive,
// so we merge related tests together.
- void Reset() {
+ virtual void Reset() {
thunk_provider_.Reset();
method_offset_map_.map.clear();
patcher_ = RelativePatcher::Create(instruction_set_,
diff --git a/runtime/arch/riscv64/asm_support_riscv64.S b/runtime/arch/riscv64/asm_support_riscv64.S
index b32fd28780..3bd24c4c29 100644
--- a/runtime/arch/riscv64/asm_support_riscv64.S
+++ b/runtime/arch/riscv64/asm_support_riscv64.S
@@ -240,12 +240,12 @@
.endm
-.macro SETUP_CALLEE_SAVE_FRAME_COMMON tmpreg, offset
+.macro SETUP_CALLEE_SAVE_FRAME_COMMON tmpreg, runtime_method_offset
// art::Runtime* tmpreg = art::Runtime::instance_;
LOAD_RUNTIME_INSTANCE \tmpreg
// ArtMethod* tmpreg = Runtime::instance_->callee_save_methods_[<callee-save-frame-type>];
- ld \tmpreg, \offset(\tmpreg)
+ ld \tmpreg, \runtime_method_offset(\tmpreg)
SETUP_CALLEE_SAVE_FRAME_COMMON_INTERNAL \tmpreg
.endm
@@ -356,7 +356,7 @@
.endm
-.macro SETUP_SAVE_EVERYTHING_FRAME offset
+.macro SETUP_SAVE_EVERYTHING_FRAME runtime_method_offset = RUNTIME_SAVE_EVERYTHING_METHOD_OFFSET
#if (FRAME_SIZE_SAVE_EVERYTHING != 8*(1 + 32 + 27))
#error "FRAME_SIZE_SAVE_EVERYTHING(RISCV64) size not as expected."
#endif
@@ -430,7 +430,7 @@
SAVE_GPR ra, 8*59 // x1, return address
- SETUP_CALLEE_SAVE_FRAME_COMMON t0, \offset
+ SETUP_CALLEE_SAVE_FRAME_COMMON t0, \runtime_method_offset
.endm
diff --git a/runtime/arch/riscv64/quick_entrypoints_riscv64.S b/runtime/arch/riscv64/quick_entrypoints_riscv64.S
index c456817b59..3ad6b365da 100644
--- a/runtime/arch/riscv64/quick_entrypoints_riscv64.S
+++ b/runtime/arch/riscv64/quick_entrypoints_riscv64.S
@@ -419,8 +419,7 @@ END art_quick_to_interpreter_bridge
.extern artMethodExitHook
ENTRY art_quick_method_exit_hook
- SETUP_SAVE_EVERYTHING_FRAME \
- RUNTIME_SAVE_EVERYTHING_METHOD_OFFSET
+ SETUP_SAVE_EVERYTHING_FRAME
// frame_size is passed in A4 from JITed code and `art_quick_generic_jni_trampoline`.
addi a3, sp, SAVE_EVERYTHING_FRAME_OFFSET_FA0 // FP result ptr in kSaveEverything frame