Revert "LSE fix to allow different operation kinds in the same heap location"

This reverts commit 717ffa8fd155e591e16fa4ac1cf4357f974450da.

Reason for revert: Some failures due to CHECKer pattern matching e.g. https://ci.chromium.org/ui/p/art/builders/ci/angler-armv7-ndebug/3635/overview and art-heap-poisoning

Change-Id: Ia00a5b81e9e9539a9cc140937ea1103f251345e9
diff --git a/compiler/optimizing/load_store_analysis.h b/compiler/optimizing/load_store_analysis.h
index c46a5b9..c0799db 100644
--- a/compiler/optimizing/load_store_analysis.h
+++ b/compiler/optimizing/load_store_analysis.h
@@ -171,16 +171,14 @@
                size_t offset,
                HInstruction* index,
                size_t vector_length,
-               int16_t declaring_class_def_index,
-               bool is_vec_op)
+               int16_t declaring_class_def_index)
       : ref_info_(ref_info),
         type_(DataType::ToSigned(type)),
         offset_(offset),
         index_(index),
         vector_length_(vector_length),
         declaring_class_def_index_(declaring_class_def_index),
-        has_aliased_locations_(false),
-        is_vec_op_(is_vec_op) {
+        has_aliased_locations_(false) {
     DCHECK(ref_info != nullptr);
     DCHECK((offset == kInvalidFieldOffset && index != nullptr) ||
            (offset != kInvalidFieldOffset && index == nullptr));
@@ -191,7 +189,6 @@
   size_t GetOffset() const { return offset_; }
   HInstruction* GetIndex() const { return index_; }
   size_t GetVectorLength() const { return vector_length_; }
-  bool IsVecOp() const { return is_vec_op_; }
 
   // Returns the definition of declaring class' dex index.
   // It's kDeclaringClassDefIndexForArrays for an array element.
@@ -230,12 +227,11 @@
   // Declaring class's def's dex index.
   // Invalid when this HeapLocation is not field access.
   const int16_t declaring_class_def_index_;
+
   // Has aliased heap locations in the method, due to either the
   // reference is aliased or the array element is aliased via different
   // index names.
   bool has_aliased_locations_;
-  // Whether this HeapLocation represents a vector operation.
-  bool is_vec_op_;
 
   DISALLOW_COPY_AND_ASSIGN(HeapLocation);
 };
@@ -322,8 +318,7 @@
                                  field->GetFieldOffset().SizeValue(),
                                  nullptr,
                                  HeapLocation::kScalar,
-                                 field->GetDeclaringClassDefIndex(),
-                                 /*is_vec_op=*/false);
+                                 field->GetDeclaringClassDefIndex());
   }
 
   size_t GetArrayHeapLocation(HInstruction* instruction) const {
@@ -332,10 +327,10 @@
     HInstruction* index = instruction->InputAt(1);
     DataType::Type type = instruction->GetType();
     size_t vector_length = HeapLocation::kScalar;
-    const bool is_vec_op = instruction->IsVecStore() || instruction->IsVecLoad();
     if (instruction->IsArraySet()) {
       type = instruction->AsArraySet()->GetComponentType();
-    } else if (is_vec_op) {
+    } else if (instruction->IsVecStore() ||
+               instruction->IsVecLoad()) {
       HVecOperation* vec_op = instruction->AsVecOperation();
       type = vec_op->GetPackedType();
       vector_length = vec_op->GetVectorLength();
@@ -347,8 +342,7 @@
                                  HeapLocation::kInvalidFieldOffset,
                                  index,
                                  vector_length,
-                                 HeapLocation::kDeclaringClassDefIndexForArrays,
-                                 is_vec_op);
+                                 HeapLocation::kDeclaringClassDefIndexForArrays);
   }
 
   bool HasHeapStores() const {
@@ -370,8 +364,7 @@
                                size_t offset,
                                HInstruction* index,
                                size_t vector_length,
-                               int16_t declaring_class_def_index,
-                               bool is_vec_op) const {
+                               int16_t declaring_class_def_index) const {
     DataType::Type lookup_type = DataType::ToSigned(type);
     for (size_t i = 0; i < heap_locations_.size(); i++) {
       HeapLocation* loc = heap_locations_[i];
@@ -380,8 +373,7 @@
           loc->GetOffset() == offset &&
           loc->GetIndex() == index &&
           loc->GetVectorLength() == vector_length &&
-          loc->GetDeclaringClassDefIndex() == declaring_class_def_index &&
-          loc->IsVecOp() == is_vec_op) {
+          loc->GetDeclaringClassDefIndex() == declaring_class_def_index) {
         return i;
       }
     }
