commit de7c9e13a45d2f9163991d89a615ead98f2d9f29
author Alex Light <allight@google.com> Thu Apr 01 17:19:05 2021 -0700
committer Treehugger Robot <treehugger-gerrit@google.com> Thu Apr 08 08:26:40 2021 +0000
tree 3318a5549571f8485b10d0e272cb75d74ab2a368
parent 92a785785423b99cf903ce0e79d06fbf62ecf51a

Fix issue with propagating partial values

We would incorrectly not propagate or calculate partial read values
sometimes in the presence of loops. This fixes that issue by correctly
interpreting merged-unknowns as being phis when before escapes and
propagating uses of removed reads when needed.

Test: ./test.py --host
Test: ./art/tools/compile-jar.py --dex2oat `which dex2oatd64`  --profile-line 'HSLcom/android/textclassifier/common/statsd/GenerateLinksLogger;->logGenerateLinks(Ljava/lang/CharSequence;Landroid/view/textclassifier/TextLinks;Ljava/lang/String;JLcom/google/common/base/Optional;Lcom/google/common/base/Optional;)V' --arch arm64 ~/GoogleExtServices.apk -j1  --runtime-arg -verbose:compiler  --dump-stats
Bug: 183554067
Change-Id: I7f6e99934237174922ef2da2b77092e74cfb6a77

compiler/optimizing/load_store_elimination.cc

diff --git a/compiler/optimizing/load_store_elimination.cc b/compiler/optimizing/load_store_elimination.cc
index 1ea2ece..c4279c7 100644
--- a/compiler/optimizing/load_store_elimination.cc
+++ b/compiler/optimizing/load_store_elimination.cc
@@ -2833,7 +2833,7 @@
   std::replace_if(
       phi_placeholder_replacements_.begin(),
       phi_placeholder_replacements_.end(),
-      [](const Value& val) { return val.IsPureUnknown(); },
+      [](const Value& val) { return !val.IsDefault() && !val.IsInstruction(); },
       Value::Invalid());
 }
 
@@ -2978,7 +2978,7 @@
       }
 
       ReplaceInput(to_replace);
-      RemoveInput(to_remove);
+      RemoveAndReplaceInputs(to_remove);
       CreateConstructorFences(constructor_fences);
       PredicateInstructions(to_predicate);
 
@@ -3092,7 +3092,7 @@
       }
     }
 
-    void RemoveInput(const ScopedArenaVector<HInstruction*>& to_remove) {
+    void RemoveAndReplaceInputs(const ScopedArenaVector<HInstruction*>& to_remove) {
       for (HInstruction* ins : to_remove) {
         if (ins->GetBlock() == nullptr) {
           // Already dealt with.
@@ -3100,6 +3100,30 @@
         }
         DCHECK(BeforeAllEscapes(ins->GetBlock())) << *ins;
         if (ins->IsInstanceFieldGet() || ins->IsInstanceFieldSet()) {
+          bool instruction_has_users =
+              ins->IsInstanceFieldGet() && (!ins->GetUses().empty() || !ins->GetEnvUses().empty());
+          if (instruction_has_users) {
+            // Make sure any remaining users of read are replaced.
+            HInstruction* replacement =
+                helper_->lse_->GetPartialValueAt(OriginalNewInstance(), ins);
+            // NB ReplaceInput will remove a use from the list so this is
+            // guaranteed to finish eventually.
+            while (!ins->GetUses().empty()) {
+              const HUseListNode<HInstruction*>& use = ins->GetUses().front();
+              use.GetUser()->ReplaceInput(replacement, use.GetIndex());
+            }
+            while (!ins->GetEnvUses().empty()) {
+              const HUseListNode<HEnvironment*>& use = ins->GetEnvUses().front();
+              use.GetUser()->ReplaceInput(replacement, use.GetIndex());
+            }
+          } else {
+            DCHECK(ins->GetUses().empty())
+                << "Instruction has users!\n"
+                << ins->DumpWithArgs() << "\nUsers are " << ins->GetUses();
+            DCHECK(ins->GetEnvUses().empty())
+                << "Instruction has users!\n"
+                << ins->DumpWithArgs() << "\nUsers are " << ins->GetEnvUses();
+          }
           ins->GetBlock()->RemoveInstruction(ins);
         } else {
           // Can only be obj == other, obj != other, obj == obj (!?) or, obj != obj (!?)
@@ -3554,9 +3578,8 @@
     info = field_infos_[loc_off];
     DCHECK(loc->GetIndex() == nullptr);
     Value value = ReplacementOrValue(heap_values_for_[old_pred->GetBlockId()][loc_off].value);
-    if (value.NeedsLoopPhi()) {
+    if (value.NeedsLoopPhi() || value.IsMergedUnknown()) {
       Value repl = phi_placeholder_replacements_[PhiPlaceholderIndex(value.GetPhiPlaceholder())];
-      DCHECK(!repl.IsUnknown());
       DCHECK(repl.IsDefault() || repl.IsInvalid() || repl.IsInstruction())
           << repl << " from " << value << " pred is " << old_pred->GetBlockId();
       if (!repl.IsInvalid()) {