Add .bss support for inlining across dexfiles within Oat

We can safely reference other dexfiles within the same oat file for cross-dex inlining.

This CL makes the OptStat#NotInlinedBss drop to less than 1% of the not-inlining cases.

Test: ART tests
Change-Id: I676d48d973abf7a6f8412cf3b7bb73afd7747f31

CleanSpec.mk

@@ -119,6 +119,7 @@ $(call add-clean-step, rm -rf $(HOST_OUT))
 $(call add-clean-step, find $(OUT_DIR) -name "*.oat" -o -name "*.odex" -o -name "*.art" -o -name '*.vdex' | xargs rm -f)
 $(call add-clean-step, find $(OUT_DIR) -name "*.oat" -o -name "*.odex" -o -name "*.art" -o -name '*.vdex' | xargs rm -f)
 $(call add-clean-step, find $(OUT_DIR) -name "*.oat" -o -name "*.odex" -o -name "*.art" -o -name '*.vdex' | xargs rm -f)
+$(call add-clean-step, find $(OUT_DIR) -name "*.oat" -o -name "*.odex" -o -name "*.art" -o -name '*.vdex' | xargs rm -f)
 
 # ************************************************
 # NEWER CLEAN STEPS MUST BE AT THE END OF THE LIST

compiler/driver/compiler_options.h

@@ -373,6 +373,11 @@ class CompilerOptions final {
     return initialize_app_image_classes_;
   }
 
+  bool WithinOatFile(const DexFile* dex_file) const {
+    return std::find(GetDexFilesForOatFile().begin(), GetDexFilesForOatFile().end(), dex_file) !=
+           GetDexFilesForOatFile().end();
+  }
+
  private:
   bool ParseDumpInitFailures(const std::string& option, std::string* error_msg);
   bool ParseRegisterAllocationStrategy(const std::string& option, std::string* error_msg);

compiler/optimizing/code_generator_arm64.cc