@@ -526,15 +518,14 @@
                                size_t offset,
                                HInstruction* index,
                                size_t vector_length,
-                               int16_t declaring_class_def_index,
-                               bool is_vec_op) {
+                               int16_t declaring_class_def_index) {
     HInstruction* original_ref = HuntForOriginalReference(ref);
     ReferenceInfo* ref_info = GetOrCreateReferenceInfo(original_ref);
     size_t heap_location_idx = FindHeapLocationIndex(
-        ref_info, type, offset, index, vector_length, declaring_class_def_index, is_vec_op);
+        ref_info, type, offset, index, vector_length, declaring_class_def_index);
     if (heap_location_idx == kHeapLocationNotFound) {
-      HeapLocation* heap_loc = new (allocator_) HeapLocation(
-          ref_info, type, offset, index, vector_length, declaring_class_def_index, is_vec_op);
+      HeapLocation* heap_loc = new (allocator_)
+          HeapLocation(ref_info, type, offset, index, vector_length, declaring_class_def_index);
       heap_locations_.push_back(heap_loc);
     }
   }
@@ -548,22 +539,19 @@
                             offset,
                             nullptr,
                             HeapLocation::kScalar,
-                            declaring_class_def_index,
-                            /*is_vec_op=*/false);
+                            declaring_class_def_index);
   }
 
   void VisitArrayAccess(HInstruction* array,
                         HInstruction* index,
                         DataType::Type type,
-                        size_t vector_length,
-                        bool is_vec_op) {
+                        size_t vector_length) {
     MaybeCreateHeapLocation(array,
                             type,
                             HeapLocation::kInvalidFieldOffset,
                             index,
                             vector_length,
-                            HeapLocation::kDeclaringClassDefIndexForArrays,
-                            is_vec_op);
+                            HeapLocation::kDeclaringClassDefIndexForArrays);
   }
 
   void VisitPredicatedInstanceFieldGet(HPredicatedInstanceFieldGet* instruction) override {
@@ -597,7 +585,7 @@
     HInstruction* array = instruction->InputAt(0);
     HInstruction* index = instruction->InputAt(1);
     DataType::Type type = instruction->GetType();
-    VisitArrayAccess(array, index, type, HeapLocation::kScalar, /*is_vec_op=*/false);
+    VisitArrayAccess(array, index, type, HeapLocation::kScalar);
     CreateReferenceInfoForReferenceType(instruction);
   }
 
@@ -605,7 +593,7 @@
     HInstruction* array = instruction->InputAt(0);
     HInstruction* index = instruction->InputAt(1);
     DataType::Type type = instruction->GetComponentType();
-    VisitArrayAccess(array, index, type, HeapLocation::kScalar, /*is_vec_op=*/false);
+    VisitArrayAccess(array, index, type, HeapLocation::kScalar);
     has_heap_stores_ = true;
   }
 
@@ -613,7 +601,7 @@
     HInstruction* array = instruction->InputAt(0);
     HInstruction* index = instruction->InputAt(1);
     DataType::Type type = instruction->GetPackedType();
