Google Git
Sign in
android/platform/art/8891ea4eac^!/.
commit
8891ea4eac12b8f48934d47f2d99da1e9b455788
[log]
author
Nicolas Geoffray <ngeoffray@google.com>
Fri Dec 01 14:47:37 2023 +0000
committer
Nicolas Geoffray <ngeoffray@google.com>
Mon Dec 04 10:35:16 2023 +0000
tree
00b523efef48ab608779156af751e1bf38c70091
parent
f51e433b5d32cb83eb2ee302544ecf67a10525b9 [diff]
Put branch profiling under a flag.

Until we evaluate its usefulness and reduce its overhead.

Bug: 306638020

Test: test.py
Change-Id: Ibb01c70a7ea19b03802dcc1b0792d3d2ff4f4d67

diff --git a/compiler/driver/compiler_options.cc b/compiler/driver/compiler_options.cc
index 45664b7..ada7685 100644
--- a/compiler/driver/compiler_options.cc
+++ b/compiler/driver/compiler_options.cc

@@ -64,6 +64,7 @@
       dump_timings_(false),
       dump_pass_timings_(false),
       dump_stats_(false),
+      profile_branches_(false),
       profile_compilation_info_(nullptr),
       verbose_methods_(),
       abort_on_hard_verifier_failure_(false),

diff --git a/compiler/driver/compiler_options.h b/compiler/driver/compiler_options.h
index f816c14..3064729 100644
--- a/compiler/driver/compiler_options.h
+++ b/compiler/driver/compiler_options.h

@@ -220,6 +220,10 @@
     return baseline_;
   }
 
+  bool ProfileBranches() const {
+    return profile_branches_;
+  }
+
   // Are we compiling an app image?
   bool IsAppImage() const {
     return image_type_ == ImageType::kAppImage;
@@ -427,6 +431,7 @@
   bool dump_timings_;
   bool dump_pass_timings_;
   bool dump_stats_;
+  bool profile_branches_;
 
   // Info for profile guided compilation.
   const ProfileCompilationInfo* profile_compilation_info_;

diff --git a/compiler/driver/compiler_options_map-inl.h b/compiler/driver/compiler_options_map-inl.h
index 4734950..9a02b57 100644
--- a/compiler/driver/compiler_options_map-inl.h
+++ b/compiler/driver/compiler_options_map-inl.h

@@ -68,6 +68,9 @@
   if (map.Exists(Base::Baseline)) {
     options->baseline_ = true;
   }
+  if (map.Exists(Base::ProfileBranches)) {
+    options->profile_branches_ = true;
+  }
   map.AssignIfExists(Base::AbortOnHardVerifierFailure, &options->abort_on_hard_verifier_failure_);
   map.AssignIfExists(Base::AbortOnSoftVerifierFailure, &options->abort_on_soft_verifier_failure_);
   if (map.Exists(Base::DumpInitFailures)) {
@@ -198,6 +201,10 @@
           .WithHelp("Produce code using the baseline compilation")
           .IntoKey(Map::Baseline)
 
+      .Define("--profile-branches")
+          .WithHelp("Profile branches in baseline generated code")
+          .IntoKey(Map::ProfileBranches)
+
       .Define({"--abort-on-hard-verifier-error", "--no-abort-on-hard-verifier-error"})
           .WithValues({true, false})
           .IntoKey(Map::AbortOnHardVerifierFailure)

diff --git a/compiler/driver/compiler_options_map.def b/compiler/driver/compiler_options_map.def
index c6666a8..133ad2b 100644
--- a/compiler/driver/compiler_options_map.def
+++ b/compiler/driver/compiler_options_map.def

@@ -47,6 +47,7 @@
 COMPILER_OPTIONS_KEY (bool,                        GenerateBuildID)
 COMPILER_OPTIONS_KEY (Unit,                        Debuggable)
 COMPILER_OPTIONS_KEY (Unit,                        Baseline)
+COMPILER_OPTIONS_KEY (Unit,                        ProfileBranches)
 COMPILER_OPTIONS_KEY (bool,                        AbortOnHardVerifierFailure)
 COMPILER_OPTIONS_KEY (bool,                        AbortOnSoftVerifierFailure)
 COMPILER_OPTIONS_KEY (bool,                        ResolveStartupConstStrings, false)

diff --git a/compiler/optimizing/code_generator_arm64.cc b/compiler/optimizing/code_generator_arm64.cc
index d855d5e..bd1f71f 100644
--- a/compiler/optimizing/code_generator_arm64.cc
+++ b/compiler/optimizing/code_generator_arm64.cc

@@ -3866,7 +3866,9 @@
     false_target = nullptr;
   }
   if (IsBooleanValueOrMaterializedCondition(if_instr->InputAt(0))) {
-    if (GetGraph()->IsCompilingBaseline() && !Runtime::Current()->IsAotCompiler()) {
+    if (GetGraph()->IsCompilingBaseline() &&
+        codegen_->GetCompilerOptions().ProfileBranches() &&
+        !Runtime::Current()->IsAotCompiler()) {
       DCHECK(if_instr->InputAt(0)->IsCondition());
       ProfilingInfo* info = GetGraph()->GetProfilingInfo();
       DCHECK(info != nullptr);
@@ -3891,8 +3893,6 @@
         __ Bind(&done);
       }
     }
-  } else {
-    DCHECK(!GetGraph()->IsCompilingBaseline()) << if_instr->InputAt(0)->DebugName();
   }
   GenerateTestAndBranch(if_instr, /* condition_input_index= */ 0, true_target, false_target);
 }

