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, 37 insertions, 125 deletions

diff --git a/compiler/optimizing/intrinsics_x86.cc b/compiler/optimizing/intrinsics_x86.cc
index dc88de474c..2b83ba8349 100644
--- a/compiler/optimizing/intrinsics_x86.cc
+++ b/compiler/optimizing/intrinsics_x86.cc
@@ -34,7 +34,6 @@
 #include "mirror/reference.h"
 #include "mirror/string.h"
 #include "mirror/var_handle.h"
-#include "optimizing/data_type.h"
 #include "scoped_thread_state_change-inl.h"
 #include "thread-current-inl.h"
 #include "utils/x86/assembler_x86.h"
@@ -2490,7 +2489,6 @@ void CreateUnsafeGetAndUpdateLocations(ArenaAllocator* allocator,
     locations->SetCustomSlowPathCallerSaves(RegisterSet::Empty());  // No caller-save registers.
   }
   locations->SetInAt(0, Location::NoLocation());        // Unused receiver.
-  const bool is_void = invoke->GetType() == DataType::Type::kVoid;
   if (type == DataType::Type::kInt64) {
     // Explicitly allocate all registers.
     locations->SetInAt(1, Location::RegisterLocation(EBP));
@@ -2504,23 +2502,14 @@ void CreateUnsafeGetAndUpdateLocations(ArenaAllocator* allocator,
       locations->SetInAt(2, Location::RegisterPairLocation(ESI, EDI));
       locations->SetInAt(3, Location::RegisterPairLocation(EBX, ECX));
     }
-    if (is_void) {
-      locations->AddTemp(Location::RegisterLocation(EAX));
-      locations->AddTemp(Location::RegisterLocation(EDX));
-    } else {
-      locations->SetOut(Location::RegisterPairLocation(EAX, EDX), Location::kOutputOverlap);
-    }
+    locations->SetOut(Location::RegisterPairLocation(EAX, EDX), Location::kOutputOverlap);
   } else {
     locations->SetInAt(1, Location::RequiresRegister());
     locations->SetInAt(2, Location::RequiresRegister());
     // Use the same register for both the output and the new value or addend
     // to take advantage of XCHG or XADD. Arbitrarily pick EAX.
     locations->SetInAt(3, Location::RegisterLocation(EAX));
-    // Only set the `out` register if it's needed. In the void case we can still use EAX in the
-    // same manner as it is used an as `in` register.
-    if (!is_void) {
-      locations->SetOut(Location::RegisterLocation(EAX));
-    }
+    locations->SetOut(Location::RegisterLocation(EAX));
   }
 }
 
