Skip to content

Commit

Permalink
riscv64: Implement CriticalNativeAbiFixupRiscv64.
Browse files Browse the repository at this point in the history
And pass integral stack args sign-extended to 64 bits for
direct @CriticalNative calls. Enable direct @CriticalNative
call codegen unconditionally and also enable `HClinitCheck`
codegen and extend the 178-app-image-native-method run-test
to properly test these use cases.

Test: # Edit `run-test` to disable checker, then
      testrunner.py --target --64 --ndebug --optimizing
      # Ignore 6 pre-existing failures (down from 7).
Bug: 283082089
Change-Id: Ia514c62006c7079b04182cc39e413eb2deb089c1
  • Loading branch information
vmarko committed Oct 17, 2023
1 parent f7bd87e commit d5c097b
Show file tree
Hide file tree
Showing 14 changed files with 484 additions and 70 deletions.
1 change: 1 addition & 0 deletions compiler/Android.bp
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,7 @@ art_cc_defaults {
srcs: [
"jni/quick/riscv64/calling_convention_riscv64.cc",
"optimizing/code_generator_riscv64.cc",
"optimizing/critical_native_abi_fixup_riscv64.cc",
"optimizing/intrinsics_riscv64.cc",
"utils/riscv64/assembler_riscv64.cc",
"utils/riscv64/jni_macro_assembler_riscv64.cc",
Expand Down
2 changes: 1 addition & 1 deletion compiler/optimizing/code_generator.h
Original file line number Diff line number Diff line change
Expand Up @@ -598,7 +598,7 @@ class CodeGenerator : public DeletableArenaObject<kArenaAllocCodeGenerator> {
if (kIsDebugBuild) {
uint32_t shorty_len;
const char* shorty = GetCriticalNativeShorty(invoke, &shorty_len);
DCHECK_EQ(GetCriticalNativeDirectCallFrameSize(shorty, shorty_len), out_frame_size);
CHECK_EQ(GetCriticalNativeDirectCallFrameSize(shorty, shorty_len), out_frame_size);
}
if (out_frame_size != 0u) {
FinishCriticalNativeFrameSetup(out_frame_size, &parallel_move);
Expand Down
37 changes: 24 additions & 13 deletions compiler/optimizing/code_generator_riscv64.cc
Original file line number Diff line number Diff line change
Expand Up @@ -207,11 +207,13 @@ Location CriticalNativeCallingConventionVisitorRiscv64::GetNextLocation(DataType
if (fpr_index_ < kParameterFpuRegistersLength) {
location = Location::FpuRegisterLocation(kParameterFpuRegisters[fpr_index_]);
++fpr_index_;
} else {
// Native ABI allows passing excessive FP args in GPRs. This is facilitated by
// inserting fake conversion intrinsic calls (`Double.doubleToRawLongBits()`
// or `Float.floatToRawIntBits()`) by `CriticalNativeAbiFixupRiscv64`.
// Remaining FP args shall be passed on the stack.
CHECK_EQ(gpr_index_, kRuntimeParameterCoreRegistersLength);
}
// Native ABI allows passing excessive FP args in GPRs. This is facilitated by
// inserting fake conversion intrinsic calls (`Double.doubleToRawLongBits()`
// or `Float.floatToRawIntBits()`) by `CriticalNativeAbiFixupRiscv64`.
// TODO(riscv64): Implement these intrinsics and `CriticalNativeAbiFixupRiscv64`.
} else {
// Native ABI uses the same core registers as a runtime call.
if (gpr_index_ < kRuntimeParameterCoreRegistersLength) {
Expand All @@ -220,10 +222,11 @@ Location CriticalNativeCallingConventionVisitorRiscv64::GetNextLocation(DataType
}
}
if (location.IsInvalid()) {
if (DataType::Is64BitType(type)) {
location = Location::DoubleStackSlot(stack_offset_);
} else {
// Only a `float` gets a single slot. Integral args need to be sign-extended to 64 bits.
if (type == DataType::Type::kFloat32) {
location = Location::StackSlot(stack_offset_);
} else {
location = Location::DoubleStackSlot(stack_offset_);
}
stack_offset_ += kFramePointerSize;

Expand Down Expand Up @@ -5840,10 +5843,13 @@ void CodeGeneratorRISCV64::MoveLocation(Location destination,
destination.IsStackSlot() ? DataType::Type::kFloat32 : DataType::Type::kFloat64;
}
}
DCHECK((destination.IsDoubleStackSlot() == DataType::Is64BitType(dst_type)) &&
(source.IsFpuRegister() == DataType::IsFloatingPointType(dst_type)));
DCHECK_EQ(source.IsFpuRegister(), DataType::IsFloatingPointType(dst_type));
// For direct @CriticalNative calls, we need to sign-extend narrow integral args
// to 64 bits, so widening integral values is allowed. Narrowing is forbidden.
DCHECK_IMPLIES(DataType::IsFloatingPointType(dst_type) || destination.IsStackSlot(),
destination.IsDoubleStackSlot() == DataType::Is64BitType(dst_type));
// Move to stack from GPR/FPR
if (DataType::Is64BitType(dst_type)) {
if (destination.IsDoubleStackSlot()) {
if (source.IsRegister()) {
__ Stored(source.AsRegister<XRegister>(), SP, destination.GetStackIndex());
} else {
Expand Down Expand Up @@ -5872,15 +5878,20 @@ void CodeGeneratorRISCV64::MoveLocation(Location destination,
}
} else {
DCHECK(source.IsStackSlot() || source.IsDoubleStackSlot());
DCHECK_EQ(source.IsDoubleStackSlot(), destination.IsDoubleStackSlot());
// For direct @CriticalNative calls, we need to sign-extend narrow integral args
// to 64 bits, so widening move is allowed. Narrowing move is forbidden.
DCHECK_IMPLIES(destination.IsStackSlot(), source.IsStackSlot());
// Move to stack from stack
ScratchRegisterScope srs(GetAssembler());
XRegister tmp = srs.AllocateXRegister();
if (destination.IsStackSlot()) {
if (source.IsStackSlot()) {
__ Loadw(tmp, SP, source.GetStackIndex());
__ Storew(tmp, SP, destination.GetStackIndex());
} else {
__ Loadd(tmp, SP, source.GetStackIndex());
}
if (destination.IsStackSlot()) {
__ Storew(tmp, SP, destination.GetStackIndex());
} else {
__ Stored(tmp, SP, destination.GetStackIndex());
}
}
Expand Down
45 changes: 1 addition & 44 deletions compiler/optimizing/critical_native_abi_fixup_arm.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,8 @@

#include "critical_native_abi_fixup_arm.h"

#include "art_method-inl.h"
#include "intrinsics.h"
#include "jni/jni_internal.h"
#include "nodes.h"
#include "scoped_thread_state_change-inl.h"
#include "well_known_classes.h"

namespace art HIDDEN {
namespace arm {
Expand All @@ -43,46 +39,7 @@ static void FixUpArguments(HInvokeStaticOrDirect* invoke) {
break; // Remaining arguments are passed on stack.
}
if (DataType::IsFloatingPointType(input_type)) {
bool is_double = (input_type == DataType::Type::kFloat64);
DataType::Type converted_type = is_double ? DataType::Type::kInt64 : DataType::Type::kInt32;
ArtMethod* resolved_method = is_double
? WellKnownClasses::java_lang_Double_doubleToRawLongBits
: WellKnownClasses::java_lang_Float_floatToRawIntBits;
DCHECK(resolved_method != nullptr);
DCHECK(resolved_method->IsIntrinsic());
MethodReference target_method(nullptr, 0);
{
ScopedObjectAccess soa(Thread::Current());
target_method =
MethodReference(resolved_method->GetDexFile(), resolved_method->GetDexMethodIndex());
}
// Use arbitrary dispatch info that does not require the method argument.
HInvokeStaticOrDirect::DispatchInfo dispatch_info = {
MethodLoadKind::kBssEntry,
CodePtrLocation::kCallArtMethod,
/*method_load_data=*/ 0u
};
HBasicBlock* block = invoke->GetBlock();
ArenaAllocator* allocator = block->GetGraph()->GetAllocator();
HInvokeStaticOrDirect* new_input = new (allocator) HInvokeStaticOrDirect(
allocator,
/*number_of_arguments=*/ 1u,
converted_type,
invoke->GetDexPc(),
/*method_reference=*/ MethodReference(nullptr, dex::kDexNoIndex),
resolved_method,
dispatch_info,
kStatic,
target_method,
HInvokeStaticOrDirect::ClinitCheckRequirement::kNone,
!block->GetGraph()->IsDebuggable());
// The intrinsic has no side effects and does not need environment or dex cache on ARM.
new_input->SetSideEffects(SideEffects::None());
IntrinsicOptimizations opt(new_input);
opt.SetDoesNotNeedEnvironment();
new_input->SetRawInputAt(0u, input);
block->InsertInstructionBefore(new_input, invoke);
invoke->ReplaceInput(new_input, i);
InsertFpToIntegralIntrinsic(invoke, i);
}
reg = next_reg;
}
Expand Down
71 changes: 71 additions & 0 deletions compiler/optimizing/critical_native_abi_fixup_riscv64.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/*
* Copyright (C) 2023 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.
*/

#include "critical_native_abi_fixup_riscv64.h"

#include "arch/riscv64/jni_frame_riscv64.h"
#include "intrinsics.h"
#include "nodes.h"

namespace art HIDDEN {
namespace riscv64 {

// Fix up FP arguments passed in core registers for call to @CriticalNative by inserting fake calls
// to Float.floatToRawIntBits() or Double.doubleToRawLongBits() to satisfy type consistency checks.
static void FixUpArguments(HInvokeStaticOrDirect* invoke) {
DCHECK_EQ(invoke->GetCodePtrLocation(), CodePtrLocation::kCallCriticalNative);
size_t core_reg = 0u;
size_t fp_reg = 0u;
for (size_t i = 0, num_args = invoke->GetNumberOfArguments(); i != num_args; ++i) {
if (core_reg == kMaxIntLikeArgumentRegisters) {
break; // Remaining arguments are passed in FP regs or on the stack.
}
HInstruction* input = invoke->InputAt(i);
DataType::Type input_type = input->GetType();
if (DataType::IsFloatingPointType(input_type)) {
if (fp_reg < kMaxFloatOrDoubleArgumentRegisters) {
++fp_reg;
} else {
DCHECK_LT(core_reg, kMaxIntLikeArgumentRegisters);
InsertFpToIntegralIntrinsic(invoke, i);
++core_reg;
}
} else {
++core_reg;
}
}
}

bool CriticalNativeAbiFixupRiscv64::Run() {
if (!graph_->HasDirectCriticalNativeCall()) {
return false;
}

for (HBasicBlock* block : graph_->GetReversePostOrder()) {
for (HInstructionIterator it(block->GetInstructions()); !it.Done(); it.Advance()) {
HInstruction* instruction = it.Current();
if (instruction->IsInvokeStaticOrDirect() &&
instruction->AsInvokeStaticOrDirect()->GetCodePtrLocation() ==
CodePtrLocation::kCallCriticalNative) {
FixUpArguments(instruction->AsInvokeStaticOrDirect());
}
}
}
return true;
}

} // namespace riscv64
} // namespace art
41 changes: 41 additions & 0 deletions compiler/optimizing/critical_native_abi_fixup_riscv64.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* Copyright (C) 2023 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.
*/

#ifndef ART_COMPILER_OPTIMIZING_CRITICAL_NATIVE_ABI_FIXUP_RISCV64_H_
#define ART_COMPILER_OPTIMIZING_CRITICAL_NATIVE_ABI_FIXUP_RISCV64_H_

#include "base/macros.h"
#include "nodes.h"
#include "optimization.h"

namespace art HIDDEN {
namespace riscv64 {

class CriticalNativeAbiFixupRiscv64 : public HOptimization {
public:
CriticalNativeAbiFixupRiscv64(HGraph* graph, OptimizingCompilerStats* stats)
: HOptimization(graph, kCriticalNativeAbiFixupRiscv64PassName, stats) {}

static constexpr const char* kCriticalNativeAbiFixupRiscv64PassName =
"critical_native_abi_fixup_riscv64";

bool Run() override;
};

} // namespace riscv64
} // namespace art

#endif // ART_COMPILER_OPTIMIZING_CRITICAL_NATIVE_ABI_FIXUP_RISCV64_H_
51 changes: 51 additions & 0 deletions compiler/optimizing/intrinsics.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include "obj_ptr-inl.h"
#include "scoped_thread_state_change-inl.h"
#include "thread-current-inl.h"
#include "well_known_classes.h"

namespace art HIDDEN {

Expand Down Expand Up @@ -412,4 +413,54 @@ void IntrinsicVisitor::AssertNonMovableStringClass() {
}
}

void InsertFpToIntegralIntrinsic(HInvokeStaticOrDirect* invoke, size_t input_index) {
DCHECK_EQ(invoke->GetCodePtrLocation(), CodePtrLocation::kCallCriticalNative);
DCHECK(!invoke->GetBlock()->GetGraph()->IsDebuggable())
<< "Unexpected direct @CriticalNative call in a debuggable graph!";
DCHECK_LT(input_index, invoke->GetNumberOfArguments());
HInstruction* input = invoke->InputAt(input_index);
DataType::Type input_type = input->GetType();
DCHECK(DataType::IsFloatingPointType(input_type));
bool is_double = (input_type == DataType::Type::kFloat64);
DataType::Type converted_type = is_double ? DataType::Type::kInt64 : DataType::Type::kInt32;
ArtMethod* resolved_method = is_double
? WellKnownClasses::java_lang_Double_doubleToRawLongBits
: WellKnownClasses::java_lang_Float_floatToRawIntBits;
DCHECK(resolved_method != nullptr);
DCHECK(resolved_method->IsIntrinsic());
MethodReference target_method(nullptr, 0);
{
ScopedObjectAccess soa(Thread::Current());
target_method =
MethodReference(resolved_method->GetDexFile(), resolved_method->GetDexMethodIndex());
}
// Use arbitrary dispatch info that does not require the method argument.
HInvokeStaticOrDirect::DispatchInfo dispatch_info = {
MethodLoadKind::kBssEntry,
CodePtrLocation::kCallArtMethod,
/*method_load_data=*/ 0u
};
HBasicBlock* block = invoke->GetBlock();
ArenaAllocator* allocator = block->GetGraph()->GetAllocator();
HInvokeStaticOrDirect* new_input = new (allocator) HInvokeStaticOrDirect(
allocator,
/*number_of_arguments=*/ 1u,
converted_type,
invoke->GetDexPc(),
/*method_reference=*/ MethodReference(nullptr, dex::kDexNoIndex),
resolved_method,
dispatch_info,
kStatic,
target_method,
HInvokeStaticOrDirect::ClinitCheckRequirement::kNone,
/*enable_intrinsic_opt=*/ true);
// The intrinsic has no side effects and does not need the environment.
new_input->SetSideEffects(SideEffects::None());
IntrinsicOptimizations opt(new_input);
opt.SetDoesNotNeedEnvironment();
new_input->SetRawInputAt(0u, input);
block->InsertInstructionBefore(new_input, invoke);
invoke->ReplaceInput(new_input, input_index);
}

} // namespace art
5 changes: 5 additions & 0 deletions compiler/optimizing/intrinsics.h
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,11 @@ bool IsCallFreeIntrinsic(HInvoke* invoke, Codegenerator* codegen) {
return false;
}

// Insert a `Float.floatToRawIntBits()` or `Double.doubleToRawLongBits()` intrinsic for a
// given input. These fake calls are needed on arm and riscv64 to satisfy type consistency
// checks while passing certain FP args in core registers for direct @CriticalNative calls.
void InsertFpToIntegralIntrinsic(HInvokeStaticOrDirect* invoke, size_t input_index);

} // namespace art

#endif // ART_COMPILER_OPTIMIZING_INTRINSICS_H_
16 changes: 16 additions & 0 deletions compiler/optimizing/optimization.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@
#ifdef ART_ENABLE_CODEGEN_arm64
#include "instruction_simplifier_arm64.h"
#endif
#ifdef ART_ENABLE_CODEGEN_riscv64
#include "critical_native_abi_fixup_riscv64.h"
#endif
#ifdef ART_ENABLE_CODEGEN_x86
#include "pc_relative_fixups_x86.h"
#include "instruction_simplifier_x86.h"
Expand Down Expand Up @@ -109,6 +112,10 @@ const char* OptimizationPassName(OptimizationPass pass) {
case OptimizationPass::kInstructionSimplifierArm64:
return arm64::InstructionSimplifierArm64::kInstructionSimplifierArm64PassName;
#endif
#ifdef ART_ENABLE_CODEGEN_riscv64
case OptimizationPass::kCriticalNativeAbiFixupRiscv64:
return riscv64::CriticalNativeAbiFixupRiscv64::kCriticalNativeAbiFixupRiscv64PassName;
#endif
#ifdef ART_ENABLE_CODEGEN_x86
case OptimizationPass::kPcRelativeFixupsX86:
return x86::PcRelativeFixups::kPcRelativeFixupsX86PassName;
Expand Down Expand Up @@ -155,6 +162,9 @@ OptimizationPass OptimizationPassByName(const std::string& pass_name) {
#ifdef ART_ENABLE_CODEGEN_arm64
X(OptimizationPass::kInstructionSimplifierArm64);
#endif
#ifdef ART_ENABLE_CODEGEN_riscv64
X(OptimizationPass::kCriticalNativeAbiFixupRiscv64);
#endif
#ifdef ART_ENABLE_CODEGEN_x86
X(OptimizationPass::kPcRelativeFixupsX86);
X(OptimizationPass::kX86MemoryOperandGeneration);
Expand Down Expand Up @@ -303,6 +313,12 @@ ArenaVector<HOptimization*> ConstructOptimizations(
opt = new (allocator) arm64::InstructionSimplifierArm64(graph, stats);
break;
#endif
#ifdef ART_ENABLE_CODEGEN_riscv64
case OptimizationPass::kCriticalNativeAbiFixupRiscv64:
DCHECK(alt_name == nullptr) << "arch-specific pass does not support alternative name";
opt = new (allocator) riscv64::CriticalNativeAbiFixupRiscv64(graph, stats);
break;
#endif
#ifdef ART_ENABLE_CODEGEN_x86
case OptimizationPass::kPcRelativeFixupsX86:
DCHECK(alt_name == nullptr) << "arch-specific pass does not support alternative name";
Expand Down
3 changes: 3 additions & 0 deletions compiler/optimizing/optimization.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,9 @@ enum class OptimizationPass {
#ifdef ART_ENABLE_CODEGEN_arm64
kInstructionSimplifierArm64,
#endif
#ifdef ART_ENABLE_CODEGEN_riscv64
kCriticalNativeAbiFixupRiscv64,
#endif
#ifdef ART_ENABLE_CODEGEN_x86
kPcRelativeFixupsX86,
kInstructionSimplifierX86,
Expand Down
Loading

0 comments on commit d5c097b

Please sign in to comment.