Revert "Add VarHandle implementations for void getAndUpdate methods"

This reverts commit 9ff2b617341bfe574e2d8706553f0cd65a1a2fc8.

Reason for revert: failed ART test 712-varhandle-invocations on riscv64 
    `F dex2oat64: intrinsics_riscv64.cc:4821 Check failed: locations->GetTempCount() == 2u (locations->GetTempCount()=3, 2u=2)`

Change-Id: I68a3830ccd730afa66e9cff277fe2a06829ddabe

1 files changed, 60 insertions, 133 deletions

diff --git a/compiler/optimizing/intrinsics_x86_64.cc b/compiler/optimizing/intrinsics_x86_64.cc
index ba254ee705..6424201c4b 100644
--- a/compiler/optimizing/intrinsics_x86_64.cc
+++ b/compiler/optimizing/intrinsics_x86_64.cc
@@ -27,8 +27,8 @@
 #include "entrypoints/quick/quick_entrypoints.h"
 #include "entrypoints/quick/quick_entrypoints_enum.h"
 #include "heap_poisoning.h"
-#include "intrinsic_objects.h"
 #include "intrinsics.h"
+#include "intrinsic_objects.h"
 #include "intrinsics_utils.h"
 #include "lock_word.h"
 #include "mirror/array-inl.h"
@@ -36,7 +36,6 @@
 #include "mirror/reference.h"
 #include "mirror/string.h"
 #include "optimizing/code_generator.h"
-#include "optimizing/data_type.h"
 #include "scoped_thread_state_change-inl.h"
 #include "thread-current-inl.h"
 #include "utils/x86_64/assembler_x86_64.h"
