ART: Fix a compiler crash for VectorizeDef() idioms.

SAD vectorization idiom (and DotProduct which was based on it)
had a bug when some instruction in the loop was visited twice
causing a compiler crash. GenerateVecOp() was called for both
the idiom root instruction and its argument in the idiom
vectorization routine; however the argument could have been
already processed by that time. It happened when two vectorization
idioms' matched patterns had a common sub-expression.

Test: test-art-target.
Test: 623-checker-loop-regressions.
Change-Id: I8823c52f8ef62377c29310f0e335b9728d11068a

1 files changed, 37 insertions, 25 deletions

diff --git a/compiler/optimizing/loop_optimization.cc b/compiler/optimizing/loop_optimization.cc
index 5784707d0e..770a011550 100644
--- a/compiler/optimizing/loop_optimization.cc
+++ b/compiler/optimizing/loop_optimization.cc
@@ -2044,13 +2044,13 @@ bool HLoopOptimization::VectorizeSADIdiom(LoopNode* node,
       (reduction_type != DataType::Type::kInt32 && reduction_type != DataType::Type::kInt64)) {
     return false;
   }
-  HInstruction* q = instruction->InputAt(0);
-  HInstruction* v = instruction->InputAt(1);
+  HInstruction* acc = instruction->InputAt(0);
+  HInstruction* abs = instruction->InputAt(1);
   HInstruction* a = nullptr;
   HInstruction* b = nullptr;
-  if (v->IsAbs() &&
-      v->GetType() == reduction_type &&
-      IsSubConst2(graph_, v->InputAt(0), /*out*/ &a, /*out*/ &b)) {
+  if (abs->IsAbs() &&
+      abs->GetType() == reduction_type &&
+      IsSubConst2(graph_, abs->InputAt(0), /*out*/ &a, /*out*/ &b)) {
     DCHECK(a != nullptr && b != nullptr);
   } else {
     return false;
@@ -2076,16 +2076,16 @@ bool HLoopOptimization::VectorizeSADIdiom(LoopNode* node,
   // idiomatic operation. Sequential code uses the original scalar expressions.
   DCHECK(r != nullptr && s != nullptr);
   if (generate_code && vector_mode_ != kVector) {  // de-idiom
-    r = s = v->InputAt(0);
+    r = s = abs->InputAt(0);
   }
-  if (VectorizeUse(node, q, generate_code, sub_type, restrictions) &&
+  if (VectorizeUse(node, acc, generate_code, sub_type, restrictions) &&
       VectorizeUse(node, r, generate_code, sub_type, restrictions) &&
       VectorizeUse(node, s, generate_code, sub_type, restrictions)) {
     if (generate_code) {
       if (vector_mode_ == kVector) {
         vector_map_->Put(instruction, new (global_allocator_) HVecSADAccumulate(
             global_allocator_,
-            vector_map_->Get(q),
+            vector_map_->Get(acc),
             vector_map_->Get(r),
             vector_map_->Get(s),
             HVecOperation::ToProperType(reduction_type, is_unsigned),
@@ -2093,8 +2093,14 @@ bool HLoopOptimization::VectorizeSADIdiom(LoopNode* node,
             kNoDexPc));
         MaybeRecordStat(stats_, MethodCompilationStat::kLoopVectorizedIdiom);
       } else {
-        GenerateVecOp(v, vector_map_->Get(r), nullptr, reduction_type);
-        GenerateVecOp(instruction, vector_map_->Get(q), vector_map_->Get(v), reduction_type);
+        // "GenerateVecOp()" must not be called more than once for each original loop body
+        // instruction. As the SAD idiom processes both "current" instruction ("instruction")
+        // and its ABS input in one go, we must check that for the scalar case the ABS instruction
+        // has not yet been processed.
+        if (vector_map_->find(abs) == vector_map_->end()) {
+          GenerateVecOp(abs, vector_map_->Get(r), nullptr, reduction_type);
+        }
+        GenerateVecOp(instruction, vector_map_->Get(acc), vector_map_->Get(abs), reduction_type);
       }
     }
     return true;
@@ -2116,20 +2122,20 @@ bool HLoopOptimization::VectorizeDotProdIdiom(LoopNode* node,
     return false;
   }
 
-  HInstruction* q = instruction->InputAt(0);
-  HInstruction* v = instruction->InputAt(1);
-  if (!v->IsMul() || v->GetType() != reduction_type) {
+  HInstruction* const acc = instruction->InputAt(0);
+  HInstruction* const mul = instruction->InputAt(1);
+  if (!mul->IsMul() || mul->GetType() != reduction_type) {
     return false;
   }
 
-  HInstruction* a = v->InputAt(0);
-  HInstruction* b = v->InputAt(1);
-  HInstruction* r = a;
-  HInstruction* s = b;
-  DataType::Type op_type = GetNarrowerType(a, b);
+  HInstruction* const mul_left = mul->InputAt(0);
+  HInstruction* const mul_right = mul->InputAt(1);
+  HInstruction* r = mul_left;
+  HInstruction* s = mul_right;
+  DataType::Type op_type = GetNarrowerType(mul_left, mul_right);
   bool is_unsigned = false;
 
-  if (!IsNarrowerOperands(a, b, op_type, &r, &s, &is_unsigned)) {
+  if (!IsNarrowerOperands(mul_left, mul_right, op_type, &r, &s, &is_unsigned)) {
     return false;
   }
   op_type = HVecOperation::ToProperType(op_type, is_unsigned);
@@ -2143,17 +2149,17 @@ bool HLoopOptimization::VectorizeDotProdIdiom(LoopNode* node,
   // Accept dot product idiom for vectorizable operands. Vectorized code uses the shorthand
   // idiomatic operation. Sequential code uses the original scalar expressions.
   if (generate_code && vector_mode_ != kVector) {  // de-idiom
-    r = a;
-    s = b;
+    r = mul_left;
+    s = mul_right;
   }
-  if (VectorizeUse(node, q, generate_code, op_type, restrictions) &&
+  if (VectorizeUse(node, acc, generate_code, op_type, restrictions) &&
       VectorizeUse(node, r, generate_code, op_type, restrictions) &&
       VectorizeUse(node, s, generate_code, op_type, restrictions)) {
     if (generate_code) {
       if (vector_mode_ == kVector) {
         vector_map_->Put(instruction, new (global_allocator_) HVecDotProd(
             global_allocator_,
-            vector_map_->Get(q),
+            vector_map_->Get(acc),
             vector_map_->Get(r),
             vector_map_->Get(s),
             reduction_type,
@@ -2162,8 +2168,14 @@ bool HLoopOptimization::VectorizeDotProdIdiom(LoopNode* node,
             kNoDexPc));
         MaybeRecordStat(stats_, MethodCompilationStat::kLoopVectorizedIdiom);
       } else {
-        GenerateVecOp(v, vector_map_->Get(r), vector_map_->Get(s), reduction_type);
-        GenerateVecOp(instruction, vector_map_->Get(q), vector_map_->Get(v), reduction_type);
+        // "GenerateVecOp()" must not be called more than once for each original loop body
+        // instruction. As the DotProd idiom processes both "current" instruction ("instruction")
+        // and its MUL input in one go, we must check that for the scalar case the MUL instruction
+        // has not yet been processed.
+        if (vector_map_->find(mul) == vector_map_->end()) {
+          GenerateVecOp(mul, vector_map_->Get(r), vector_map_->Get(s), reduction_type);
+        }
+        GenerateVecOp(instruction, vector_map_->Get(acc), vector_map_->Get(mul), reduction_type);
       }
     }
     return true;