@@ -281,7 +281,8 @@ class LoadClassSlowPathARM64 : public SlowPathCodeARM64 {
 
     InvokeRuntimeCallingConvention calling_convention;
     if (must_resolve_type) {
-      DCHECK(IsSameDexFile(cls_->GetDexFile(), arm64_codegen->GetGraph()->GetDexFile()));
+      DCHECK(IsSameDexFile(cls_->GetDexFile(), arm64_codegen->GetGraph()->GetDexFile()) ||
+             arm64_codegen->GetCompilerOptions().WithinOatFile(&cls_->GetDexFile()));
       dex::TypeIndex type_index = cls_->GetTypeIndex();
       __ Mov(calling_convention.GetRegisterAt(0).W(), type_index.index_);
       if (cls_->NeedsAccessCheck()) {

compiler/optimizing/code_generator_arm_vixl.cc

@@ -529,7 +529,8 @@ class LoadClassSlowPathARMVIXL : public SlowPathCodeARMVIXL {
 
     InvokeRuntimeCallingConventionARMVIXL calling_convention;
     if (must_resolve_type) {
-      DCHECK(IsSameDexFile(cls_->GetDexFile(), arm_codegen->GetGraph()->GetDexFile()));
+      DCHECK(IsSameDexFile(cls_->GetDexFile(), arm_codegen->GetGraph()->GetDexFile()) ||
+             arm_codegen->GetCompilerOptions().WithinOatFile(&cls_->GetDexFile()));
       dex::TypeIndex type_index = cls_->GetTypeIndex();
       __ Mov(calling_convention.GetRegisterAt(0), type_index.index_);
       if (cls_->NeedsAccessCheck()) {

compiler/optimizing/code_generator_x86.cc

@@ -288,7 +288,8 @@ class LoadClassSlowPathX86 : public SlowPathCode {
 
     InvokeRuntimeCallingConvention calling_convention;
     if (must_resolve_type) {
-      DCHECK(IsSameDexFile(cls_->GetDexFile(), x86_codegen->GetGraph()->GetDexFile()));
+      DCHECK(IsSameDexFile(cls_->GetDexFile(), x86_codegen->GetGraph()->GetDexFile()) ||
+             x86_codegen->GetCompilerOptions().WithinOatFile(&cls_->GetDexFile()));
       dex::TypeIndex type_index = cls_->GetTypeIndex();
       __ movl(calling_convention.GetRegisterAt(0), Immediate(type_index.index_));
       if (cls_->NeedsAccessCheck()) {
@@ -5497,7 +5498,8 @@ void CodeGeneratorX86::RecordMethodBssEntryPatch(HInvoke* invoke) {
   size_t index = invoke->IsInvokeInterface()
       ? invoke->AsInvokeInterface()->GetSpecialInputIndex()
       : invoke->AsInvokeStaticOrDirect()->GetSpecialInputIndex();
-  DCHECK(IsSameDexFile(GetGraph()->GetDexFile(), *invoke->GetMethodReference().dex_file));
+  DCHECK(IsSameDexFile(GetGraph()->GetDexFile(), *invoke->GetMethodReference().dex_file) ||
+         GetCompilerOptions().WithinOatFile(invoke->GetMethodReference().dex_file));
   HX86ComputeBaseMethodAddress* method_address =
       invoke->InputAt(index)->AsX86ComputeBaseMethodAddress();
   // Add the patch entry and bind its label at the end of the instruction.

compiler/optimizing/code_generator_x86_64.cc

@@ -272,7 +272,8 @@ class LoadClassSlowPathX86_64 : public SlowPathCode {
 
     // Custom calling convention: RAX serves as both input and output.
     if (must_resolve_type) {
-      DCHECK(IsSameDexFile(cls_->GetDexFile(), x86_64_codegen->GetGraph()->GetDexFile()));
+      DCHECK(IsSameDexFile(cls_->GetDexFile(), x86_64_codegen->GetGraph()->GetDexFile()) ||
+             x86_64_codegen->GetCompilerOptions().WithinOatFile(&cls_->GetDexFile()));
       dex::TypeIndex type_index = cls_->GetTypeIndex();
       __ movl(CpuRegister(RAX), Immediate(type_index.index_));
       if (cls_->NeedsAccessCheck()) {
@@ -1228,7 +1229,8 @@ void CodeGeneratorX86_64::RecordBootImageMethodPatch(HInvoke* invoke) {
 }
 
 void CodeGeneratorX86_64::RecordMethodBssEntryPatch(HInvoke* invoke) {
-  DCHECK(IsSameDexFile(GetGraph()->GetDexFile(), *invoke->GetMethodReference().dex_file));
+  DCHECK(IsSameDexFile(GetGraph()->GetDexFile(), *invoke->GetMethodReference().dex_file) ||
+         GetCompilerOptions().WithinOatFile(invoke->GetMethodReference().dex_file));
   method_bss_entry_patches_.emplace_back(invoke->GetMethodReference().dex_file,
                                          invoke->GetMethodReference().index);
   __ Bind(&method_bss_entry_patches_.back().label);

compiler/optimizing/inliner.cc

@@ -1710,6 +1710,7 @@ static bool CanEncodeInlinedMethodInStackMap(const DexFile& outer_dex_file,
     // JIT can always encode methods in stack maps.
     return true;
   }
+
   const DexFile* dex_file = callee->GetDexFile();
   if (IsSameDexFile(outer_dex_file, *dex_file)) {
     return true;
@@ -1718,15 +1719,16 @@ static bool CanEncodeInlinedMethodInStackMap(const DexFile& outer_dex_file,
   // Inline across dexfiles if the callee's DexFile is:
   // 1) in the bootclasspath, or
   if (callee->GetDeclaringClass()->GetClassLoader() == nullptr) {
+    // There are cases in which the BCP DexFiles are within the OatFile as far as the compiler
+    // options are concerned, but they have their own OatWriter (and therefore not in the same
+    // OatFile). Then, we request the BSS check for all BCP DexFiles.
+    // TODO(solanes): Add .bss support for BCP.
     *out_needs_bss_check = true;
     return true;
   }
 
-  // 2) is a non-BCP dexfile with an OatDexFile.
-  const std::vector<const DexFile*>& dex_files =
-      codegen->GetCompilerOptions().GetDexFilesForOatFile();
-  if (std::find(dex_files.begin(), dex_files.end(), dex_file) != dex_files.end()) {
-    *out_needs_bss_check = true;
+  // 2) is a non-BCP dexfile with the OatFile we are compiling.
+  if (codegen->GetCompilerOptions().WithinOatFile(dex_file)) {
     return true;
   }
 

dex2oat/linker/oat_writer.cc

@@ -944,8 +944,9 @@ class OatWriter::InitBssLayoutMethodVisitor : public DexMethodVisitor {
                        size_t number_of_indexes,
                        /*inout*/ SafeMap<const DexFile*, BitVector>* references) {
     // We currently support inlining of throwing instructions only when they originate in the
-    // same dex file as the outer method. All .bss references are used by throwing instructions.
-    DCHECK_EQ(dex_file_, ref.dex_file);
+    // same oat file as the outer method. All .bss references are used by throwing instructions.
+    DCHECK(std::find(writer_->dex_files_->begin(), writer_->dex_files_->end(), ref.dex_file) !=
+           writer_->dex_files_->end());
     DCHECK_LT(ref.index, number_of_indexes);
 
     auto refs_it = references->find(ref.dex_file);

runtime/oat.h

@@ -32,8 +32,8 @@ class InstructionSetFeatures;
 class PACKED(4) OatHeader {
  public:
   static constexpr std::array<uint8_t, 4> kOatMagic { { 'o', 'a', 't', '\n' } };
-  // Last oat version changed reason: Inlining across dex files for non-BCP methods.
-  static constexpr std::array<uint8_t, 4> kOatVersion { { '2', '1', '0', '\0' } };
+  // Last oat version changed reason: Inlining across dex files for bss within OAT.
+  static constexpr std::array<uint8_t, 4> kOatVersion { { '2', '1', '1', '\0' } };
 
   static constexpr const char* kDex2OatCmdLineKey = "dex2oat-cmdline";
   static constexpr const char* kDebuggableKey = "debuggable";

test/2237-checker-inline-multidex/expected-stdout.txt

@@ -0,0 +1,4 @@
+abc
+def
+ghi
+class Multi$Multi2

test/2237-checker-inline-multidex/src-multidex/Multi.java

@@ -15,9 +15,27 @@
  */
 
 public class Multi {
-  public static String $inline$doThings(String str) {
+  public static String $inline$NeedsEnvironmentMultiDex(String str) {
     // StringBuilderAppend needs an environment but it doesn't need a .bss entry.
     StringBuilder sb = new StringBuilder();
     return sb.append(str).toString();
   }
+
+  public static String $inline$NeedsBssEntryStringMultiDex() {
+    return "def";
+  }
+
+  private static String $noinline$InnerInvokeMultiDex() {
+    return "ghi";
+  }
+
+  public static String $inline$NeedsBssEntryInvokeMultiDex() {
+    return $noinline$InnerInvokeMultiDex();
+  }
+
+  public static Class<?> NeedsBssEntryClassMultiDex() {
+    return Multi2.class;
+  }
+
+  private class Multi2 {}
 }

test/2237-checker-inline-multidex/src/Main.java

@@ -15,15 +15,58 @@
  */
 
 public class Main {
-  /// CHECK-START: void Main.main(java.lang.String[]) inliner (before)
-  /// CHECK:       InvokeStaticOrDirect method_name:Multi.$inline$doThings
+  public static void main(String[] args) {
+    System.out.println(testNeedsEnvironment());
+    System.out.println(testNeedsBssEntryString());
+    System.out.println(testNeedsBssEntryInvoke());
+    System.out.println(testClass());
+  }
 
-  /// CHECK-START: void Main.main(java.lang.String[]) inliner (after)
-  /// CHECK-NOT:   InvokeStaticOrDirect method_name:Multi.$inline$doThings
+  /// CHECK-START: java.lang.String Main.testNeedsEnvironment() inliner (before)
+  /// CHECK:       InvokeStaticOrDirect method_name:Multi.$inline$NeedsEnvironmentMultiDex
 
-  /// CHECK-START: void Main.main(java.lang.String[]) inliner (after)
+  /// CHECK-START: java.lang.String Main.testNeedsEnvironment() inliner (after)
+  /// CHECK-NOT:   InvokeStaticOrDirect method_name:Multi.$inline$NeedsEnvironmentMultiDex
+
+  /// CHECK-START: java.lang.String Main.testNeedsEnvironment() inliner (after)
   /// CHECK:       StringBuilderAppend
-  public static void main(String[] args) {
-    Multi.$inline$doThings("abc");
+  public static String testNeedsEnvironment() {
+    return Multi.$inline$NeedsEnvironmentMultiDex("abc");
+  }
+
+  /// CHECK-START: java.lang.String Main.testNeedsBssEntryString() inliner (before)
+  /// CHECK:       InvokeStaticOrDirect method_name:Multi.$inline$NeedsBssEntryStringMultiDex
+
+  /// CHECK-START: java.lang.String Main.testNeedsBssEntryString() inliner (after)
+  /// CHECK-NOT:   InvokeStaticOrDirect method_name:Multi.$inline$NeedsBssEntryStringMultiDex
+
+  /// CHECK-START: java.lang.String Main.testNeedsBssEntryString() inliner (after)
+  /// CHECK:       LoadString load_kind:BssEntry
+  public static String testNeedsBssEntryString() {
+    return Multi.$inline$NeedsBssEntryStringMultiDex();
+  }
+
+  /// CHECK-START: java.lang.String Main.testNeedsBssEntryInvoke() inliner (before)
+  /// CHECK:       InvokeStaticOrDirect method_name:Multi.$inline$NeedsBssEntryInvokeMultiDex
+
+  /// CHECK-START: java.lang.String Main.testNeedsBssEntryInvoke() inliner (after)
+  /// CHECK-NOT:   InvokeStaticOrDirect method_name:Multi.$inline$NeedsBssEntryInvokeMultiDex
+
+  /// CHECK-START: java.lang.String Main.testNeedsBssEntryInvoke() inliner (after)
+  /// CHECK:       InvokeStaticOrDirect method_name:Multi.$noinline$InnerInvokeMultiDex method_load_kind:BssEntry
+  public static String testNeedsBssEntryInvoke() {
+    return Multi.$inline$NeedsBssEntryInvokeMultiDex();
+  }
+
+  /// CHECK-START: java.lang.Class Main.testClass() inliner (before)
+  /// CHECK:       InvokeStaticOrDirect method_name:Multi.NeedsBssEntryClassMultiDex
+
+  /// CHECK-START: java.lang.Class Main.testClass() inliner (after)
+  /// CHECK-NOT:   InvokeStaticOrDirect method_name:Multi.NeedsBssEntryClassMultiDex
+
+  /// CHECK-START: java.lang.Class Main.testClass() inliner (after)
+  /// CHECK:       LoadClass load_kind:BssEntry class_name:Multi$Multi2
+  public static Class<?> testClass() {
+    return Multi.NeedsBssEntryClassMultiDex();
   }
 }

test/462-checker-inlining-dex-files/src/Main.java

@@ -47,15 +47,14 @@ public static int inlineReturnIntMethod() {
     return OtherDex.returnIntMethod();
   }
 
-  /// CHECK-START: int Main.dontInlineOtherDexStatic() inliner (before)
-  /// CHECK-DAG:     <<Invoke:i\d+>>  InvokeStaticOrDirect
+  /// CHECK-START: int Main.inlineOtherDexStatic() inliner (before)
+  /// CHECK-DAG:     <<Invoke:i\d+>>  InvokeStaticOrDirect method_name:OtherDex.returnOtherDexStatic
   /// CHECK-DAG:                      Return [<<Invoke>>]
 
-  /// CHECK-START: int Main.dontInlineOtherDexStatic() inliner (after)
-  /// CHECK-DAG:     <<Invoke:i\d+>>  InvokeStaticOrDirect
-  /// CHECK-DAG:                      Return [<<Invoke>>]
+  /// CHECK-START: int Main.inlineOtherDexStatic() inliner (after)
+  /// CHECK-NOT:                      InvokeStaticOrDirect method_name:OtherDex.returnOtherDexStatic
 
-  public static int dontInlineOtherDexStatic() {
+  public static int inlineOtherDexStatic() {
     return OtherDex.returnOtherDexStatic();
   }
 
@@ -75,38 +74,36 @@ public static int inlineMainStatic() {
   }
 
   /// CHECK-START: int Main.dontInlineRecursiveCall() inliner (before)
-  /// CHECK-DAG:     <<Invoke:i\d+>>  InvokeStaticOrDirect
+  /// CHECK-DAG:     <<Invoke:i\d+>>  InvokeStaticOrDirect method_name:OtherDex.recursiveCall
   /// CHECK-DAG:                      Return [<<Invoke>>]
 
   /// CHECK-START: int Main.dontInlineRecursiveCall() inliner (after)
-  /// CHECK-DAG:     <<Invoke:i\d+>>  InvokeStaticOrDirect
+  /// CHECK-DAG:     <<Invoke:i\d+>>  InvokeStaticOrDirect method_name:OtherDex.recursiveCall
   /// CHECK-DAG:                      Return [<<Invoke>>]
 
   public static int dontInlineRecursiveCall() {
     return OtherDex.recursiveCall();
   }
 
-  /// CHECK-START: java.lang.String Main.dontInlineReturnString() inliner (before)
-  /// CHECK-DAG:     <<Invoke:l\d+>>  InvokeStaticOrDirect
+  /// CHECK-START: java.lang.String Main.inlineReturnString() inliner (before)
+  /// CHECK-DAG:     <<Invoke:l\d+>>  InvokeStaticOrDirect method_name:OtherDex.returnString
   /// CHECK-DAG:                      Return [<<Invoke>>]
 
-  /// CHECK-START: java.lang.String Main.dontInlineReturnString() inliner (after)
-  /// CHECK-DAG:     <<Invoke:l\d+>>  InvokeStaticOrDirect
-  /// CHECK-DAG:                      Return [<<Invoke>>]
+  /// CHECK-START: java.lang.String Main.inlineReturnString() inliner (after)
+  /// CHECK-NOT:                      InvokeStaticOrDirect method_name:OtherDex.returnString
 
-  public static String dontInlineReturnString() {
+  public static String inlineReturnString() {
     return OtherDex.returnString();
   }
 
-  /// CHECK-START: java.lang.Class Main.dontInlineOtherDexClass() inliner (before)
-  /// CHECK-DAG:     <<Invoke:l\d+>>  InvokeStaticOrDirect
+  /// CHECK-START: java.lang.Class Main.inlineOtherDexClass() inliner (before)
+  /// CHECK-DAG:     <<Invoke:l\d+>>  InvokeStaticOrDirect method_name:OtherDex.returnOtherDexClass
   /// CHECK-DAG:                      Return [<<Invoke>>]
 
-  /// CHECK-START: java.lang.Class Main.dontInlineOtherDexClass() inliner (after)
-  /// CHECK-DAG:     <<Invoke:l\d+>>  InvokeStaticOrDirect
-  /// CHECK-DAG:                      Return [<<Invoke>>]
+  /// CHECK-START: java.lang.Class Main.inlineOtherDexClass() inliner (after)
+  /// CHECK-NOT:                      InvokeStaticOrDirect method_name:OtherDex.returnOtherDexClass
 
-  public static Class<?> dontInlineOtherDexClass() {
+  public static Class<?> inlineOtherDexClass() {
     return OtherDex.returnOtherDexClass();
   }
 
@@ -127,15 +124,14 @@ public static Class<?> inlineMainClass() {
     return OtherDex.returnMainClass();
   }
 
-  /// CHECK-START: java.lang.Class Main.dontInlineOtherDexClassStaticCall() inliner (before)
-  /// CHECK-DAG:     <<Invoke:l\d+>>  InvokeStaticOrDirect
+  /// CHECK-START: java.lang.Class Main.inlineOtherDexClassStaticCall() inliner (before)
+  /// CHECK-DAG:     <<Invoke:l\d+>>  InvokeStaticOrDirect method_name:OtherDex.returnOtherDexClassStaticCall
   /// CHECK-DAG:                      Return [<<Invoke>>]
 
-  /// CHECK-START: java.lang.Class Main.dontInlineOtherDexClassStaticCall() inliner (after)
-  /// CHECK-DAG:     <<Invoke:l\d+>>  InvokeStaticOrDirect
-  /// CHECK-DAG:                      Return [<<Invoke>>]
+  /// CHECK-START: java.lang.Class Main.inlineOtherDexClassStaticCall() inliner (after)
+  /// CHECK-NOT:                      InvokeStaticOrDirect method_name:OtherDex.returnOtherDexClassStaticCall
 
-  public static Class<?> dontInlineOtherDexClassStaticCall() {
+  public static Class<?> inlineOtherDexClassStaticCall() {
     return OtherDex.returnOtherDexClassStaticCall();
   }
 
@@ -166,23 +162,23 @@ public static void main(String[] args) {
       throw new Error("Expected 38");
     }
 
-    if (dontInlineOtherDexStatic() != 1) {
+    if (inlineOtherDexStatic() != 1) {
       throw new Error("Expected 1");
     }
 
     if (inlineMainStatic() != 42) {
       throw new Error("Expected 42");
     }
 
-    if (dontInlineReturnString() != "OtherDex") {
+    if (inlineReturnString() != "OtherDex") {
       throw new Error("Expected OtherDex");
     }
 
-    if (dontInlineOtherDexClass() != OtherDex.class) {
+    if (inlineOtherDexClass() != OtherDex.class) {
       throw new Error("Expected " + OtherDex.class);
     }
 
-    if (dontInlineOtherDexClassStaticCall() != OtherDex.class) {
+    if (inlineOtherDexClassStaticCall() != OtherDex.class) {
       throw new Error("Expected " + OtherDex.class);
     }