-    VisitArrayAccess(array, index, type, instruction->GetVectorLength(), /*is_vec_op=*/true);
+    VisitArrayAccess(array, index, type, instruction->GetVectorLength());
     CreateReferenceInfoForReferenceType(instruction);
   }
 
@@ -621,7 +609,7 @@
     HInstruction* array = instruction->InputAt(0);
     HInstruction* index = instruction->InputAt(1);
     DataType::Type type = instruction->GetPackedType();
-    VisitArrayAccess(array, index, type, instruction->GetVectorLength(), /*is_vec_op=*/true);
+    VisitArrayAccess(array, index, type, instruction->GetVectorLength());
     has_heap_stores_ = true;
   }
 
diff --git a/compiler/optimizing/load_store_analysis_test.cc b/compiler/optimizing/load_store_analysis_test.cc
index b6cf8b4..7930c0d 100644
--- a/compiler/optimizing/load_store_analysis_test.cc
+++ b/compiler/optimizing/load_store_analysis_test.cc
@@ -118,13 +118,12 @@
   size_t field = HeapLocation::kInvalidFieldOffset;
   size_t vec = HeapLocation::kScalar;
   size_t class_def = HeapLocation::kDeclaringClassDefIndexForArrays;
-  const bool is_vec_op = false;
   size_t loc1 = heap_location_collector.FindHeapLocationIndex(
-      ref, type, field, c1, vec, class_def, is_vec_op);
+      ref, type, field, c1, vec, class_def);
   size_t loc2 = heap_location_collector.FindHeapLocationIndex(
-      ref, type, field, c2, vec, class_def, is_vec_op);
+      ref, type, field, c2, vec, class_def);
   size_t loc3 = heap_location_collector.FindHeapLocationIndex(
-      ref, type, field, index, vec, class_def, is_vec_op);
+      ref, type, field, index, vec, class_def);
   // must find this reference info for array in HeapLocationCollector.
   ASSERT_TRUE(ref != nullptr);
   // must find these heap locations;
diff --git a/compiler/optimizing/load_store_elimination.cc b/compiler/optimizing/load_store_elimination.cc
index cd44082..b385e03 100644
--- a/compiler/optimizing/load_store_elimination.cc
+++ b/compiler/optimizing/load_store_elimination.cc
@@ -2113,15 +2113,9 @@
   HInstruction* replacement = GetDefaultValue(type);
   for (uint32_t phi_placeholder_index : visited.Indexes()) {
     DCHECK(phi_placeholder_replacements_[phi_placeholder_index].IsInvalid());
-    PhiPlaceholder curr = GetPhiPlaceholderAt(phi_placeholder_index);
-    HeapLocation* hl = heap_location_collector_.GetHeapLocation(curr.GetHeapLocation());
-    // We use both vector and non vector operations to analyze the information. However, we replace
-    // only non vector operations in this code path.
-    if (!hl->IsVecOp()) {
-      phi_placeholder_replacements_[phi_placeholder_index] = Value::ForInstruction(replacement);
-      phi_placeholders_to_materialize->ClearBit(phi_placeholder_index);
-    }
+    phi_placeholder_replacements_[phi_placeholder_index] = Value::ForInstruction(replacement);
   }
+  phi_placeholders_to_materialize->Subtract(&visited);
   return true;
 }
 
@@ -2176,15 +2170,9 @@
   DCHECK(replacement != nullptr);
   for (uint32_t phi_placeholder_index : visited.Indexes()) {
     DCHECK(phi_placeholder_replacements_[phi_placeholder_index].IsInvalid());
-    PhiPlaceholder curr = GetPhiPlaceholderAt(phi_placeholder_index);
-    HeapLocation* hl = heap_location_collector_.GetHeapLocation(curr.GetHeapLocation());
-    // We use both vector and non vector operations to analyze the information. However, we replace
-    // only vector operations in this code path.
-    if (hl->IsVecOp()) {
-      phi_placeholder_replacements_[phi_placeholder_index] = Value::ForInstruction(replacement);
-      phi_placeholders_to_materialize->ClearBit(phi_placeholder_index);
-    }
+    phi_placeholder_replacements_[phi_placeholder_index] = Value::ForInstruction(replacement);
   }
