Merge "Revert "Use implicit null checks inside try blocks.""

8 files changed, 71 insertions, 40 deletions

diff --git a/compiler/optimizing/code_generator.cc b/compiler/optimizing/code_generator.cc
index ac7d5fe427..2087888f4e 100644
--- a/compiler/optimizing/code_generator.cc
+++ b/compiler/optimizing/code_generator.cc
@@ -1081,6 +1081,13 @@ void CodeGenerator::EmitEnvironment(HEnvironment* environment, SlowPathCode* slo
   }
 }
 
+bool CodeGenerator::IsImplicitNullCheckAllowed(HNullCheck* null_check) const {
+  return compiler_options_.GetImplicitNullChecks() &&
+         // Null checks which might throw into a catch block need to save live
+         // registers and therefore cannot be done implicitly.
+         !null_check->CanThrowIntoCatchBlock();
+}
+
 bool CodeGenerator::CanMoveNullCheckToUser(HNullCheck* null_check) {
   HInstruction* first_next_not_move = null_check->GetNextDisregardingMoves();
 
@@ -1089,10 +1096,6 @@ bool CodeGenerator::CanMoveNullCheckToUser(HNullCheck* null_check) {
 }
 
 void CodeGenerator::MaybeRecordImplicitNullCheck(HInstruction* instr) {
-  if (!compiler_options_.GetImplicitNullChecks()) {
-    return;
-  }
-
   // If we are from a static path don't record the pc as we can't throw NPE.
   // NB: having the checks here makes the code much less verbose in the arch
   // specific code generators.
@@ -1111,31 +1114,16 @@ void CodeGenerator::MaybeRecordImplicitNullCheck(HInstruction* instr) {
   // and needs to record the pc.
   if (first_prev_not_move != nullptr && first_prev_not_move->IsNullCheck()) {
     HNullCheck* null_check = first_prev_not_move->AsNullCheck();
-    // TODO: The parallel moves modify the environment. Their changes need to be
-    // reverted otherwise the stack maps at the throw point will not be correct.
-    RecordPcInfo(null_check, null_check->GetDexPc());
-  }
-}
-
-LocationSummary* CodeGenerator::CreateNullCheckLocations(HNullCheck* null_check) {
-  // Note: Using kNoCall allows the method to be treated as leaf (and eliminate the
-  // HSuspendCheck from entry block). However, it will still get a valid stack frame
-  // because the HNullCheck needs an environment.
-  LocationSummary::CallKind call_kind = LocationSummary::kNoCall;
-  // When throwing from a try block, we may need to retrieve dalvik registers from
-  // physical registers. For implicit null checks, this is done by using kSaveEverything
-  // runtime method but for explicit null checks we need to save live registers.
-  if (!compiler_options_.GetImplicitNullChecks() && null_check->CanThrowIntoCatchBlock()) {
-    call_kind = LocationSummary::kCallOnSlowPath;
+    if (IsImplicitNullCheckAllowed(null_check)) {
+      // TODO: The parallel moves modify the environment. Their changes need to be
+      // reverted otherwise the stack maps at the throw point will not be correct.
+      RecordPcInfo(null_check, null_check->GetDexPc());
+    }
   }
-  LocationSummary* locations = new (GetGraph()->GetArena()) LocationSummary(null_check, call_kind);
-  locations->SetInAt(0, Location::RequiresRegister());
-  DCHECK(!null_check->HasUses());
-  return locations;
 }
 
 void CodeGenerator::GenerateNullCheck(HNullCheck* instruction) {
-  if (compiler_options_.GetImplicitNullChecks()) {
+  if (IsImplicitNullCheckAllowed(instruction)) {
     MaybeRecordStat(kImplicitNullCheckGenerated);
     GenerateImplicitNullCheck(instruction);
   } else {
diff --git a/compiler/optimizing/code_generator.h b/compiler/optimizing/code_generator.h
index b4d4b9b760..0c60a98139 100644
--- a/compiler/optimizing/code_generator.h
+++ b/compiler/optimizing/code_generator.h
@@ -313,7 +313,6 @@ class CodeGenerator : public DeletableArenaObject<kArenaAllocCodeGenerator> {
 
   bool CanMoveNullCheckToUser(HNullCheck* null_check);
   void MaybeRecordImplicitNullCheck(HInstruction* instruction);
-  LocationSummary* CreateNullCheckLocations(HNullCheck* null_check);
   void GenerateNullCheck(HNullCheck* null_check);
   virtual void GenerateImplicitNullCheck(HNullCheck* null_check) = 0;
   virtual void GenerateExplicitNullCheck(HNullCheck* null_check) = 0;
@@ -323,6 +322,12 @@ class CodeGenerator : public DeletableArenaObject<kArenaAllocCodeGenerator> {
   // TODO: Replace with a catch-entering instruction that records the environment.
   void RecordCatchBlockInfo();
 
+  // Returns true if implicit null checks are allowed in the compiler options
+  // and if the null check is not inside a try block. We currently cannot do
+  // implicit null checks in that case because we need the NullCheckSlowPath to
+  // save live registers, which may be needed by the runtime to set catch phis.
+  bool IsImplicitNullCheckAllowed(HNullCheck* null_check) const;
+
   // TODO: Avoid creating the `std::unique_ptr` here.
   void AddSlowPath(SlowPathCode* slow_path) {
     slow_paths_.push_back(std::unique_ptr<SlowPathCode>(slow_path));
@@ -708,8 +713,6 @@ class CodeGenerator : public DeletableArenaObject<kArenaAllocCodeGenerator> {
   bool is_leaf_;
 
   // Whether an instruction in the graph accesses the current method.
-  // TODO: Rename: this actually indicates that some instruction in the method
-  // needs the environment including a valid stack frame.
   bool requires_current_method_;
 
   friend class OptimizingCFITest;
diff --git a/compiler/optimizing/code_generator_arm.cc b/compiler/optimizing/code_generator_arm.cc
index 40c2b9c1ec..3cc2598f8f 100644
--- a/compiler/optimizing/code_generator_arm.cc
+++ b/compiler/optimizing/code_generator_arm.cc
@@ -4251,7 +4251,14 @@ void InstructionCodeGeneratorARM::VisitUnresolvedStaticFieldSet(
 }
 
 void LocationsBuilderARM::VisitNullCheck(HNullCheck* instruction) {
-  codegen_->CreateNullCheckLocations(instruction);
+  LocationSummary::CallKind call_kind = instruction->CanThrowIntoCatchBlock()
+      ? LocationSummary::kCallOnSlowPath
+      : LocationSummary::kNoCall;
+  LocationSummary* locations = new (GetGraph()->GetArena()) LocationSummary(instruction, call_kind);
+  locations->SetInAt(0, Location::RequiresRegister());
+  if (instruction->HasUses()) {
+    locations->SetOut(Location::SameAsFirstInput());
+  }
 }
 
 void CodeGeneratorARM::GenerateImplicitNullCheck(HNullCheck* instruction) {
diff --git a/compiler/optimizing/code_generator_arm64.cc b/compiler/optimizing/code_generator_arm64.cc
index c00ab56a4c..179bf76f5b 100644
--- a/compiler/optimizing/code_generator_arm64.cc
+++ b/compiler/optimizing/code_generator_arm64.cc
@@ -4384,7 +4384,14 @@ void InstructionCodeGeneratorARM64::VisitBooleanNot(HBooleanNot* instruction) {
 }
 
 void LocationsBuilderARM64::VisitNullCheck(HNullCheck* instruction) {
-  codegen_->CreateNullCheckLocations(instruction);
+  LocationSummary::CallKind call_kind = instruction->CanThrowIntoCatchBlock()
+      ? LocationSummary::kCallOnSlowPath
+      : LocationSummary::kNoCall;
+  LocationSummary* locations = new (GetGraph()->GetArena()) LocationSummary(instruction, call_kind);
+  locations->SetInAt(0, Location::RequiresRegister());
+  if (instruction->HasUses()) {
+    locations->SetOut(Location::SameAsFirstInput());
+  }
 }
 
 void CodeGeneratorARM64::GenerateImplicitNullCheck(HNullCheck* instruction) {
diff --git a/compiler/optimizing/code_generator_mips.cc b/compiler/optimizing/code_generator_mips.cc
index fdfc5514f8..f07f8a0d91 100644
--- a/compiler/optimizing/code_generator_mips.cc
+++ b/compiler/optimizing/code_generator_mips.cc
@@ -5075,7 +5075,14 @@ void InstructionCodeGeneratorMIPS::VisitBooleanNot(HBooleanNot* instruction) {
 }
 
 void LocationsBuilderMIPS::VisitNullCheck(HNullCheck* instruction) {
-  codegen_->CreateNullCheckLocations(instruction);
+  LocationSummary::CallKind call_kind = instruction->CanThrowIntoCatchBlock()
+      ? LocationSummary::kCallOnSlowPath
+      : LocationSummary::kNoCall;
+  LocationSummary* locations = new (GetGraph()->GetArena()) LocationSummary(instruction, call_kind);
+  locations->SetInAt(0, Location::RequiresRegister());
+  if (instruction->HasUses()) {
+    locations->SetOut(Location::SameAsFirstInput());
+  }
 }
 
 void CodeGeneratorMIPS::GenerateImplicitNullCheck(HNullCheck* instruction) {
diff --git a/compiler/optimizing/code_generator_mips64.cc b/compiler/optimizing/code_generator_mips64.cc
index 4d87523206..664d498b18 100644
--- a/compiler/optimizing/code_generator_mips64.cc
+++ b/compiler/optimizing/code_generator_mips64.cc
@@ -3461,7 +3461,14 @@ void InstructionCodeGeneratorMIPS64::VisitBooleanNot(HBooleanNot* instruction) {
 }
 
 void LocationsBuilderMIPS64::VisitNullCheck(HNullCheck* instruction) {
-  codegen_->CreateNullCheckLocations(instruction);
+  LocationSummary::CallKind call_kind = instruction->CanThrowIntoCatchBlock()
+      ? LocationSummary::kCallOnSlowPath
+      : LocationSummary::kNoCall;
+  LocationSummary* locations = new (GetGraph()->GetArena()) LocationSummary(instruction, call_kind);
+  locations->SetInAt(0, Location::RequiresRegister());
+  if (instruction->HasUses()) {
+    locations->SetOut(Location::SameAsFirstInput());
+  }
 }
 
 void CodeGeneratorMIPS64::GenerateImplicitNullCheck(HNullCheck* instruction) {
diff --git a/compiler/optimizing/code_generator_x86.cc b/compiler/optimizing/code_generator_x86.cc
index 28db29cb58..e18b366411 100644
--- a/compiler/optimizing/code_generator_x86.cc
+++ b/compiler/optimizing/code_generator_x86.cc
@@ -4950,10 +4950,16 @@ void InstructionCodeGeneratorX86::VisitUnresolvedStaticFieldSet(
 }
 
 void LocationsBuilderX86::VisitNullCheck(HNullCheck* instruction) {
-  LocationSummary* locations = codegen_->CreateNullCheckLocations(instruction);
-  if (!codegen_->GetCompilerOptions().GetImplicitNullChecks()) {
-    // Explicit null checks can use any location.
-    locations->SetInAt(0, Location::Any());
+  LocationSummary::CallKind call_kind = instruction->CanThrowIntoCatchBlock()
+      ? LocationSummary::kCallOnSlowPath
+      : LocationSummary::kNoCall;
+  LocationSummary* locations = new (GetGraph()->GetArena()) LocationSummary(instruction, call_kind);
+  Location loc = codegen_->IsImplicitNullCheckAllowed(instruction)
+      ? Location::RequiresRegister()
+      : Location::Any();
+  locations->SetInAt(0, loc);
+  if (instruction->HasUses()) {
+    locations->SetOut(Location::SameAsFirstInput());
   }
 }
 
diff --git a/compiler/optimizing/code_generator_x86_64.cc b/compiler/optimizing/code_generator_x86_64.cc
index 88d98fc1e1..15307fe50c 100644
--- a/compiler/optimizing/code_generator_x86_64.cc
+++ b/compiler/optimizing/code_generator_x86_64.cc
@@ -4459,10 +4459,16 @@ void InstructionCodeGeneratorX86_64::VisitUnresolvedStaticFieldSet(
 }
 
 void LocationsBuilderX86_64::VisitNullCheck(HNullCheck* instruction) {
-  LocationSummary* locations = codegen_->CreateNullCheckLocations(instruction);
-  if (!codegen_->GetCompilerOptions().GetImplicitNullChecks()) {
-    // Explicit null checks can use any location.
-    locations->SetInAt(0, Location::Any());
+  LocationSummary::CallKind call_kind = instruction->CanThrowIntoCatchBlock()
+      ? LocationSummary::kCallOnSlowPath
+      : LocationSummary::kNoCall;
+  LocationSummary* locations = new (GetGraph()->GetArena()) LocationSummary(instruction, call_kind);
+  Location loc = codegen_->IsImplicitNullCheckAllowed(instruction)
+      ? Location::RequiresRegister()
+      : Location::Any();
+  locations->SetInAt(0, loc);
+  if (instruction->HasUses()) {
+    locations->SetOut(Location::SameAsFirstInput());
   }
 }