diff --git a/compiler/optimizing/code_generator_arm_vixl.cc b/compiler/optimizing/code_generator_arm_vixl.cc
index f98f4d7..25331df 100644
--- a/compiler/optimizing/code_generator_arm_vixl.cc
+++ b/compiler/optimizing/code_generator_arm_vixl.cc

@@ -2990,7 +2990,9 @@
   LocationSummary* locations = new (GetGraph()->GetAllocator()) LocationSummary(if_instr);
   if (IsBooleanValueOrMaterializedCondition(if_instr->InputAt(0))) {
     locations->SetInAt(0, Location::RequiresRegister());
-    if (GetGraph()->IsCompilingBaseline() && !Runtime::Current()->IsAotCompiler()) {
+    if (GetGraph()->IsCompilingBaseline() &&
+        codegen_->GetCompilerOptions().ProfileBranches() &&
+        !Runtime::Current()->IsAotCompiler()) {
       locations->AddTemp(Location::RequiresRegister());
     }
   }
@@ -3004,7 +3006,9 @@
   vixl32::Label* false_target = codegen_->GoesToNextBlock(if_instr->GetBlock(), false_successor) ?
       nullptr : codegen_->GetLabelOf(false_successor);
   if (IsBooleanValueOrMaterializedCondition(if_instr->InputAt(0))) {
-    if (GetGraph()->IsCompilingBaseline() && !Runtime::Current()->IsAotCompiler()) {
+    if (GetGraph()->IsCompilingBaseline() &&
+        codegen_->GetCompilerOptions().ProfileBranches() &&
+        !Runtime::Current()->IsAotCompiler()) {
       DCHECK(if_instr->InputAt(0)->IsCondition());
       ProfilingInfo* info = GetGraph()->GetProfilingInfo();
       DCHECK(info != nullptr);
@@ -3030,8 +3034,6 @@
         __ Bind(&done);
       }
     }
-  } else {
-    DCHECK(!GetGraph()->IsCompilingBaseline()) << if_instr->InputAt(0)->DebugName();
   }
   GenerateTestAndBranch(if_instr, /* condition_input_index= */ 0, true_target, false_target);
 }

diff --git a/compiler/optimizing/code_generator_riscv64.cc b/compiler/optimizing/code_generator_riscv64.cc
index 7331ad6..92ed3f1 100644
--- a/compiler/optimizing/code_generator_riscv64.cc
+++ b/compiler/optimizing/code_generator_riscv64.cc