+  phi_placeholders_to_materialize->Subtract(&visited);
   return true;
 }
 
diff --git a/test/2256-checker-vector-replacement/expected-stderr.txt b/test/2256-checker-vector-replacement/expected-stderr.txt
deleted file mode 100644
index e69de29..0000000
--- a/test/2256-checker-vector-replacement/expected-stderr.txt
+++ /dev/null
diff --git a/test/2256-checker-vector-replacement/expected-stdout.txt b/test/2256-checker-vector-replacement/expected-stdout.txt
deleted file mode 100644
index e69de29..0000000
--- a/test/2256-checker-vector-replacement/expected-stdout.txt
+++ /dev/null
diff --git a/test/2256-checker-vector-replacement/info.txt b/test/2256-checker-vector-replacement/info.txt
deleted file mode 100644
index 374d7eb..0000000
--- a/test/2256-checker-vector-replacement/info.txt
+++ /dev/null
@@ -1,2 +0,0 @@
-Regression test to check we can perform LSE even when vector and
-non-vector operations reference the same heap location.
diff --git a/test/2256-checker-vector-replacement/src/Main.java b/test/2256-checker-vector-replacement/src/Main.java
deleted file mode 100644
index 43ec0d6..0000000
--- a/test/2256-checker-vector-replacement/src/Main.java
+++ /dev/null
@@ -1,69 +0,0 @@
-/*
- * Copyright (C) 2023 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-public class Main {
-    public static void main(String[] args) {
-        $noinline$testVectorAndNonVector();
-    }
-
-    // Before loop optimization we only had an array get. After it, we optimized to also have
-    // VecLoad operations.
-
-    /// CHECK-START: void Main.$noinline$testVectorAndNonVector() loop_optimization (before)
-    /// CHECK:     ArrayGet
-
-    /// CHECK-START: void Main.$noinline$testVectorAndNonVector() loop_optimization (after)
-    /// CHECK:     ArrayGet
-
-    /// CHECK-START: void Main.$noinline$testVectorAndNonVector() loop_optimization (before)
-    /// CHECK-NOT: VecLoad
-
-    /// CHECK-START: void Main.$noinline$testVectorAndNonVector() loop_optimization (after)
-    /// CHECK:     VecLoad
-
-    // In LoadStoreElimination both ArrayGet and VecLoad have the same heap location. We will try to
-    // replace the ArrayGet with the constant 0. The crash happens when we want to do the same with
-    // the vector operation, changing the vector operation to a scalar.
-
-    // We can eliminate the ArraySet and ArrayGet, but not the VectorOperations.
-
-    /// CHECK-START: void Main.$noinline$testVectorAndNonVector() load_store_elimination (before)
-    /// CHECK:     ArraySet
-    /// CHECK:     VecLoad
-    /// CHECK:     ArrayGet
-
-    /// CHECK-START: void Main.$noinline$testVectorAndNonVector() load_store_elimination (after)
-    /// CHECK-NOT: ArraySet
-
-    /// CHECK-START: void Main.$noinline$testVectorAndNonVector() load_store_elimination (after)
-    /// CHECK-NOT: ArrayGet
-
-    private static void $noinline$testVectorAndNonVector() {
-        int[] result = new int[10];
-        int[] source = new int[50];
-
-        for (int i = 0; i < result.length; ++i) {
-            int value = 0;
-            // Always true but needed to repro a crash since we need Phis.
-            if (i + 20 < source.length) {
-                for (int j = 0; j < 20; j++) {
-                    value += Math.abs(source[i + j]);
-                }
-            }
-            result[i] = value;
-        }
-    }
-}