Fix Portable build
A previous change broke the ability to compile the Portable build.
Instead of building Portable, we unwittingly were building Iceland.
This CL fixes that, along with some bitrot unknowingly introduced
by subsequent CLs.
In particular, the debugger support change to add an intrinsic
call to associate value definitions with the original Dalvik
VReg broke llvm's Phi constraint. Fixed.
However, problems remain. Though the build is now fixed, CLs
subsequent to the makefile breakage broke the Portable llvm backend.
This CL will shift all Portable compilations to Iceland while the
new breakage is being addressed.
Change-Id: I7b08b26ed653a7b477f0ae9a226b7ee09c6b3398
diff --git a/build/Android.libart-compiler-llvm.mk b/build/Android.libart-compiler-llvm.mk
index daef06e..4909f81 100644
--- a/build/Android.libart-compiler-llvm.mk
+++ b/build/Android.libart-compiler-llvm.mk
@@ -15,9 +15,9 @@
#
-LIBART_COMPILER_LLVM_CFLAGS := -DART_USE_LLVM_COMPILER
+LIBART_COMPILER_LLVM_CFLAGS := -DART_USE_LLVM_COMPILER=1
ifeq ($(ART_USE_PORTABLE_COMPILER),true)
- ART_TEST_CFLAGS += -DART_USE_PORTABLE_COMPILER=1
+ LIBART_COMPILER_LLVM_CFLAGS += -DART_USE_PORTABLE_COMPILER=1
endif
LIBART_COMPILER_LLVM_SRC_FILES += \
diff --git a/build/Android.libart.mk b/build/Android.libart.mk
index 435056a..5226db7 100644
--- a/build/Android.libart.mk
+++ b/build/Android.libart.mk
@@ -19,7 +19,7 @@
LIBART_CFLAGS += -DART_USE_LLVM_COMPILER=1
endif
ifeq ($(ART_USE_PORTABLE_COMPILER),true)
- ART_TEST_CFLAGS += -DART_USE_PORTABLE_COMPILER=1
+ LIBART_CFLAGS += -DART_USE_PORTABLE_COMPILER=1
endif
# $(1): target or host
diff --git a/src/compiler/codegen/mir_to_gbc.cc b/src/compiler/codegen/mir_to_gbc.cc
index e1e1fb1..766a630 100644
--- a/src/compiler/codegen/mir_to_gbc.cc
+++ b/src/compiler/codegen/mir_to_gbc.cc
@@ -50,8 +50,22 @@
return reinterpret_cast<llvm::Value*>(GrowableListGetElement(&cu->llvm_values, s_reg));
}
+static void SetVregOnValue(CompilationUnit* cu, llvm::Value* val, int s_reg)
+{
+ // Set vreg for debugging
+ if (cu->compiler->IsDebuggingSupported()) {
+ greenland::IntrinsicHelper::IntrinsicId id =
+ greenland::IntrinsicHelper::SetVReg;
+ llvm::Function* func = cu->intrinsic_helper->GetIntrinsicFunction(id);
+ int v_reg = SRegToVReg(cu, s_reg);
+ llvm::Value* table_slot = cu->irb->getInt32(v_reg);
+ llvm::Value* args[] = { table_slot, val };
+ cu->irb->CreateCall(func, args);
+ }
+}
+
// Replace the placeholder value with the real definition
-static void DefineValue(CompilationUnit* cu, llvm::Value* val, int s_reg)
+static void DefineValueOnly(CompilationUnit* cu, llvm::Value* val, int s_reg)
{
llvm::Value* placeholder = GetLLVMValue(cu, s_reg);
if (placeholder == NULL) {
@@ -66,16 +80,12 @@
DCHECK(inst != NULL);
inst->eraseFromParent();
- // Set vreg for debugging
- if (!cu->compiler->IsDebuggingSupported()) {
- greenland::IntrinsicHelper::IntrinsicId id =
- greenland::IntrinsicHelper::SetVReg;
- llvm::Function* func = cu->intrinsic_helper->GetIntrinsicFunction(id);
- int v_reg = SRegToVReg(cu, s_reg);
- llvm::Value* table_slot = cu->irb->getInt32(v_reg);
- llvm::Value* args[] = { table_slot, val };
- cu->irb->CreateCall(func, args);
- }
+}
+
+static void DefineValue(CompilationUnit* cu, llvm::Value* val, int s_reg)
+{
+ DefineValueOnly(cu, val, s_reg);
+ SetVregOnValue(cu, val, s_reg);
}
static llvm::Type* LlvmTypeFromLocRec(CompilationUnit* cu, RegLocation loc)
@@ -1625,83 +1635,6 @@
return res;
}
-/* Extended MIR instructions like PHI */
-static void ConvertExtendedMIR(CompilationUnit* cu, BasicBlock* bb, MIR* mir,
- llvm::BasicBlock* llvm_bb)
-{
-
- switch (static_cast<ExtendedMIROpcode>(mir->dalvikInsn.opcode)) {
- case kMirOpPhi: {
- RegLocation rl_dest = cu->reg_location[mir->ssa_rep->defs[0]];
- /*
- * The Art compiler's Phi nodes only handle 32-bit operands,
- * representing wide values using a matched set of Phi nodes
- * for the lower and upper halves. In the llvm world, we only
- * want a single Phi for wides. Here we will simply discard
- * the Phi node representing the high word.
- */
- if (rl_dest.high_word) {
- return; // No Phi node - handled via low word
- }
- // LLVM requires that all Phi nodes are at the beginning of the block
- llvm::IRBuilderBase::InsertPoint ip = cu->irb->saveAndClearIP();
- cu->irb->SetInsertPoint(llvm_bb);
- int* incoming = reinterpret_cast<int*>(mir->dalvikInsn.vB);
- llvm::Type* phi_type =
- LlvmTypeFromLocRec(cu, rl_dest);
- llvm::PHINode* phi = cu->irb->CreatePHI(phi_type, mir->ssa_rep->num_uses);
- for (int i = 0; i < mir->ssa_rep->num_uses; i++) {
- RegLocation loc;
- // Don't check width here.
- loc = GetRawSrc(cu, mir, i);
- DCHECK_EQ(rl_dest.wide, loc.wide);
- DCHECK_EQ(rl_dest.wide & rl_dest.high_word, loc.wide & loc.high_word);
- DCHECK_EQ(rl_dest.fp, loc.fp);
- DCHECK_EQ(rl_dest.core, loc.core);
- DCHECK_EQ(rl_dest.ref, loc.ref);
- SafeMap<unsigned int, unsigned int>::iterator it;
- it = cu->block_id_map.find(incoming[i]);
- DCHECK(it != cu->block_id_map.end());
- phi->addIncoming(GetLLVMValue(cu, loc.orig_sreg),
- GetLLVMBlock(cu, it->second));
- }
- // Now that Phi node is emitted, add definition at old insert point
- cu->irb->restoreIP(ip);
- DefineValue(cu, phi, rl_dest.orig_sreg);
- break;
- }
- case kMirOpCopy: {
- UNIMPLEMENTED(WARNING) << "unimp kMirOpPhi";
- break;
- }
- case kMirOpNop:
- if ((mir == bb->last_mir_insn) && (bb->taken == NULL) &&
- (bb->fall_through == NULL)) {
- cu->irb->CreateUnreachable();
- }
- break;
-
- // TODO: need GBC intrinsic to take advantage of fused operations
- case kMirOpFusedCmplFloat:
- UNIMPLEMENTED(FATAL) << "kMirOpFusedCmpFloat unsupported";
- break;
- case kMirOpFusedCmpgFloat:
- UNIMPLEMENTED(FATAL) << "kMirOpFusedCmgFloat unsupported";
- break;
- case kMirOpFusedCmplDouble:
- UNIMPLEMENTED(FATAL) << "kMirOpFusedCmplDouble unsupported";
- break;
- case kMirOpFusedCmpgDouble:
- UNIMPLEMENTED(FATAL) << "kMirOpFusedCmpgDouble unsupported";
- break;
- case kMirOpFusedCmpLong:
- UNIMPLEMENTED(FATAL) << "kMirOpLongCmpBranch unsupported";
- break;
- default:
- break;
- }
-}
-
static void SetDexOffset(CompilationUnit* cu, int32_t offset)
{
cu->current_dalvik_offset = offset;
@@ -1744,6 +1677,104 @@
SetDexOffset(cu, cu->current_dalvik_offset);
}
+static void HandlePhiNodes(CompilationUnit* cu, BasicBlock* bb, llvm::BasicBlock* llvm_bb)
+{
+ SetDexOffset(cu, bb->start_offset);
+ for (MIR* mir = bb->first_mir_insn; mir != NULL; mir = mir->next) {
+ int opcode = mir->dalvikInsn.opcode;
+ if (opcode < kMirOpFirst) {
+ // Stop after first non-pseudo MIR op.
+ continue;
+ }
+ if (opcode != kMirOpPhi) {
+ // Skip other mir Pseudos.
+ continue;
+ }
+ RegLocation rl_dest = cu->reg_location[mir->ssa_rep->defs[0]];
+ /*
+ * The Art compiler's Phi nodes only handle 32-bit operands,
+ * representing wide values using a matched set of Phi nodes
+ * for the lower and upper halves. In the llvm world, we only
+ * want a single Phi for wides. Here we will simply discard
+ * the Phi node representing the high word.
+ */
+ if (rl_dest.high_word) {
+ continue; // No Phi node - handled via low word
+ }
+ int* incoming = reinterpret_cast<int*>(mir->dalvikInsn.vB);
+ llvm::Type* phi_type =
+ LlvmTypeFromLocRec(cu, rl_dest);
+ llvm::PHINode* phi = cu->irb->CreatePHI(phi_type, mir->ssa_rep->num_uses);
+ for (int i = 0; i < mir->ssa_rep->num_uses; i++) {
+ RegLocation loc;
+ // Don't check width here.
+ loc = GetRawSrc(cu, mir, i);
+ DCHECK_EQ(rl_dest.wide, loc.wide);
+ DCHECK_EQ(rl_dest.wide & rl_dest.high_word, loc.wide & loc.high_word);
+ DCHECK_EQ(rl_dest.fp, loc.fp);
+ DCHECK_EQ(rl_dest.core, loc.core);
+ DCHECK_EQ(rl_dest.ref, loc.ref);
+ SafeMap<unsigned int, unsigned int>::iterator it;
+ it = cu->block_id_map.find(incoming[i]);
+ DCHECK(it != cu->block_id_map.end());
+ DCHECK(GetLLVMValue(cu, loc.orig_sreg) != NULL);
+ DCHECK(GetLLVMBlock(cu, it->second) != NULL);
+ phi->addIncoming(GetLLVMValue(cu, loc.orig_sreg),
+ GetLLVMBlock(cu, it->second));
+ }
+ DefineValueOnly(cu, phi, rl_dest.orig_sreg);
+ }
+}
+
+/* Extended MIR instructions like PHI */
+static void ConvertExtendedMIR(CompilationUnit* cu, BasicBlock* bb, MIR* mir,
+ llvm::BasicBlock* llvm_bb)
+{
+
+ switch (static_cast<ExtendedMIROpcode>(mir->dalvikInsn.opcode)) {
+ case kMirOpPhi: {
+ // The llvm Phi node already emitted - just DefineValue() here.
+ RegLocation rl_dest = cu->reg_location[mir->ssa_rep->defs[0]];
+ if (!rl_dest.high_word) {
+ // Only consider low word of pairs.
+ DCHECK(GetLLVMValue(cu, rl_dest.orig_sreg) != NULL);
+ llvm::Value* phi = GetLLVMValue(cu, rl_dest.orig_sreg);
+ if (1) SetVregOnValue(cu, phi, rl_dest.orig_sreg);
+ }
+ break;
+ }
+ case kMirOpCopy: {
+ UNIMPLEMENTED(WARNING) << "unimp kMirOpPhi";
+ break;
+ }
+ case kMirOpNop:
+ if ((mir == bb->last_mir_insn) && (bb->taken == NULL) &&
+ (bb->fall_through == NULL)) {
+ cu->irb->CreateUnreachable();
+ }
+ break;
+
+ // TODO: need GBC intrinsic to take advantage of fused operations
+ case kMirOpFusedCmplFloat:
+ UNIMPLEMENTED(FATAL) << "kMirOpFusedCmpFloat unsupported";
+ break;
+ case kMirOpFusedCmpgFloat:
+ UNIMPLEMENTED(FATAL) << "kMirOpFusedCmgFloat unsupported";
+ break;
+ case kMirOpFusedCmplDouble:
+ UNIMPLEMENTED(FATAL) << "kMirOpFusedCmplDouble unsupported";
+ break;
+ case kMirOpFusedCmpgDouble:
+ UNIMPLEMENTED(FATAL) << "kMirOpFusedCmpgDouble unsupported";
+ break;
+ case kMirOpFusedCmpLong:
+ UNIMPLEMENTED(FATAL) << "kMirOpLongCmpBranch unsupported";
+ break;
+ default:
+ break;
+ }
+}
+
/* Handle the content in each basic block */
static bool BlockBitcodeConversion(CompilationUnit* cu, BasicBlock* bb)
{
@@ -1811,6 +1842,8 @@
return false;
}
+ HandlePhiNodes(cu, bb, llvm_bb);
+
for (MIR* mir = bb->first_mir_insn; mir != NULL; mir = mir->next) {
SetDexOffset(cu, mir->offset);
diff --git a/src/compiler/frontend.h b/src/compiler/frontend.h
index fb32cdd..9cf3080 100644
--- a/src/compiler/frontend.h
+++ b/src/compiler/frontend.h
@@ -145,6 +145,7 @@
const art::DexFile::CodeItem* code_item,
uint32_t access_flags,
art::InvokeType invoke_type,
+ uint32_t class_dex_idx,
uint32_t method_idx,
jobject class_loader,
const art::DexFile& dex_file);
diff --git a/src/compiler_llvm/compilation_unit.h b/src/compiler_llvm/compilation_unit.h
index 591d4f0..6ebac9a 100644
--- a/src/compiler_llvm/compilation_unit.h
+++ b/src/compiler_llvm/compilation_unit.h
@@ -29,7 +29,7 @@
#include "safe_map.h"
#if defined(ART_USE_PORTABLE_COMPILER)
-# include "compiler/Dalvik.h"
+# include "compiler/compiler_internals.h"
# include "compiler.h"
# include "oat_compilation_unit.h"
#endif
diff --git a/src/compiler_llvm/compiler_llvm.cc b/src/compiler_llvm/compiler_llvm.cc
index 6de10e6..29c381d 100644
--- a/src/compiler_llvm/compiler_llvm.cc
+++ b/src/compiler_llvm/compiler_llvm.cc
@@ -164,6 +164,18 @@
return method_compiler->Compile();
} else {
+
+#if 1
+ /*
+ * FIXME: temporary workaround
+ * Until Portable/llvm is fixed, use Iceland.
+ */
+ UniquePtr<MethodCompiler> method_compiler(
+ new MethodCompiler(cunit.get(), compiler_, oat_compilation_unit));
+
+ return method_compiler->Compile();
+#endif
+
// TODO: consolidate ArtCompileMethods
CompileOneMethod(*compiler_,
kPortable,
@@ -181,9 +193,10 @@
cunit->Materialize();
- Compiler::MethodReference mref(dex_file_, method_idx_);
- return new CompiledMethod(cunit_->GetInstructionSet(),
- cunit_->GetCompiledCode(),
+ Compiler::MethodReference mref(oat_compilation_unit->GetDexFile(),
+ oat_compilation_unit->GetDexMethodIndex());
+ return new CompiledMethod(compiler_->GetInstructionSet(),
+ cunit->GetCompiledCode(),
*verifier::MethodVerifier::GetDexGcMap(mref));
}
#else
diff --git a/src/compiler_llvm/gbc_expander.cc b/src/compiler_llvm/gbc_expander.cc
index 93d7d48..0f71512 100644
--- a/src/compiler_llvm/gbc_expander.cc
+++ b/src/compiler_llvm/gbc_expander.cc
@@ -45,7 +45,7 @@
using art::greenland::IntrinsicHelper;
namespace art {
-extern char remapShorty(char shortyType);
+extern char RemapShorty(char shortyType);
};
namespace {
@@ -2485,7 +2485,7 @@
char ret_shorty = shorty[0];
#if defined(ART_USE_PORTABLE_COMPILER)
- ret_shorty = art::remapShorty(ret_shorty);
+ ret_shorty = art::RemapShorty(ret_shorty);
#endif
llvm::Type* ret_type = irb_.getJType(ret_shorty, kAccurate);
@@ -2500,7 +2500,7 @@
for (uint32_t i = 1; i < shorty_size; ++i) {
#if defined(ART_USE_PORTABLE_COMPILER)
- char shorty_type = art::remapShorty(shorty[i]);
+ char shorty_type = art::RemapShorty(shorty[i]);
args_type.push_back(irb_.getJType(shorty_type, kAccurate));
#else
args_type.push_back(irb_.getJType(shorty[i], kAccurate));
@@ -2635,7 +2635,7 @@
// Emit the code to return default value (zero) for the given return type.
char ret_shorty = oat_compilation_unit_->GetShorty()[0];
#if defined(ART_USE_PORTABLE_COMPILER)
- ret_shorty = art::remapShorty(ret_shorty);
+ ret_shorty = art::RemapShorty(ret_shorty);
#endif
if (ret_shorty == 'V') {
irb_.CreateRetVoid();