@@ -2584,20 +2573,14 @@ static void GenUnsafeGetAndUpdate(HInvoke* invoke,
   X86Assembler* assembler = down_cast<X86Assembler*>(codegen->GetAssembler());
   LocationSummary* locations = invoke->GetLocations();
 
-  const bool is_void = invoke->GetType() == DataType::Type::kVoid;
-  // We use requested specific registers to use as temps for void methods, as we don't return the
-  // value.
-  Location out_or_temp =
-      is_void ? (type == DataType::Type::kInt64 ? Location::RegisterPairLocation(EAX, EDX) :
-                                                  Location::RegisterLocation(EAX)) :
-                locations->Out();
+  Location out = locations->Out();                            // Result.
   Register base = locations->InAt(1).AsRegister<Register>();  // Object pointer.
   Location offset = locations->InAt(2);                       // Long offset.
   Location arg = locations->InAt(3);                          // New value or addend.
 
   if (type == DataType::Type::kInt32) {
-    DCHECK(out_or_temp.Equals(arg));
-    Register out_reg = out_or_temp.AsRegister<Register>();
+    DCHECK(out.Equals(arg));
+    Register out_reg = out.AsRegister<Register>();
     Address field_address(base, offset.AsRegisterPairLow<Register>(), TIMES_1, 0);
     if (get_and_update_op == GetAndUpdateOp::kAdd) {
       __ LockXaddl(field_address, out_reg);
@@ -2638,7 +2621,7 @@ static void GenUnsafeGetAndUpdate(HInvoke* invoke,
   } else {
     DCHECK_EQ(type, DataType::Type::kReference);
     DCHECK(get_and_update_op == GetAndUpdateOp::kSet);
-    Register out_reg = out_or_temp.AsRegister<Register>();
+    Register out_reg = out.AsRegister<Register>();
     Address field_address(base, offset.AsRegisterPairLow<Register>(), TIMES_1, 0);
     Register temp1 = locations->GetTemp(0).AsRegister<Register>();
     Register temp2 = locations->GetTemp(1).AsRegister<Register>();
@@ -2667,10 +2650,8 @@ static void GenUnsafeGetAndUpdate(HInvoke* invoke,
       __ movl(temp1, out_reg);
       __ PoisonHeapReference(temp1);
       __ xchgl(temp1, field_address);
-      if (!is_void) {
-        __ UnpoisonHeapReference(temp1);
-        __ movl(out_reg, temp1);
-      }
+      __ UnpoisonHeapReference(temp1);
+      __ movl(out_reg, temp1);
     } else {
       __ xchgl(out_reg, field_address);
     }
@@ -4256,13 +4237,9 @@ 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 value_index = number_of_arguments - 1;
   DataType::Type value_type = GetDataTypeFromShorty(invoke, 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);
 
   if (DataType::Is64BitType(value_type)) {
     // We avoid the case of an Int64/Float64 value because we would need to place it in a register
@@ -4289,17 +4266,10 @@ static void CreateVarHandleGetAndSetLocations(HInvoke* invoke, CodeGeneratorX86*
   if (value_type == DataType::Type::kFloat32) {
     locations->AddTemp(Location::RegisterLocation(EAX));
     locations->SetInAt(value_index, Location::FpuRegisterOrConstant(invoke->InputAt(value_index)));
-    // Only set the `out` register if it's needed. In the void case, we will not use `out`.
-    if (!is_void) {
-      locations->SetOut(Location::RequiresFpuRegister());
-    }
+    locations->SetOut(Location::RequiresFpuRegister());
   } else {
     locations->SetInAt(value_index, Location::RegisterLocation(EAX));
-    // Only set the `out` register if it's needed. In the void case we can still use EAX in the
-    // same manner as it is used an as `in` register.
-    if (!is_void) {
-      locations->SetOut(Location::RegisterLocation(EAX));
-    }
+    locations->SetOut(Location::RegisterLocation(EAX));
   }
 }
 
@@ -4313,7 +4283,6 @@ static void GenerateVarHandleGetAndSet(HInvoke* invoke, CodeGeneratorX86* codege
   // The value we want to set is the last argument
   uint32_t value_index = invoke->GetNumberOfArguments() - 1;
   Location value = locations->InAt(value_index);
-  // Get the type from the shorty as the invokes may not return a value.
   DataType::Type value_type = GetDataTypeFromShorty(invoke, value_index);
   Register temp = locations->GetTemp(1).AsRegister<Register>();
   Register temp2 = locations->GetTemp(2).AsRegister<Register>();
@@ -4338,36 +4307,24 @@ static void GenerateVarHandleGetAndSet(HInvoke* invoke, CodeGeneratorX86* codege
   // fields the object is in a separate register, it is safe to use the first temporary register.
   temp = expected_coordinates_count == 1u ? temp : locations->GetTemp(3).AsRegister<Register>();
   // No need for a lock prefix. `xchg` has an implicit lock when it is used with an address.
-
-  DataType::Type return_type = invoke->GetType();
-  const bool is_void = return_type == DataType::Type::kVoid;
-  DCHECK_IMPLIES(!is_void, return_type == value_type);
   switch (value_type) {
     case DataType::Type::kBool:
       __ xchgb(value.AsRegister<ByteRegister>(), field_addr);
-      if (!is_void) {
-        __ movzxb(locations->Out().AsRegister<Register>(),
-                  locations->Out().AsRegister<ByteRegister>());
-      }
+      __ movzxb(locations->Out().AsRegister<Register>(),
+                locations->Out().AsRegister<ByteRegister>());
       break;
     case DataType::Type::kInt8:
       __ xchgb(value.AsRegister<ByteRegister>(), field_addr);
-      if (!is_void) {
-        __ movsxb(locations->Out().AsRegister<Register>(),
-                  locations->Out().AsRegister<ByteRegister>());
-      }
+      __ movsxb(locations->Out().AsRegister<Register>(),
+                locations->Out().AsRegister<ByteRegister>());
       break;
     case DataType::Type::kUint16:
       __ xchgw(value.AsRegister<Register>(), field_addr);
-      if (!is_void) {
-        __ movzxw(locations->Out().AsRegister<Register>(), locations->Out().AsRegister<Register>());
-      }
+      __ movzxw(locations->Out().AsRegister<Register>(), locations->Out().AsRegister<Register>());
       break;
     case DataType::Type::kInt16:
       __ xchgw(value.AsRegister<Register>(), field_addr);
-      if (!is_void) {
-        __ movsxw(locations->Out().AsRegister<Register>(), locations->Out().AsRegister<Register>());
-      }
+      __ movsxw(locations->Out().AsRegister<Register>(), locations->Out().AsRegister<Register>());
       break;
     case DataType::Type::kInt32:
       __ xchgl(value.AsRegister<Register>(), field_addr);
@@ -4375,9 +4332,7 @@ static void GenerateVarHandleGetAndSet(HInvoke* invoke, CodeGeneratorX86* codege
     case DataType::Type::kFloat32:
       codegen->Move32(Location::RegisterLocation(EAX), value);
       __ xchgl(EAX, field_addr);
-      if (!is_void) {
-        __ movd(locations->Out().AsFpuRegister<XmmRegister>(), EAX);
-      }
+      __ movd(locations->Out().AsFpuRegister<XmmRegister>(), EAX);
       break;
     case DataType::Type::kReference: {
       if (codegen->EmitBakerReadBarrier()) {
@@ -4398,13 +4353,10 @@ static void GenerateVarHandleGetAndSet(HInvoke* invoke, CodeGeneratorX86* codege
         __ movl(temp, value.AsRegister<Register>());
         __ PoisonHeapReference(temp);
         __ xchgl(temp, field_addr);
-        if (!is_void) {
-          __ UnpoisonHeapReference(temp);
-          __ movl(locations->Out().AsRegister<Register>(), temp);
-        }
+        __ UnpoisonHeapReference(temp);
+        __ movl(locations->Out().AsRegister<Register>(), temp);
       } else {
-        DCHECK_IMPLIES(!is_void, locations->Out().Equals(Location::RegisterLocation(EAX)));
-        __ xchgl(Location::RegisterLocation(EAX).AsRegister<Register>(), field_addr);
+        __ xchgl(locations->Out().AsRegister<Register>(), field_addr);
       }
       break;
     }
@@ -4638,7 +4590,6 @@ static void CreateVarHandleGetAndAddLocations(HInvoke* invoke, CodeGeneratorX86*
     return;
   }
 
-  // Get the type from the shorty as the invokes may not return a value.
   // The last argument should be the value we intend to set.
   uint32_t value_index = invoke->GetNumberOfArguments() - 1;
   DataType::Type value_type = GetDataTypeFromShorty(invoke, value_index);
@@ -4664,26 +4615,15 @@ static void CreateVarHandleGetAndAddLocations(HInvoke* invoke, CodeGeneratorX86*
     locations->AddTemp(Location::RequiresRegister());
   }
 
-  DataType::Type return_type = invoke->GetType();
-  const bool is_void = return_type == DataType::Type::kVoid;
-  DCHECK_IMPLIES(!is_void, return_type == value_type);
-
   if (DataType::IsFloatingPointType(value_type)) {
     locations->AddTemp(Location::RequiresFpuRegister());
     locations->AddTemp(Location::RegisterLocation(EAX));
     locations->SetInAt(value_index, Location::RequiresFpuRegister());
-    // Only set the `out` register if it's needed. In the void case, we do not use `out`.
-    if (!is_void) {
-      locations->SetOut(Location::RequiresFpuRegister());
-    }
+    locations->SetOut(Location::RequiresFpuRegister());
   } else {
     // xadd updates the register argument with the old value. ByteRegister required for xaddb.
     locations->SetInAt(value_index, Location::RegisterLocation(EAX));
-    // Only set the `out` register if it's needed. In the void case we can still use EAX in the
-    // same manner as it is used an as `in` register.
-    if (!is_void) {
-      locations->SetOut(Location::RegisterLocation(EAX));
-    }
+    locations->SetOut(Location::RegisterLocation(EAX));
   }
 }
 
@@ -4696,11 +4636,8 @@ static void GenerateVarHandleGetAndAdd(HInvoke* invoke, CodeGeneratorX86* codege
   LocationSummary* locations = invoke->GetLocations();
   uint32_t number_of_arguments = invoke->GetNumberOfArguments();
   uint32_t value_index = number_of_arguments - 1;
-  // Get the type from the shorty as the invokes may not return a value.
   DataType::Type type = GetDataTypeFromShorty(invoke, value_index);
-  DataType::Type return_type = invoke->GetType();
-  const bool is_void = return_type == DataType::Type::kVoid;
-  DCHECK_IMPLIES(!is_void, return_type == type);
+  DCHECK_EQ(type, invoke->GetType());
   Location value_loc = locations->InAt(value_index);
   Register temp = locations->GetTemp(0).AsRegister<Register>();
   SlowPathCode* slow_path = new (codegen->GetScopedAllocator()) IntrinsicSlowPathX86(invoke);
@@ -4722,22 +4659,16 @@ static void GenerateVarHandleGetAndAdd(HInvoke* invoke, CodeGeneratorX86* codege
   switch (type) {
     case DataType::Type::kInt8:
       __ LockXaddb(field_addr, value_loc.AsRegister<ByteRegister>());
-      if (!is_void) {
-        __ movsxb(locations->Out().AsRegister<Register>(),
-                  locations->Out().AsRegister<ByteRegister>());
-      }
+      __ movsxb(locations->Out().AsRegister<Register>(),
+                locations->Out().AsRegister<ByteRegister>());
       break;
     case DataType::Type::kInt16:
       __ LockXaddw(field_addr, value_loc.AsRegister<Register>());
-      if (!is_void) {
-        __ movsxw(locations->Out().AsRegister<Register>(), locations->Out().AsRegister<Register>());
-      }
+      __ movsxw(locations->Out().AsRegister<Register>(), locations->Out().AsRegister<Register>());
       break;
     case DataType::Type::kUint16:
       __ LockXaddw(field_addr, value_loc.AsRegister<Register>());
-      if (!is_void) {
-        __ movzxw(locations->Out().AsRegister<Register>(), locations->Out().AsRegister<Register>());
-      }
+      __ movzxw(locations->Out().AsRegister<Register>(), locations->Out().AsRegister<Register>());
       break;
     case DataType::Type::kInt32:
       __ LockXaddl(field_addr, value_loc.AsRegister<Register>());
@@ -4762,10 +4693,8 @@ static void GenerateVarHandleGetAndAdd(HInvoke* invoke, CodeGeneratorX86* codege
                                 temp);
       __ j(kNotZero, &try_again);
 
-      if (!is_void) {
-        // The old value is present in EAX.
-        codegen->Move32(locations->Out(), eax);
-      }
+      // The old value is present in EAX.
+      codegen->Move32(locations->Out(), eax);
       break;
     }
     default:
@@ -4811,11 +4740,9 @@ static void CreateVarHandleGetAndBitwiseOpLocations(HInvoke* invoke, CodeGenerat
     return;
   }
 
-  // Get the type from the shorty as the invokes may not return a value.
   // The last argument should be the value we intend to set.
   uint32_t value_index = invoke->GetNumberOfArguments() - 1;
-  DataType::Type value_type = GetDataTypeFromShorty(invoke, value_index);
-  if (DataType::Is64BitType(value_type)) {
+  if (DataType::Is64BitType(GetDataTypeFromShorty(invoke, value_index))) {
     // We avoid the case of an Int64 value because we would need to place it in a register pair.
     // If the slow path is taken, the ParallelMove might fail to move the pair according to the
     // X86DexCallingConvention in case of an overlap (e.g., move the 64 bit value from
@@ -4840,18 +4767,7 @@ static void CreateVarHandleGetAndBitwiseOpLocations(HInvoke* invoke, CodeGenerat
   }
 
   locations->SetInAt(value_index, Location::RegisterOrConstant(invoke->InputAt(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);
-  if (is_void) {
-    // 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(EAX));
-  } else {
-    locations->SetOut(Location::RegisterLocation(EAX));
-  }
+  locations->SetOut(Location::RegisterLocation(EAX));
 }
 
 static void GenerateBitwiseOp(HInvoke* invoke,
@@ -4889,12 +4805,9 @@ static void GenerateVarHandleGetAndBitwiseOp(HInvoke* invoke, CodeGeneratorX86*
 
   X86Assembler* assembler = codegen->GetAssembler();
   LocationSummary* locations = invoke->GetLocations();
-  // Get the type from the shorty as the invokes may not return a value.
   uint32_t value_index = invoke->GetNumberOfArguments() - 1;
   DataType::Type type = GetDataTypeFromShorty(invoke, value_index);
-  DataType::Type return_type = invoke->GetType();
-  const bool is_void = return_type == DataType::Type::kVoid;
-  DCHECK_IMPLIES(!is_void, return_type == type);
+  DCHECK_EQ(type, invoke->GetType());
   Register temp = locations->GetTemp(0).AsRegister<Register>();
   SlowPathCode* slow_path = new (codegen->GetScopedAllocator()) IntrinsicSlowPathX86(invoke);
   codegen->AddSlowPath(slow_path);
@@ -4913,9 +4826,8 @@ static void GenerateVarHandleGetAndBitwiseOp(HInvoke* invoke, CodeGeneratorX86*
   DCHECK_NE(temp, reference);
   Address field_addr(reference, offset, TIMES_1, 0);
 
-  Location eax_loc = Location::RegisterLocation(EAX);
-  Register eax = eax_loc.AsRegister<Register>();
-  DCHECK_IMPLIES(!is_void, locations->Out().Equals(eax_loc));
+  Register out = locations->Out().AsRegister<Register>();
+  DCHECK_EQ(out, EAX);
 
   if (invoke->GetIntrinsic() == Intrinsics::kVarHandleGetAndBitwiseOrRelease ||
       invoke->GetIntrinsic() == Intrinsics::kVarHandleGetAndBitwiseXorRelease ||
@@ -4926,12 +4838,12 @@ static void GenerateVarHandleGetAndBitwiseOp(HInvoke* invoke, CodeGeneratorX86*
   NearLabel try_again;
   __ Bind(&try_again);
   // Place the expected value in EAX for cmpxchg
-  codegen->LoadFromMemoryNoBarrier(type, eax_loc, field_addr);
+  codegen->LoadFromMemoryNoBarrier(type, locations->Out(), field_addr);
   codegen->Move32(locations->GetTemp(0), locations->InAt(value_index));
-  GenerateBitwiseOp(invoke, codegen, temp, eax);
+  GenerateBitwiseOp(invoke, codegen, temp, out);
   GenPrimitiveLockedCmpxchg(type,
                             codegen,
-                            /* expected_value= */ eax_loc,
+                            /* expected_value= */ locations->Out(),
                             /* new_value= */ locations->GetTemp(0),
                             reference,
                             offset);