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
diff --git a/compiler/driver/compiler_options.cc b/compiler/driver/compiler_options.cc
index 3ed5f88..d0770e9 100644
--- a/compiler/driver/compiler_options.cc
+++ b/compiler/driver/compiler_options.cc
@@ -57,7 +57,7 @@
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 74d081d..a5b3ae1 100644
--- a/compiler/driver/compiler_options.h
+++ b/compiler/driver/compiler_options.h
@@ -42,6 +42,7 @@
namespace linker {
class Arm64RelativePatcherTest;
+class Thumb2RelativePatcherTest;
} // namespace linker
class ArtMethod;
@@ -502,6 +503,7 @@
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 b4f0caf..ce0fdf2 100644
--- a/compiler/optimizing/code_generator_riscv64.cc
+++ b/compiler/optimizing/code_generator_riscv64.cc
@@ -2775,13 +2775,12 @@
}
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 0829bc2..cca11e0 100644
--- a/compiler/optimizing/optimizing_compiler.cc
+++ b/compiler/optimizing/optimizing_compiler.cc
@@ -782,6 +782,7 @@
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 f7abd6b..4885cfc 100644
--- a/dex2oat/linker/arm/relative_patcher_thumb2_test.cc
+++ b/dex2oat/linker/arm/relative_patcher_thumb2_test.cc
@@ -211,6 +211,19 @@
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 @@
(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 @@
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 @@
}
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::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 @@
}
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 @@
};
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 @@
};
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 09fbfc7..b140a5e 100644
--- a/dex2oat/linker/arm64/relative_patcher_arm64_test.cc
+++ b/dex2oat/linker/arm64/relative_patcher_arm64_test.cc
@@ -184,6 +184,12 @@
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 @@
(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 @@
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 @@
}
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 @@
};
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 1e9b0e1..7a8497d 100644
--- a/dex2oat/linker/relative_patcher_test.h
+++ b/dex2oat/linker/relative_patcher_test.h
@@ -78,7 +78,7 @@
// 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 b32fd28..3bd24c4 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 c456817..3ad6b36 100644
--- a/runtime/arch/riscv64/quick_entrypoints_riscv64.S
+++ b/runtime/arch/riscv64/quick_entrypoints_riscv64.S
@@ -419,8 +419,7 @@
.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