@@ -3698,7 +3698,9 @@
   LocationSummary* locations = new (GetGraph()->GetAllocator()) LocationSummary(instruction);
   if (IsBooleanValueOrMaterializedCondition(instruction->InputAt(0))) {
     locations->SetInAt(0, Location::RequiresRegister());
-    if (GetGraph()->IsCompilingBaseline() && !Runtime::Current()->IsAotCompiler()) {
+    if (GetGraph()->IsCompilingBaseline() &&
+        codegen_->GetCompilerOptions().ProfileBranches() &&
+        !Runtime::Current()->IsAotCompiler()) {
       DCHECK(instruction->InputAt(0)->IsCondition());
       ProfilingInfo* info = GetGraph()->GetProfilingInfo();
       DCHECK(info != nullptr);
@@ -3720,7 +3722,9 @@
       ? nullptr
       : codegen_->GetLabelOf(false_successor);
   if (IsBooleanValueOrMaterializedCondition(instruction->InputAt(0))) {
-    if (GetGraph()->IsCompilingBaseline() && !Runtime::Current()->IsAotCompiler()) {
+    if (GetGraph()->IsCompilingBaseline() &&
+        codegen_->GetCompilerOptions().ProfileBranches() &&
+        !Runtime::Current()->IsAotCompiler()) {
       DCHECK(instruction->InputAt(0)->IsCondition());
       ProfilingInfo* info = GetGraph()->GetProfilingInfo();
       DCHECK(info != nullptr);
@@ -3751,8 +3755,6 @@
         __ Bind(&done);
       }
     }
-  } else {
-    DCHECK(!GetGraph()->IsCompilingBaseline()) << instruction->InputAt(0)->DebugName();
   }
   GenerateTestAndBranch(instruction, /* condition_input_index= */ 0, true_target, false_target);
 }

diff --git a/compiler/optimizing/code_generator_x86.cc b/compiler/optimizing/code_generator_x86.cc
index c353087..461d14a 100644
--- a/compiler/optimizing/code_generator_x86.cc
+++ b/compiler/optimizing/code_generator_x86.cc

@@ -2109,7 +2109,9 @@
   }
 }
 