@@ -2634,11 +2633,7 @@ static void CreateUnsafeGetAndUpdateLocations(ArenaAllocator* allocator,
   // Use the same register for both the output and the new value or addend
   // to take advantage of XCHG or XADD. Arbitrarily pick RAX.
   locations->SetInAt(3, Location::RegisterLocation(RAX));
-  // Only set the `out` register if it's needed. In the void case we can still use RAX in the
-  // same manner as it is used an as `in` register.
-  if (invoke->GetType() != DataType::Type::kVoid) {
-    locations->SetOut(Location::RegisterLocation(RAX));
-  }
+  locations->SetOut(Location::RegisterLocation(RAX));
 }
 
 void IntrinsicLocationsBuilderX86_64::VisitUnsafeGetAndAddInt(HInvoke* invoke) {
@@ -2702,29 +2697,25 @@ static void GenUnsafeGetAndUpdate(HInvoke* invoke,
   X86_64Assembler* assembler = down_cast<X86_64Assembler*>(codegen->GetAssembler());
   LocationSummary* locations = invoke->GetLocations();
 
-  const bool is_void = invoke->GetType() == DataType::Type::kVoid;
-  Location rax_loc = Location::RegisterLocation(RAX);
-  // We requested RAX to use as a temporary for void methods, as we don't return the value.
-  DCHECK_IMPLIES(!is_void, locations->Out().Equals(rax_loc));
-  CpuRegister out_or_temp = rax_loc.AsRegister<CpuRegister>();           // Result.
-  CpuRegister base = locations->InAt(1).AsRegister<CpuRegister>();       // Object pointer.
-  CpuRegister offset = locations->InAt(2).AsRegister<CpuRegister>();     // Long offset.
-  DCHECK_EQ(out_or_temp, locations->InAt(3).AsRegister<CpuRegister>());  // New value or addend.
+  CpuRegister out = locations->Out().AsRegister<CpuRegister>();       // Result.
+  CpuRegister base = locations->InAt(1).AsRegister<CpuRegister>();    // Object pointer.
+  CpuRegister offset = locations->InAt(2).AsRegister<CpuRegister>();  // Long offset.
+  DCHECK_EQ(out, locations->InAt(3).AsRegister<CpuRegister>());       // New value or addend.
   Address field_address(base, offset, TIMES_1, 0);
 
   if (type == DataType::Type::kInt32) {
     if (get_and_update_op == GetAndUpdateOp::kAdd) {
-      __ LockXaddl(field_address, out_or_temp);
+      __ LockXaddl(field_address, out);
     } else {
       DCHECK(get_and_update_op == GetAndUpdateOp::kSet);
-      __ xchgl(out_or_temp, field_address);
+      __ xchgl(out, field_address);
     }
   } else if (type == DataType::Type::kInt64) {
     if (get_and_update_op == GetAndUpdateOp::kAdd) {
-      __ LockXaddq(field_address, out_or_temp);
+      __ LockXaddq(field_address, out);
     } else {
       DCHECK(get_and_update_op == GetAndUpdateOp::kSet);
-      __ xchgq(out_or_temp, field_address);
+      __ xchgq(out, field_address);
     }
   } else {
     DCHECK_EQ(type, DataType::Type::kReference);
@@ -2749,20 +2740,18 @@ static void GenUnsafeGetAndUpdate(HInvoke* invoke,
 
     // Mark card for object as a new value shall be stored.
     bool new_value_can_be_null = true;  // TODO: Worth finding out this information?
-    codegen->MaybeMarkGCCard(temp1, temp2, base, /*value=*/out_or_temp, new_value_can_be_null);
+    codegen->MaybeMarkGCCard(temp1, temp2, base, /*value=*/out, new_value_can_be_null);
 
     if (kPoisonHeapReferences) {
       // Use a temp to avoid poisoning base of the field address, which might happen if `out`
       // is the same as `base` (for code like `unsafe.getAndSet(obj, offset, obj)`).
-      __ movl(temp1, out_or_temp);
+      __ movl(temp1, out);
       __ PoisonHeapReference(temp1);
       __ xchgl(temp1, field_address);
-      if (!is_void) {
-        __ UnpoisonHeapReference(temp1);
-        __ movl(out_or_temp, temp1);
-      }
+      __ UnpoisonHeapReference(temp1);
+      __ movl(out, temp1);
     } else {
-      __ xchgl(out_or_temp, field_address);
+      __ xchgl(out, field_address);
     }
   }
 }
@@ -4498,33 +4487,23 @@ static void CreateVarHandleGetAndSetLocations(HInvoke* invoke, CodeGeneratorX86_
     return;
   }
 
-  // Get the type from the shorty as the invokes may not return a value.
   uint32_t number_of_arguments = invoke->GetNumberOfArguments();
   uint32_t new_value_index = number_of_arguments - 1;
-  DataType::Type value_type = GetDataTypeFromShorty(invoke, new_value_index);
-  DataType::Type return_type = invoke->GetType();
-  const bool is_void = return_type == DataType::Type::kVoid;
-  DCHECK_IMPLIES(!is_void, return_type == value_type);
+  DataType::Type type = invoke->GetType();
+  DCHECK_EQ(type, GetDataTypeFromShorty(invoke, new_value_index));
 
   LocationSummary* locations = CreateVarHandleCommonLocations(invoke);
 
-  if (DataType::IsFloatingPointType(value_type)) {
-    // Only set the `out` register if it's needed. In the void case we don't use `out`.
-    if (!is_void) {
-      locations->SetOut(Location::RequiresFpuRegister());
-    }
+  if (DataType::IsFloatingPointType(type)) {
+    locations->SetOut(Location::RequiresFpuRegister());
     // A temporary is needed to load the new floating-point value into a register for XCHG.
     locations->AddTemp(Location::RequiresRegister());
   } else {
-    // Only set the `out` register if it's needed. In the void case we can still use RAX in the
-    // same manner as it is used an as `in` register.
-    if (!is_void) {
-      // Use the same register for both the new value and output to take advantage of XCHG.
-      // It doesn't have to be RAX, but we need to choose some to make sure it's the same.
-      locations->SetOut(Location::RegisterLocation(RAX));
-    }
+    // Use the same register for both the new value and output to take advantage of XCHG.
+    // It doesn't have to be RAX, but we need to choose some to make sure it's the same.
+    locations->SetOut(Location::RegisterLocation(RAX));
     locations->SetInAt(new_value_index, Location::RegisterLocation(RAX));
-    if (value_type == DataType::Type::kReference) {
+    if (type == DataType::Type::kReference) {
       // Need two temporaries for MarkGCCard.
       locations->AddRegisterTemps(2);
       if (codegen->EmitReadBarrier()) {
@@ -4547,9 +4526,6 @@ static void GenerateVarHandleGetAndSet(HInvoke* invoke,
   LocationSummary* locations = invoke->GetLocations();
   Location out = locations->Out();
   uint32_t temp_count = locations->GetTempCount();
-  DataType::Type return_type = invoke->GetType();
-  const bool is_void = return_type == DataType::Type::kVoid;
-  DCHECK_IMPLIES(!is_void, return_type == type);
 
   if (DataType::IsFloatingPointType(type)) {
     // `getAndSet` for floating-point types: move the new FP value into a register, atomically
@@ -4569,9 +4545,7 @@ static void GenerateVarHandleGetAndSet(HInvoke* invoke,
     if (byte_swap) {
       codegen->GetInstructionCodegen()->Bswap(temp, bswap_type);
     }
-    if (!is_void) {
-      __ movd(out.AsFpuRegister<XmmRegister>(), temp.AsRegister<CpuRegister>(), is64bit);
-    }
+    __ movd(out.AsFpuRegister<XmmRegister>(), temp.AsRegister<CpuRegister>(), is64bit);
   } else if (type == DataType::Type::kReference) {
     // `getAndSet` for references: load reference and atomically exchange it with the field.
     // Output register is the same as the one holding new value, so no need to move the result.
@@ -4594,17 +4568,15 @@ static void GenerateVarHandleGetAndSet(HInvoke* invoke,
     }
     codegen->MarkGCCard(temp1, temp2, ref);
 
-    DCHECK_IMPLIES(!is_void, valreg == out.AsRegister<CpuRegister>());
+    DCHECK_EQ(valreg, out.AsRegister<CpuRegister>());
     if (kPoisonHeapReferences) {
       // Use a temp to avoid poisoning base of the field address, which might happen if `valreg` is
       // the same as `target.object` (for code like `vh.getAndSet(obj, obj)`).
       __ movl(temp1, valreg);
       __ PoisonHeapReference(temp1);
       __ xchgl(temp1, field_addr);
-      if (!is_void) {
-        __ UnpoisonHeapReference(temp1);
-        __ movl(valreg, temp1);
-      }
+      __ UnpoisonHeapReference(temp1);
+      __ movl(valreg, temp1);
     } else {
       __ xchgl(valreg, field_addr);
     }
@@ -4615,32 +4587,24 @@ static void GenerateVarHandleGetAndSet(HInvoke* invoke,
       codegen->GetInstructionCodegen()->Bswap(value, type);
     }
     CpuRegister valreg = value.AsRegister<CpuRegister>();
-    DCHECK_IMPLIES(!is_void, valreg == out.AsRegister<CpuRegister>());
+    DCHECK_EQ(valreg, out.AsRegister<CpuRegister>());
     switch (type) {
       case DataType::Type::kBool:
       case DataType::Type::kUint8:
         __ xchgb(valreg, field_addr);
-        if (!is_void) {
-          __ movzxb(valreg, valreg);
-        }
+        __ movzxb(valreg, valreg);
         break;
       case DataType::Type::kInt8:
         __ xchgb(valreg, field_addr);
-        if (!is_void) {
-          __ movsxb(valreg, valreg);
-        }
+        __ movsxb(valreg, valreg);
         break;
       case DataType::Type::kUint16:
         __ xchgw(valreg, field_addr);
-        if (!is_void) {
-          __ movzxw(valreg, valreg);
-        }
+        __ movzxw(valreg, valreg);
         break;
       case DataType::Type::kInt16:
         __ xchgw(valreg, field_addr);
-        if (!is_void) {
-          __ movsxw(valreg, valreg);
-        }
+        __ movsxw(valreg, valreg);
         break;
       case DataType::Type::kInt32:
       case DataType::Type::kUint32:
@@ -4665,36 +4629,25 @@ static void CreateVarHandleGetAndBitwiseOpLocations(HInvoke* invoke, CodeGenerat
     return;
   }
 
-  // Get the type from the shorty as the invokes may not return a value.
   uint32_t number_of_arguments = invoke->GetNumberOfArguments();
   uint32_t new_value_index = number_of_arguments - 1;
-  DataType::Type value_type = GetDataTypeFromShorty(invoke, new_value_index);
-  DataType::Type return_type = invoke->GetType();
-  const bool is_void = return_type == DataType::Type::kVoid;
-  DCHECK_IMPLIES(!is_void, return_type == value_type);
+  DataType::Type type = invoke->GetType();
+  DCHECK_EQ(type, GetDataTypeFromShorty(invoke, new_value_index));
 
   LocationSummary* locations = CreateVarHandleCommonLocations(invoke);
 
-  DCHECK_NE(DataType::Type::kReference, value_type);
-  DCHECK(!DataType::IsFloatingPointType(value_type));
+  DCHECK_NE(DataType::Type::kReference, type);
+  DCHECK(!DataType::IsFloatingPointType(type));
 
-  if (!is_void) {
-    // Output is in RAX to accommodate CMPXCHG. It is also used as a temporary.
-    locations->SetOut(Location::RegisterLocation(RAX));
-  } else {
-    // Used as a temporary, even when we are not outputting it so reserve it. This has to be
-    // requested before the other temporary since there's variable number of temp registers and the
-    // other temp register is expected to be the last one.
-    locations->AddTemp(Location::RegisterLocation(RAX));
-  }
   // A temporary to compute the bitwise operation on the old and the new values.
   locations->AddTemp(Location::RequiresRegister());
   // We need value to be either in a register, or a 32-bit constant (as there are no arithmetic
   // instructions that accept 64-bit immediate on x86_64).
-  locations->SetInAt(new_value_index,
-                     DataType::Is64BitType(value_type) ?
-                         Location::RequiresRegister() :
-                         Location::RegisterOrConstant(invoke->InputAt(new_value_index)));
+  locations->SetInAt(new_value_index, DataType::Is64BitType(type)
+      ? Location::RequiresRegister()
+      : Location::RegisterOrConstant(invoke->InputAt(new_value_index)));
+  // Output is in RAX to accommodate CMPXCHG. It is also used as a temporary.
+  locations->SetOut(Location::RegisterLocation(RAX));
 }
 
 static void GenerateVarHandleGetAndOp(HInvoke* invoke,
@@ -4706,9 +4659,8 @@ static void GenerateVarHandleGetAndOp(HInvoke* invoke,
                                       bool byte_swap) {
   X86_64Assembler* assembler = codegen->GetAssembler();
   LocationSummary* locations = invoke->GetLocations();
-  Location temp_loc = locations->GetTemp(locations->GetTempCount() - 1u);
-  Location rax_loc = Location::RegisterLocation(RAX);
-  DCHECK_IMPLIES(invoke->GetType() != DataType::Type::kVoid, locations->Out().Equals(rax_loc));
+  Location temp_loc = locations->GetTemp(locations->GetTempCount() - 1);
+  Location rax_loc = locations->Out();
   CpuRegister temp = temp_loc.AsRegister<CpuRegister>();
   CpuRegister rax = rax_loc.AsRegister<CpuRegister>();
   DCHECK_EQ(rax.AsRegister(), RAX);
@@ -4816,21 +4768,15 @@ static void CreateVarHandleGetAndAddLocations(HInvoke* invoke, CodeGeneratorX86_
     return;
   }
 
-  // Get the type from the shorty as the invokes may not return a value.
   uint32_t number_of_arguments = invoke->GetNumberOfArguments();
   uint32_t new_value_index = number_of_arguments - 1;
-  DataType::Type value_type = GetDataTypeFromShorty(invoke, new_value_index);
-  DataType::Type return_type = invoke->GetType();
-  const bool is_void = return_type == DataType::Type::kVoid;
-  DCHECK_IMPLIES(!is_void, return_type == value_type);
+  DataType::Type type = invoke->GetType();
+  DCHECK_EQ(type, GetDataTypeFromShorty(invoke, new_value_index));
 
   LocationSummary* locations = CreateVarHandleCommonLocations(invoke);
 
-  if (DataType::IsFloatingPointType(value_type)) {
-    // Only set the `out` register if it's needed. In the void case we don't use `out`
-    if (!is_void) {
-      locations->SetOut(Location::RequiresFpuRegister());
-    }
+  if (DataType::IsFloatingPointType(type)) {
+    locations->SetOut(Location::RequiresFpuRegister());
     // Require that the new FP value is in a register (and not a constant) for ADDSS/ADDSD.
     locations->SetInAt(new_value_index, Location::RequiresFpuRegister());
     // CMPXCHG clobbers RAX.
@@ -4840,15 +4786,11 @@ static void CreateVarHandleGetAndAddLocations(HInvoke* invoke, CodeGeneratorX86_
     // A temporary to hold the new value for CMPXCHG.
     locations->AddTemp(Location::RequiresRegister());
   } else {
-    DCHECK_NE(value_type, DataType::Type::kReference);
-    // Only set the `out` register if it's needed. In the void case we can still use RAX in the
-    // same manner as it is used an as `in` register.
-    if (!is_void) {
-      // Use the same register for both the new value and output to take advantage of XADD.
-      // It should be RAX, because the byte-swapping path of GenerateVarHandleGetAndAdd falls
-      // back to GenerateVarHandleGetAndOp that expects out in RAX.
-      locations->SetOut(Location::RegisterLocation(RAX));
-    }
+    DCHECK_NE(type, DataType::Type::kReference);
+    // Use the same register for both the new value and output to take advantage of XADD.
+    // It should be RAX, because the byte-swapping path of GenerateVarHandleGetAndAdd falls
+    // back to GenerateVarHandleGetAndOp that expects out in RAX.
+    locations->SetOut(Location::RegisterLocation(RAX));
     locations->SetInAt(new_value_index, Location::RegisterLocation(RAX));
     if (GetExpectedVarHandleCoordinatesCount(invoke) == 2) {
       // For byte array views with non-native endianness we need extra BSWAP operations, so we
@@ -4872,10 +4814,6 @@ static void GenerateVarHandleGetAndAdd(HInvoke* invoke,
   Location out = locations->Out();
   uint32_t temp_count = locations->GetTempCount();
 
-  DataType::Type return_type = invoke->GetType();
-  const bool is_void = return_type == DataType::Type::kVoid;
-  DCHECK_IMPLIES(!is_void, return_type == type);
-
   if (DataType::IsFloatingPointType(type)) {
     if (byte_swap) {
       // This code should never be executed: it is the case of a byte array view (since it requires
@@ -4938,9 +4876,7 @@ static void GenerateVarHandleGetAndAdd(HInvoke* invoke,
     if (byte_swap) {
       codegen->GetInstructionCodegen()->Bswap(rax_loc, bswap_type);
     }
-    if (!is_void) {
-      __ movd(out.AsFpuRegister<XmmRegister>(), CpuRegister(RAX), is64bit);
-    }
+    __ movd(out.AsFpuRegister<XmmRegister>(), CpuRegister(RAX), is64bit);
   } else {
     if (byte_swap) {
       // We cannot use XADD since we need to byte-swap the old value when reading it from memory,
@@ -4957,32 +4893,24 @@ static void GenerateVarHandleGetAndAdd(HInvoke* invoke,
       // the old value to the field. Output register is the same as the one holding new value. Do
       // sign extend / zero extend as needed.
       CpuRegister valreg = value.AsRegister<CpuRegister>();
-      DCHECK_IMPLIES(!is_void, valreg == out.AsRegister<CpuRegister>());
+      DCHECK_EQ(valreg, out.AsRegister<CpuRegister>());
       switch (type) {
         case DataType::Type::kBool:
         case DataType::Type::kUint8:
           __ LockXaddb(field_addr, valreg);
-          if (!is_void) {
-            __ movzxb(valreg, valreg);
-          }
+          __ movzxb(valreg, valreg);
           break;
         case DataType::Type::kInt8:
           __ LockXaddb(field_addr, valreg);
-          if (!is_void) {
-            __ movsxb(valreg, valreg);
-          }
+          __ movsxb(valreg, valreg);
           break;
         case DataType::Type::kUint16:
           __ LockXaddw(field_addr, valreg);
-          if (!is_void) {
-            __ movzxw(valreg, valreg);
-          }
+          __ movzxw(valreg, valreg);
           break;
         case DataType::Type::kInt16:
           __ LockXaddw(field_addr, valreg);
-          if (!is_void) {
-            __ movsxw(valreg, valreg);
-          }
+          __ movsxw(valreg, valreg);
           break;
         case DataType::Type::kInt32:
         case DataType::Type::kUint32:
@@ -5011,10 +4939,9 @@ static void GenerateVarHandleGetAndUpdate(HInvoke* invoke,
   X86_64Assembler* assembler = codegen->GetAssembler();
   LocationSummary* locations = invoke->GetLocations();
 
-  // Get the type from the shorty as the invokes may not return a value.
   uint32_t number_of_arguments = invoke->GetNumberOfArguments();
   Location value = locations->InAt(number_of_arguments - 1);
-  DataType::Type type = GetDataTypeFromShorty(invoke, number_of_arguments - 1);
+  DataType::Type type = invoke->GetType();
 
   VarHandleSlowPathX86_64* slow_path = nullptr;
   VarHandleTarget target = GetVarHandleTarget(invoke);
@@ -5272,7 +5199,7 @@ void VarHandleSlowPathX86_64::EmitByteArrayViewCode(CodeGeneratorX86_64* codegen
   CpuRegister varhandle = locations->InAt(0).AsRegister<CpuRegister>();
   CpuRegister object = locations->InAt(1).AsRegister<CpuRegister>();
   CpuRegister index = locations->InAt(2).AsRegister<CpuRegister>();
-  CpuRegister temp = locations->GetTemp(locations->GetTempCount() - 1u).AsRegister<CpuRegister>();
+  CpuRegister temp = locations->GetTemp(locations->GetTempCount() - 1).AsRegister<CpuRegister>();
 
   MemberOffset class_offset = mirror::Object::ClassOffset();
   MemberOffset array_length_offset = mirror::Array::LengthOffset();