Skip to content

Commit

Permalink
ART: Make InstructionSet an enum class and add kLast.
Browse files Browse the repository at this point in the history 
Adding InstructionSet::kLast shall make it easier to encode
the InstructionSet in fewer bits using BitField<>. However,
introducing `kLast` into the `art` namespace is not a good
idea, so we change the InstructionSet to an enum class.
This also uncovered a case of InstructionSet::kNone being
erroneously used instead of vixl32::Condition::None(), so
it's good to remove `kNone` from the `art` namespace.

Test: m test-art-host-gtest
Test: testrunner.py --host --optimizing
Change-Id: I6fa6168dfba4ed6da86d021a69c80224f09997a6
  • Loading branch information
@vmarko
vmarko committed Nov 2, 2017
1 parent 321b3ca commit 33bff25
Show file tree
Hide file tree
Showing 114 changed files with 721 additions and 658 deletions.
4 changes: 2 additions & 2 deletions cmdline/cmdline.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ struct CmdlineArgs {
} else if (option.starts_with("--instruction-set=")) {
StringPiece instruction_set_str = option.substr(strlen("--instruction-set=")).data();
instruction_set_ = GetInstructionSetFromString(instruction_set_str.data());
if (instruction_set_ == kNone) {
if (instruction_set_ == InstructionSet::kNone) {
fprintf(stderr, "Unsupported instruction set %s\n", instruction_set_str.data());
PrintUsage();
return false;
Expand Down Expand Up @@ -263,7 +263,7 @@ struct CmdlineArgs {

DBG_LOG << "boot_image_location parent_dir_name was " << parent_dir_name;

if (GetInstructionSetFromString(parent_dir_name.c_str()) != kNone) {
if (GetInstructionSetFromString(parent_dir_name.c_str()) != InstructionSet::kNone) {
*error_msg = "Do not specify the architecture as part of the boot image location";
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/cfi_test.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class CFITest : public dwarf::DwarfTest {
: &Thread::DumpThreadOffset<PointerSize::k32>);
std::unique_ptr<Disassembler> disasm(Disassembler::Create(isa, opts));
std::stringstream stream;
const uint8_t* base = actual_asm.data() + (isa == kThumb2 ? 1 : 0);
const uint8_t* base = actual_asm.data() + (isa == InstructionSet::kThumb2 ? 1 : 0);
disasm->Dump(stream, base, base + actual_asm.size());
ReformatAsm(&stream, &lines);
// Print CFI and assembly interleaved.
Expand Down
28 changes: 14 additions & 14 deletions compiler/compiled_method.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,14 @@ size_t CompiledCode::CodeDelta() const {

size_t CompiledCode::CodeDelta(InstructionSet instruction_set) {
switch (instruction_set) {
case kArm:
case kArm64:
case kMips:
case kMips64:
case kX86:
case kX86_64:
case InstructionSet::kArm:
case InstructionSet::kArm64:
case InstructionSet::kMips:
case InstructionSet::kMips64:
case InstructionSet::kX86:
case InstructionSet::kX86_64:
return 0;
case kThumb2: {
case InstructionSet::kThumb2: {
// +1 to set the low-order bit so a BLX will switch to Thumb mode
return 1;
}
Expand All @@ -80,14 +80,14 @@ size_t CompiledCode::CodeDelta(InstructionSet instruction_set) {

const void* CompiledCode::CodePointer(const void* code_pointer, InstructionSet instruction_set) {
switch (instruction_set) {
case kArm:
case kArm64:
case kMips:
case kMips64:
case kX86:
case kX86_64:
case InstructionSet::kArm:
case InstructionSet::kArm64:
case InstructionSet::kMips:
case InstructionSet::kMips64:
case InstructionSet::kX86:
case InstructionSet::kX86_64:
return code_pointer;
case kThumb2: {
case InstructionSet::kThumb2: {
uintptr_t address = reinterpret_cast<uintptr_t>(code_pointer);
// Set the low-order bit so a BLX will switch to Thumb mode
address |= 0x1;
Expand Down
3 changes: 2 additions & 1 deletion compiler/debug/dwarf/dwarf_test.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ class DwarfTest : public CommonRuntimeTest {
template<typename ElfTypes>
std::vector<std::string> Objdump(const char* args) {
// Write simple elf file with just the DWARF sections.
InstructionSet isa = (sizeof(typename ElfTypes::Addr) == 8) ? kX86_64 : kX86;
InstructionSet isa =
(sizeof(typename ElfTypes::Addr) == 8) ? InstructionSet::kX86_64 : InstructionSet::kX86;
ScratchFile file;
linker::FileOutputStream output_stream(file.GetFile());
linker::ElfBuilder<ElfTypes> builder(isa, nullptr, &output_stream);
Expand Down
16 changes: 8 additions & 8 deletions compiler/debug/elf_debug_frame_writer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ static void WriteCIE(InstructionSet isa,
// debugger that its value in the previous frame is not recoverable.
bool is64bit = Is64BitInstructionSet(isa);
switch (isa) {
case kArm:
case kThumb2: {
case InstructionSet::kArm:
case InstructionSet::kThumb2: {
dwarf::DebugFrameOpCodeWriter<> opcodes;
opcodes.DefCFA(Reg::ArmCore(13), 0); // R13(SP).
// core registers.
Expand All @@ -61,7 +61,7 @@ static void WriteCIE(InstructionSet isa,
WriteCIE(is64bit, return_reg, opcodes, format, buffer);
return;
}
case kArm64: {
case InstructionSet::kArm64: {
dwarf::DebugFrameOpCodeWriter<> opcodes;
opcodes.DefCFA(Reg::Arm64Core(31), 0); // R31(SP).
// core registers.
Expand All @@ -84,8 +84,8 @@ static void WriteCIE(InstructionSet isa,
WriteCIE(is64bit, return_reg, opcodes, format, buffer);
return;
}
case kMips:
case kMips64: {
case InstructionSet::kMips:
case InstructionSet::kMips64: {
dwarf::DebugFrameOpCodeWriter<> opcodes;
opcodes.DefCFA(Reg::MipsCore(29), 0); // R29(SP).
// core registers.
Expand All @@ -108,7 +108,7 @@ static void WriteCIE(InstructionSet isa,
WriteCIE(is64bit, return_reg, opcodes, format, buffer);
return;
}
case kX86: {
case InstructionSet::kX86: {
// FIXME: Add fp registers once libunwind adds support for them. Bug: 20491296
constexpr bool generate_opcodes_for_x86_fp = false;
dwarf::DebugFrameOpCodeWriter<> opcodes;
Expand All @@ -134,7 +134,7 @@ static void WriteCIE(InstructionSet isa,
WriteCIE(is64bit, return_reg, opcodes, format, buffer);
return;
}
case kX86_64: {
case InstructionSet::kX86_64: {
dwarf::DebugFrameOpCodeWriter<> opcodes;
opcodes.DefCFA(Reg::X86_64Core(4), 8); // R4(RSP).
opcodes.Offset(Reg::X86_64Core(16), -8); // R16(RIP).
Expand All @@ -160,7 +160,7 @@ static void WriteCIE(InstructionSet isa,
WriteCIE(is64bit, return_reg, opcodes, format, buffer);
return;
}
case kNone:
case InstructionSet::kNone:
break;
}
LOG(FATAL) << "Cannot write CIE frame for ISA " << isa;
Expand Down
16 changes: 8 additions & 8 deletions compiler/debug/elf_debug_line_writer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,19 +68,19 @@ class ElfDebugLineWriter {
int code_factor_bits_ = 0;
int dwarf_isa = -1;
switch (isa) {
case kArm: // arm actually means thumb2.
case kThumb2:
case InstructionSet::kArm: // arm actually means thumb2.
case InstructionSet::kThumb2:
code_factor_bits_ = 1; // 16-bit instuctions
dwarf_isa = 1; // DW_ISA_ARM_thumb.
break;
case kArm64:
case kMips:
case kMips64:
case InstructionSet::kArm64:
case InstructionSet::kMips:
case InstructionSet::kMips64:
code_factor_bits_ = 2; // 32-bit instructions
break;
case kNone:
case kX86:
case kX86_64:
case InstructionSet::kNone:
case InstructionSet::kX86:
case InstructionSet::kX86_64:
break;
}
std::unordered_set<uint64_t> seen_addresses(compilation_unit.methods.size());
Expand Down
34 changes: 17 additions & 17 deletions compiler/debug/elf_debug_loc_writer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,41 +33,41 @@ using Reg = dwarf::Reg;

static Reg GetDwarfCoreReg(InstructionSet isa, int machine_reg) {
switch (isa) {
case kArm:
case kThumb2:
case InstructionSet::kArm:
case InstructionSet::kThumb2:
return Reg::ArmCore(machine_reg);
case kArm64:
case InstructionSet::kArm64:
return Reg::Arm64Core(machine_reg);
case kX86:
case InstructionSet::kX86:
return Reg::X86Core(machine_reg);
case kX86_64:
case InstructionSet::kX86_64:
return Reg::X86_64Core(machine_reg);
case kMips:
case InstructionSet::kMips:
return Reg::MipsCore(machine_reg);
case kMips64:
case InstructionSet::kMips64:
return Reg::Mips64Core(machine_reg);
case kNone:
case InstructionSet::kNone:
LOG(FATAL) << "No instruction set";
}
UNREACHABLE();
}

static Reg GetDwarfFpReg(InstructionSet isa, int machine_reg) {
switch (isa) {
case kArm:
case kThumb2:
case InstructionSet::kArm:
case InstructionSet::kThumb2:
return Reg::ArmFp(machine_reg);
case kArm64:
case InstructionSet::kArm64:
return Reg::Arm64Fp(machine_reg);
case kX86:
case InstructionSet::kX86:
return Reg::X86Fp(machine_reg);
case kX86_64:
case InstructionSet::kX86_64:
return Reg::X86_64Fp(machine_reg);
case kMips:
case InstructionSet::kMips:
return Reg::MipsFp(machine_reg);
case kMips64:
case InstructionSet::kMips64:
return Reg::Mips64Fp(machine_reg);
case kNone:
case InstructionSet::kNone:
LOG(FATAL) << "No instruction set";
}
UNREACHABLE();
Expand Down Expand Up @@ -230,7 +230,7 @@ static void WriteDebugLocEntry(const MethodDebugInfo* method_info,
break; // the high word is correctly implied by the low word.
}
} else if (kind == Kind::kInFpuRegister) {
if ((isa == kArm || isa == kThumb2) &&
if ((isa == InstructionSet::kArm || isa == InstructionSet::kThumb2) &&
piece == 0 && reg_hi.GetKind() == Kind::kInFpuRegister &&
reg_hi.GetValue() == value + 1 && value % 2 == 0) {
// Translate S register pair to D register (e.g. S4+S5 to D2).
Expand Down
2 changes: 1 addition & 1 deletion compiler/debug/elf_symtab_writer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ static void WriteDebugSymbols(linker::ElfBuilder<ElfTypes>* builder,
// instructions, so that disassembler tools can correctly disassemble.
// Note that even if we generate just a single mapping symbol, ARM's Streamline
// requires it to match function symbol. Just address 0 does not work.
if (info.isa == kThumb2) {
if (info.isa == InstructionSet::kThumb2) {
if (address < mapping_symbol_address || !kGenerateSingleArmMappingSymbol) {
symtab->Add(strtab->Write("$t"), text, address & ~1, 0, STB_LOCAL, STT_NOTYPE);
mapping_symbol_address = address;
Expand Down
4 changes: 2 additions & 2 deletions compiler/dex/dex_to_dex_compiler.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -381,9 +381,9 @@ CompiledMethod* ArtCompileDEX(
quicken_data.push_back(static_cast<uint8_t>(info.dex_member_index >> 8));
}
InstructionSet instruction_set = driver->GetInstructionSet();
if (instruction_set == kThumb2) {
if (instruction_set == InstructionSet::kThumb2) {
// Don't use the thumb2 instruction set to avoid the one off code delta.
instruction_set = kArm;
instruction_set = InstructionSet::kArm;
}
return CompiledMethod::SwapAllocCompiledMethod(
driver,
Expand Down
4 changes: 2 additions & 2 deletions compiler/driver/compiled_method_storage_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ TEST(CompiledMethodStorage, Deduplicate) {
CompilerDriver driver(&compiler_options,
&verification_results,
Compiler::kOptimizing,
/* instruction_set_ */ kNone,
/* instruction_set_ */ InstructionSet::kNone,
/* instruction_set_features */ nullptr,
/* image_classes */ nullptr,
/* compiled_classes */ nullptr,
Expand Down Expand Up @@ -91,7 +91,7 @@ TEST(CompiledMethodStorage, Deduplicate) {
for (auto&& f : cfi_info) {
for (auto&& p : patches) {
compiled_methods.push_back(CompiledMethod::SwapAllocCompiledMethod(
&driver, kNone, c, 0u, 0u, 0u, s, v, f, p));
&driver, InstructionSet::kNone, c, 0u, 0u, 0u, s, v, f, p));
}
}
}
Expand Down
17 changes: 9 additions & 8 deletions compiler/driver/compiler_driver.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,8 @@ CompilerDriver::CompilerDriver(
verification_results_(verification_results),
compiler_(Compiler::Create(this, compiler_kind)),
compiler_kind_(compiler_kind),
instruction_set_(instruction_set == kArm ? kThumb2 : instruction_set),
instruction_set_(
instruction_set == InstructionSet::kArm ? InstructionSet::kThumb2 : instruction_set),
instruction_set_features_(instruction_set_features),
requires_constructor_barrier_lock_("constructor barrier lock"),
non_relative_linker_patch_count_(0u),
Expand Down Expand Up @@ -451,13 +452,13 @@ static optimizer::DexToDexCompilationLevel GetDexToDexCompilationLevel(
// GetQuickGenericJniStub allowing down calls that aren't compiled using a JNI compiler?
static bool InstructionSetHasGenericJniStub(InstructionSet isa) {
switch (isa) {
case kArm:
case kArm64:
case kThumb2:
case kMips:
case kMips64:
case kX86:
case kX86_64: return true;
case InstructionSet::kArm:
case InstructionSet::kArm64:
case InstructionSet::kThumb2:
case InstructionSet::kMips:
case InstructionSet::kMips64:
case InstructionSet::kX86:
case InstructionSet::kX86_64: return true;
default: return false;
}
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/exception_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ class ExceptionTest : public CommonRuntimeTest {
static_cast<const void*>(fake_header_code_and_maps_.data() +
(fake_header_code_and_maps_.size() - code_size)));

if (kRuntimeISA == kArm) {
if (kRuntimeISA == InstructionSet::kArm) {
// Check that the Thumb2 adjustment will be a NOP, see EntryPointToCodePointer().
CHECK_ALIGNED(stack_maps_offset, 2);
}
Expand Down
14 changes: 7 additions & 7 deletions compiler/jni/jni_cfi_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,13 +102,13 @@ class JNICFITest : public CFITest {
}
};

#define TEST_ISA(isa) \
TEST_F(JNICFITest, isa) { \
std::vector<uint8_t> expected_asm(expected_asm_##isa, \
expected_asm_##isa + arraysize(expected_asm_##isa)); \
std::vector<uint8_t> expected_cfi(expected_cfi_##isa, \
expected_cfi_##isa + arraysize(expected_cfi_##isa)); \
TestImpl(isa, #isa, expected_asm, expected_cfi); \
#define TEST_ISA(isa) \
TEST_F(JNICFITest, isa) { \
std::vector<uint8_t> expected_asm(expected_asm_##isa, \
expected_asm_##isa + arraysize(expected_asm_##isa)); \
std::vector<uint8_t> expected_cfi(expected_cfi_##isa, \
expected_cfi_##isa + arraysize(expected_cfi_##isa)); \
TestImpl(InstructionSet::isa, #isa, expected_asm, expected_cfi); \
}

#ifdef ART_ENABLE_CODEGEN_arm
Expand Down
Loading

0 comments on commit 33bff25

Please sign in to comment.