-static bool AreEflagsSetFrom(HInstruction* cond, HInstruction* branch) {
+static bool AreEflagsSetFrom(HInstruction* cond,
+                             HInstruction* branch,
+                             const CompilerOptions& compiler_options) {
   // Moves may affect the eflags register (move zero uses xorl), so the EFLAGS
   // are set only strictly before `branch`. We can't use the eflags on long/FP
   // conditions if they are materialized due to the complex branching.
@@ -2117,7 +2119,8 @@
          cond->GetNext() == branch &&
          cond->InputAt(0)->GetType() != DataType::Type::kInt64 &&
          !DataType::IsFloatingPointType(cond->InputAt(0)->GetType()) &&
-         !cond->GetBlock()->GetGraph()->IsCompilingBaseline();
+         !(cond->GetBlock()->GetGraph()->IsCompilingBaseline() &&
+           compiler_options.ProfileBranches());
 }
 
 template<class LabelType>
@@ -2154,7 +2157,7 @@
   //        - condition true => branch to true_target
   //        - branch to false_target
   if (IsBooleanValueOrMaterializedCondition(cond)) {
-    if (AreEflagsSetFrom(cond, instruction)) {
+    if (AreEflagsSetFrom(cond, instruction, codegen_->GetCompilerOptions())) {
       if (true_target == nullptr) {
         __ j(X86Condition(cond->AsCondition()->GetOppositeCondition()), false_target);
       } else {
@@ -2208,7 +2211,9 @@
 void LocationsBuilderX86::VisitIf(HIf* if_instr) {
   LocationSummary* locations = new (GetGraph()->GetAllocator()) LocationSummary(if_instr);
   if (IsBooleanValueOrMaterializedCondition(if_instr->InputAt(0))) {
-    if (GetGraph()->IsCompilingBaseline() && !Runtime::Current()->IsAotCompiler()) {
+    if (GetGraph()->IsCompilingBaseline() &&
+        codegen_->GetCompilerOptions().ProfileBranches() &&
+        !Runtime::Current()->IsAotCompiler()) {
       locations->SetInAt(0, Location::RequiresRegister());
       locations->AddTemp(Location::RequiresRegister());
       locations->AddTemp(Location::RequiresRegister());
@@ -2226,7 +2231,9 @@
   Label* false_target = codegen_->GoesToNextBlock(if_instr->GetBlock(), false_successor) ?
       nullptr : codegen_->GetLabelOf(false_successor);
   if (IsBooleanValueOrMaterializedCondition(if_instr->InputAt(0))) {
-    if (GetGraph()->IsCompilingBaseline() && !Runtime::Current()->IsAotCompiler()) {
+    if (GetGraph()->IsCompilingBaseline() &&
+        codegen_->GetCompilerOptions().ProfileBranches() &&
+        !Runtime::Current()->IsAotCompiler()) {
       DCHECK(if_instr->InputAt(0)->IsCondition());
       Register temp = if_instr->GetLocations()->GetTemp(0).AsRegister<Register>();
       Register counter = if_instr->GetLocations()->GetTemp(1).AsRegister<Register>();
@@ -2250,8 +2257,6 @@
         __ Bind(&done);
       }
     }
-  } else {
-    DCHECK(!GetGraph()->IsCompilingBaseline()) << if_instr->InputAt(0)->DebugName();
   }
   GenerateTestAndBranch(if_instr, /* condition_input_index= */ 0, true_target, false_target);
 }
@@ -2348,7 +2353,7 @@
       if (!condition->IsEmittedAtUseSite()) {
         // This was a previously materialized condition.
         // Can we use the existing condition code?
-        if (AreEflagsSetFrom(condition, select)) {
+        if (AreEflagsSetFrom(condition, select, codegen_->GetCompilerOptions())) {
           // Materialization was the previous instruction. Condition codes are right.
           cond = X86Condition(condition->GetCondition());
         } else {

diff --git a/compiler/optimizing/code_generator_x86_64.cc b/compiler/optimizing/code_generator_x86_64.cc
index d72a436..46c52a2 100644
--- a/compiler/optimizing/code_generator_x86_64.cc
+++ b/compiler/optimizing/code_generator_x86_64.cc

@@ -2173,14 +2173,17 @@
   }
 }
 
-static bool AreEflagsSetFrom(HInstruction* cond, HInstruction* branch) {
+static bool AreEflagsSetFrom(HInstruction* cond,
+                             HInstruction* branch,
+                             const CompilerOptions& compiler_options) {
   // Moves may affect the eflags register (move zero uses xorl), so the EFLAGS
   // are set only strictly before `branch`. We can't use the eflags on long
   // conditions if they are materialized due to the complex branching.
   return cond->IsCondition() &&
          cond->GetNext() == branch &&
          !DataType::IsFloatingPointType(cond->InputAt(0)->GetType()) &&
-         !cond->GetBlock()->GetGraph()->IsCompilingBaseline();
+         !(cond->GetBlock()->GetGraph()->IsCompilingBaseline() &&
+           compiler_options.ProfileBranches());
 }
 
 template<class LabelType>
@@ -2217,7 +2220,7 @@
   //        - condition true => branch to true_target
   //        - branch to false_target
   if (IsBooleanValueOrMaterializedCondition(cond)) {
-    if (AreEflagsSetFrom(cond, instruction)) {
+    if (AreEflagsSetFrom(cond, instruction, codegen_->GetCompilerOptions())) {
       if (true_target == nullptr) {
         __ j(X86_64IntegerCondition(cond->AsCondition()->GetOppositeCondition()), false_target);
       } else {
@@ -2270,7 +2273,9 @@
 void LocationsBuilderX86_64::VisitIf(HIf* if_instr) {
   LocationSummary* locations = new (GetGraph()->GetAllocator()) LocationSummary(if_instr);
   if (IsBooleanValueOrMaterializedCondition(if_instr->InputAt(0))) {
-    if (GetGraph()->IsCompilingBaseline() && !Runtime::Current()->IsAotCompiler()) {
+    if (GetGraph()->IsCompilingBaseline() &&
+        codegen_->GetCompilerOptions().ProfileBranches() &&
+        !Runtime::Current()->IsAotCompiler()) {
       locations->SetInAt(0, Location::RequiresRegister());
       locations->AddTemp(Location::RequiresRegister());
     } else {
@@ -2287,7 +2292,9 @@
   Label* false_target = codegen_->GoesToNextBlock(if_instr->GetBlock(), false_successor) ?
       nullptr : codegen_->GetLabelOf(false_successor);
   if (IsBooleanValueOrMaterializedCondition(if_instr->InputAt(0))) {
-    if (GetGraph()->IsCompilingBaseline() && !Runtime::Current()->IsAotCompiler()) {
+    if (GetGraph()->IsCompilingBaseline() &&
+        codegen_->GetCompilerOptions().ProfileBranches() &&
+        !Runtime::Current()->IsAotCompiler()) {
       DCHECK(if_instr->InputAt(0)->IsCondition());
       CpuRegister temp = if_instr->GetLocations()->GetTemp(0).AsRegister<CpuRegister>();
       ProfilingInfo* info = GetGraph()->GetProfilingInfo();
@@ -2310,8 +2317,6 @@
         __ Bind(&done);
       }
     }
-  } else {
-    DCHECK(!GetGraph()->IsCompilingBaseline()) << if_instr->InputAt(0)->DebugName();
   }
   GenerateTestAndBranch(if_instr, /* condition_input_index= */ 0, true_target, false_target);
 }
@@ -2405,7 +2410,7 @@
       if (!condition->IsEmittedAtUseSite()) {
         // This was a previously materialized condition.
         // Can we use the existing condition code?
-        if (AreEflagsSetFrom(condition, select)) {
+        if (AreEflagsSetFrom(condition, select, codegen_->GetCompilerOptions())) {
           // Materialization was the previous instruction.  Condition codes are right.
           cond = X86_64IntegerCondition(condition->GetCondition());
         } else {

diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h
index 4db72a6..6ec83be 100644
--- a/compiler/optimizing/nodes.h
+++ b/compiler/optimizing/nodes.h

@@ -2663,12 +2663,7 @@
   void RemoveEnvironmentUsers();
 
   bool IsEmittedAtUseSite() const { return GetPackedFlag<kFlagEmittedAtUseSite>(); }
-  void MarkEmittedAtUseSite() {
-    // When compiling baseline, in order to do branch profiling, we don't want to
-    // emit conditions at use site.
-    DCHECK(!IsCondition() || !GetBlock()->GetGraph()->IsCompilingBaseline());
-    SetPackedFlag<kFlagEmittedAtUseSite>(true);
-  }
+  void MarkEmittedAtUseSite() { SetPackedFlag<kFlagEmittedAtUseSite>(true); }
 
  protected:
   // If set, the machine code for this instruction is assumed to be generated by

diff --git a/compiler/optimizing/prepare_for_register_allocation.cc b/compiler/optimizing/prepare_for_register_allocation.cc
index 59282a7..1e99732 100644
--- a/compiler/optimizing/prepare_for_register_allocation.cc
+++ b/compiler/optimizing/prepare_for_register_allocation.cc

@@ -180,7 +180,7 @@
     return false;
   }
 
-  if (GetGraph()->IsCompilingBaseline()) {
+  if (GetGraph()->IsCompilingBaseline() && compiler_options_.ProfileBranches()) {
     // To do branch profiling, we cannot emit conditions at use site.
     return false;
   }

diff --git a/test/850-checker-branches/run.py b/test/850-checker-branches/run.py
index b7ff987..9bdc033 100644
--- a/test/850-checker-branches/run.py
+++ b/test/850-checker-branches/run.py

@@ -25,5 +25,6 @@
       jit=True,
       runtime_option=["-Xjitinitialsize:32M"],
       Xcompiler_option=[
+          "--profile-branches",
           "--verbose-methods=withBranch"
       ])