ART: Add (Fpu)RegHigh stack map location kinds

When running Optimized code on 64-bit, high value of vreg pair may be
stored in the high 32 bits of a CPU register. This is not reflected in
stack maps which would encode both the low and high vreg as
kInRegister with the same register number, making it indistinguishable
from two non-wide vregs with the same value in the lower 32 bits.

Deoptimization deals with this by running the verifier and thus
obtaining vreg pair information, but this would be too slow for try/
catch. This patch therefore adds two new stack map location kinds:
kInRegisterHigh and kInFpuRegisterHigh to differentiate between the
two cases.

Note that this also applies to floating-point registers on x86.

Change-Id: I15092323e56a661673e77bee1f0fca4261374732

diff --git a/compiler/optimizing/code_generator.cc b/compiler/optimizing/code_generator.cc
index 6568ea4..503187b 100644 (file)
--- a/compiler/optimizing/code_generator.cc
+++ b/compiler/optimizing/code_generator.cc
@@ -887,7 +887,7 @@ void CodeGenerator::EmitEnvironment(HEnvironment* environment, SlowPathCode* slo
         } else {
           stack_map_stream_.AddDexRegisterEntry(DexRegisterLocation::Kind::kInRegister, id);
           if (current->GetType() == Primitive::kPrimLong) {
-            stack_map_stream_.AddDexRegisterEntry(DexRegisterLocation::Kind::kInRegister, id);
+            stack_map_stream_.AddDexRegisterEntry(DexRegisterLocation::Kind::kInRegisterHigh, id);
             ++i;
             DCHECK_LT(i, environment_size);
           }
@@ -909,7 +909,8 @@ void CodeGenerator::EmitEnvironment(HEnvironment* environment, SlowPathCode* slo
         } else {
           stack_map_stream_.AddDexRegisterEntry(DexRegisterLocation::Kind::kInFpuRegister, id);
           if (current->GetType() == Primitive::kPrimDouble) {
-            stack_map_stream_.AddDexRegisterEntry(DexRegisterLocation::Kind::kInFpuRegister, id);
+            stack_map_stream_.AddDexRegisterEntry(
+                DexRegisterLocation::Kind::kInFpuRegisterHigh, id);
             ++i;
             DCHECK_LT(i, environment_size);
           }

diff --git a/compiler/optimizing/stack_map_test.cc b/compiler/optimizing/stack_map_test.cc
index 33207d9..c4a3b28 100644 (file)
--- a/compiler/optimizing/stack_map_test.cc
+++ b/compiler/optimizing/stack_map_test.cc
@@ -143,6 +143,22 @@ TEST(StackMapTest, Test2) {
   stream.AddDexRegisterEntry(Kind::kInFpuRegister, 3);   // Short location.
   stream.EndStackMapEntry();
 
+  ArenaBitVector sp_mask3(&arena, 0, true);
+  sp_mask3.SetBit(1);
+  sp_mask3.SetBit(5);
+  stream.BeginStackMapEntry(2, 192, 0xAB, &sp_mask3, number_of_dex_registers, 0);
+  stream.AddDexRegisterEntry(Kind::kInRegister, 6);       // Short location.
+  stream.AddDexRegisterEntry(Kind::kInRegisterHigh, 8);   // Short location.
+  stream.EndStackMapEntry();
+
+  ArenaBitVector sp_mask4(&arena, 0, true);
+  sp_mask4.SetBit(6);
+  sp_mask4.SetBit(7);
+  stream.BeginStackMapEntry(3, 256, 0xCD, &sp_mask4, number_of_dex_registers, 0);
+  stream.AddDexRegisterEntry(Kind::kInFpuRegister, 3);      // Short location, same in stack map 2.
+  stream.AddDexRegisterEntry(Kind::kInFpuRegisterHigh, 1);  // Short location.
+  stream.EndStackMapEntry();
+
   size_t size = stream.PrepareForFillIn();
   void* memory = arena.Alloc(size, kArenaAllocMisc);
   MemoryRegion region(memory, size);
@@ -151,15 +167,15 @@ TEST(StackMapTest, Test2) {
   CodeInfo code_info(region);
   StackMapEncoding encoding = code_info.ExtractEncoding();
   ASSERT_EQ(2u, encoding.NumberOfBytesForStackMask());
-  ASSERT_EQ(2u, code_info.GetNumberOfStackMaps());
+  ASSERT_EQ(4u, code_info.GetNumberOfStackMaps());
 
   uint32_t number_of_location_catalog_entries = code_info.GetNumberOfLocationCatalogEntries();
-  ASSERT_EQ(4u, number_of_location_catalog_entries);
+  ASSERT_EQ(7u, number_of_location_catalog_entries);
   DexRegisterLocationCatalog location_catalog = code_info.GetDexRegisterLocationCatalog(encoding);
   // The Dex register location catalog contains:
-  // - three 1-byte short Dex register locations, and
+  // - six 1-byte short Dex register locations, and
   // - one 5-byte large Dex register location.
-  size_t expected_location_catalog_size = 3u * 1u + 5u;
+  size_t expected_location_catalog_size = 6u * 1u + 5u;
   ASSERT_EQ(expected_location_catalog_size, location_catalog.Size());
 
   // First stack map.
@@ -278,6 +294,116 @@ TEST(StackMapTest, Test2) {
 
     ASSERT_FALSE(stack_map.HasInlineInfo(encoding));
   }
+
+  // Third stack map.
+  {
+    StackMap stack_map = code_info.GetStackMapAt(2, encoding);
+    ASSERT_TRUE(stack_map.Equals(code_info.GetStackMapForDexPc(2u, encoding)));
+    ASSERT_TRUE(stack_map.Equals(code_info.GetStackMapForNativePcOffset(192u, encoding)));
+    ASSERT_EQ(2u, stack_map.GetDexPc(encoding));
+    ASSERT_EQ(192u, stack_map.GetNativePcOffset(encoding));
+    ASSERT_EQ(0xABu, stack_map.GetRegisterMask(encoding));
+
+    MemoryRegion stack_mask = stack_map.GetStackMask(encoding);
+    ASSERT_TRUE(SameBits(stack_mask, sp_mask3));
+
+    ASSERT_TRUE(stack_map.HasDexRegisterMap(encoding));
+    DexRegisterMap dex_register_map =
+        code_info.GetDexRegisterMapOf(stack_map, encoding, number_of_dex_registers);
+    ASSERT_TRUE(dex_register_map.IsDexRegisterLive(0));
+    ASSERT_TRUE(dex_register_map.IsDexRegisterLive(1));
+    ASSERT_EQ(2u, dex_register_map.GetNumberOfLiveDexRegisters(number_of_dex_registers));
+    // The Dex register map contains:
+    // - one 1-byte live bit mask, and
+    // - one 1-byte set of location catalog entry indices composed of two 2-bit values.
+    size_t expected_dex_register_map_size = 1u + 1u;
+    ASSERT_EQ(expected_dex_register_map_size, dex_register_map.Size());
+
+    ASSERT_EQ(Kind::kInRegister, dex_register_map.GetLocationKind(
+                  0, number_of_dex_registers, code_info, encoding));
+    ASSERT_EQ(Kind::kInRegisterHigh, dex_register_map.GetLocationKind(
+                  1, number_of_dex_registers, code_info, encoding));
+    ASSERT_EQ(Kind::kInRegister, dex_register_map.GetLocationInternalKind(
+                  0, number_of_dex_registers, code_info, encoding));
+    ASSERT_EQ(Kind::kInRegisterHigh, dex_register_map.GetLocationInternalKind(
+                  1, number_of_dex_registers, code_info, encoding));
+    ASSERT_EQ(6, dex_register_map.GetMachineRegister(
+                  0, number_of_dex_registers, code_info, encoding));
+    ASSERT_EQ(8, dex_register_map.GetMachineRegister(
+                  1, number_of_dex_registers, code_info, encoding));
+
+    size_t index0 = dex_register_map.GetLocationCatalogEntryIndex(
+        0, number_of_dex_registers, number_of_location_catalog_entries);
+    size_t index1 = dex_register_map.GetLocationCatalogEntryIndex(
+        1, number_of_dex_registers, number_of_location_catalog_entries);
+    ASSERT_EQ(4u, index0);
+    ASSERT_EQ(5u, index1);
+    DexRegisterLocation location0 = location_catalog.GetDexRegisterLocation(index0);
+    DexRegisterLocation location1 = location_catalog.GetDexRegisterLocation(index1);
+    ASSERT_EQ(Kind::kInRegister, location0.GetKind());
+    ASSERT_EQ(Kind::kInRegisterHigh, location1.GetKind());
+    ASSERT_EQ(Kind::kInRegister, location0.GetInternalKind());
+    ASSERT_EQ(Kind::kInRegisterHigh, location1.GetInternalKind());
+    ASSERT_EQ(6, location0.GetValue());
+    ASSERT_EQ(8, location1.GetValue());
+
+    ASSERT_FALSE(stack_map.HasInlineInfo(encoding));
+  }
+
+  // Fourth stack map.
+  {
+    StackMap stack_map = code_info.GetStackMapAt(3, encoding);
+    ASSERT_TRUE(stack_map.Equals(code_info.GetStackMapForDexPc(3u, encoding)));
+    ASSERT_TRUE(stack_map.Equals(code_info.GetStackMapForNativePcOffset(256u, encoding)));
+    ASSERT_EQ(3u, stack_map.GetDexPc(encoding));
+    ASSERT_EQ(256u, stack_map.GetNativePcOffset(encoding));
+    ASSERT_EQ(0xCDu, stack_map.GetRegisterMask(encoding));
+
+    MemoryRegion stack_mask = stack_map.GetStackMask(encoding);
+    ASSERT_TRUE(SameBits(stack_mask, sp_mask4));
+
+    ASSERT_TRUE(stack_map.HasDexRegisterMap(encoding));
+    DexRegisterMap dex_register_map =
+        code_info.GetDexRegisterMapOf(stack_map, encoding, number_of_dex_registers);
+    ASSERT_TRUE(dex_register_map.IsDexRegisterLive(0));
+    ASSERT_TRUE(dex_register_map.IsDexRegisterLive(1));
+    ASSERT_EQ(2u, dex_register_map.GetNumberOfLiveDexRegisters(number_of_dex_registers));
+    // The Dex register map contains:
+    // - one 1-byte live bit mask, and
+    // - one 1-byte set of location catalog entry indices composed of two 2-bit values.
+    size_t expected_dex_register_map_size = 1u + 1u;
+    ASSERT_EQ(expected_dex_register_map_size, dex_register_map.Size());
+
+    ASSERT_EQ(Kind::kInFpuRegister, dex_register_map.GetLocationKind(
+                  0, number_of_dex_registers, code_info, encoding));
+    ASSERT_EQ(Kind::kInFpuRegisterHigh, dex_register_map.GetLocationKind(
+                  1, number_of_dex_registers, code_info, encoding));
+    ASSERT_EQ(Kind::kInFpuRegister, dex_register_map.GetLocationInternalKind(
+                  0, number_of_dex_registers, code_info, encoding));
+    ASSERT_EQ(Kind::kInFpuRegisterHigh, dex_register_map.GetLocationInternalKind(
+                  1, number_of_dex_registers, code_info, encoding));
+    ASSERT_EQ(3, dex_register_map.GetMachineRegister(
+                  0, number_of_dex_registers, code_info, encoding));
+    ASSERT_EQ(1, dex_register_map.GetMachineRegister(
+                  1, number_of_dex_registers, code_info, encoding));
+
+    size_t index0 = dex_register_map.GetLocationCatalogEntryIndex(
+        0, number_of_dex_registers, number_of_location_catalog_entries);
+    size_t index1 = dex_register_map.GetLocationCatalogEntryIndex(
+        1, number_of_dex_registers, number_of_location_catalog_entries);
+    ASSERT_EQ(3u, index0);  // Shared with second stack map.
+    ASSERT_EQ(6u, index1);
+    DexRegisterLocation location0 = location_catalog.GetDexRegisterLocation(index0);
+    DexRegisterLocation location1 = location_catalog.GetDexRegisterLocation(index1);
+    ASSERT_EQ(Kind::kInFpuRegister, location0.GetKind());
+    ASSERT_EQ(Kind::kInFpuRegisterHigh, location1.GetKind());
+    ASSERT_EQ(Kind::kInFpuRegister, location0.GetInternalKind());
+    ASSERT_EQ(Kind::kInFpuRegisterHigh, location1.GetInternalKind());
+    ASSERT_EQ(3, location0.GetValue());
+    ASSERT_EQ(1, location1.GetValue());
+
+    ASSERT_FALSE(stack_map.HasInlineInfo(encoding));
+  }
 }
 
 TEST(StackMapTest, TestNonLiveDexRegisters) {

diff --git a/runtime/check_reference_map_visitor.h b/runtime/check_reference_map_visitor.h
index 3155b51..7965cd7 100644 (file)
--- a/runtime/check_reference_map_visitor.h
+++ b/runtime/check_reference_map_visitor.h
@@ -87,9 +87,11 @@ class CheckReferenceMapVisitor : public StackVisitor {
           CHECK(stack_mask.LoadBit(location.GetValue() / kFrameSlotSize));
           break;
         case DexRegisterLocation::Kind::kInRegister:
+        case DexRegisterLocation::Kind::kInRegisterHigh:
           CHECK_NE(register_mask & (1 << location.GetValue()), 0u);
           break;
         case DexRegisterLocation::Kind::kInFpuRegister:
+        case DexRegisterLocation::Kind::kInFpuRegisterHigh:
           // In Fpu register, should not be a reference.
           CHECK(false);
           break;

diff --git a/runtime/stack.cc b/runtime/stack.cc
index b07b244..a765a3f 100644 (file)
--- a/runtime/stack.cc
+++ b/runtime/stack.cc
@@ -299,7 +299,9 @@ bool StackVisitor::GetVRegFromOptimizedCode(ArtMethod* m, uint16_t vreg, VRegKin
       return true;
     }
     case DexRegisterLocation::Kind::kInRegister:
-    case DexRegisterLocation::Kind::kInFpuRegister: {
+    case DexRegisterLocation::Kind::kInRegisterHigh:
+    case DexRegisterLocation::Kind::kInFpuRegister:
+    case DexRegisterLocation::Kind::kInFpuRegisterHigh: {
       uint32_t reg =
           dex_register_map.GetMachineRegister(vreg, number_of_dex_registers, code_info, encoding);
       return GetRegisterIfAccessible(reg, kind, val);

diff --git a/runtime/stack_map.h b/runtime/stack_map.h
index 0d3816b..07b79b5 100644 (file)
--- a/runtime/stack_map.h
+++ b/runtime/stack_map.h
@@ -59,26 +59,33 @@ class DexRegisterLocation {
   /*
    * The location kind used to populate the Dex register information in a
    * StackMapStream can either be:
-   * - kNone: the register has no location yet, meaning it has not been set;
+   * - kStack: vreg stored on the stack, value holds the stack offset;
+   * - kInRegister: vreg stored in low 32 bits of a core physical register,
+   *                value holds the register number;
+   * - kInRegisterHigh: vreg stored in high 32 bits of a core physical register,
+   *                    value holds the register number;
+   * - kInFpuRegister: vreg stored in low 32 bits of an FPU register,
+   *                   value holds the register number;
+   * - kInFpuRegisterHigh: vreg stored in high 32 bits of an FPU register,
+   *                       value holds the register number;
    * - kConstant: value holds the constant;
-   * - kStack: value holds the stack offset;
-   * - kRegister: value holds the physical register number;
-   * - kFpuRegister: value holds the physical register number.
    *
    * In addition, DexRegisterMap also uses these values:
    * - kInStackLargeOffset: value holds a "large" stack offset (greater than
    *   or equal to 128 bytes);
    * - kConstantLargeValue: value holds a "large" constant (lower than 0, or
-   *   or greater than or equal to 32).
+   *   or greater than or equal to 32);
+   * - kNone: the register has no location, meaning it has not been set.
    */
   enum class Kind : uint8_t {
     // Short location kinds, for entries fitting on one byte (3 bits
     // for the kind, 5 bits for the value) in a DexRegisterMap.
-    kNone = 0,                // 0b000
-    kInStack = 1,             // 0b001
-    kInRegister = 2,          // 0b010
+    kInStack = 0,             // 0b000
+    kInRegister = 1,          // 0b001
+    kInRegisterHigh = 2,      // 0b010
     kInFpuRegister = 3,       // 0b011
-    kConstant = 4,            // 0b100
+    kInFpuRegisterHigh = 4,   // 0b100
+    kConstant = 5,            // 0b101
 
     // Large location kinds, requiring a 5-byte encoding (1 byte for the
     // kind, 4 bytes for the value).
@@ -87,11 +94,14 @@ class DexRegisterLocation {
     // divided by the stack frame slot size (4 bytes) cannot fit on a
     // 5-bit unsigned integer (i.e., this offset value is greater than
     // or equal to 2^5 * 4 = 128 bytes).
-    kInStackLargeOffset = 5,  // 0b101
+    kInStackLargeOffset = 6,  // 0b110
 
     // Large constant, that cannot fit on a 5-bit signed integer (i.e.,
     // lower than 0, or greater than or equal to 2^5 = 32).
-    kConstantLargeValue = 6,  // 0b110
+    kConstantLargeValue = 7,  // 0b111
+
+    // Entries with no location are not stored and do not need own marker.
+    kNone = static_cast<uint8_t>(-1),
 
     kLastLocationKind = kConstantLargeValue
   };
@@ -108,25 +118,29 @@ class DexRegisterLocation {
         return "in stack";
       case Kind::kInRegister:
         return "in register";
+      case Kind::kInRegisterHigh:
+        return "in register high";
       case Kind::kInFpuRegister:
         return "in fpu register";
+      case Kind::kInFpuRegisterHigh:
+        return "in fpu register high";
       case Kind::kConstant:
         return "as constant";
       case Kind::kInStackLargeOffset:
         return "in stack (large offset)";
       case Kind::kConstantLargeValue:
         return "as constant (large value)";
-      default:
-        UNREACHABLE();
     }
+    UNREACHABLE();
   }
 
   static bool IsShortLocationKind(Kind kind) {
     switch (kind) {
-      case Kind::kNone:
       case Kind::kInStack:
       case Kind::kInRegister:
+      case Kind::kInRegisterHigh:
       case Kind::kInFpuRegister:
+      case Kind::kInFpuRegisterHigh:
       case Kind::kConstant:
         return true;
 
@@ -134,9 +148,10 @@ class DexRegisterLocation {
       case Kind::kConstantLargeValue:
         return false;
 
-      default:
-        UNREACHABLE();
+      case Kind::kNone:
+        LOG(FATAL) << "Unexpected location kind " << PrettyDescriptor(kind);
     }
+    UNREACHABLE();
   }
 
   // Convert `kind` to a "surface" kind, i.e. one that doesn't include
@@ -144,10 +159,11 @@ class DexRegisterLocation {
   // TODO: Introduce another enum type for the surface kind?
   static Kind ConvertToSurfaceKind(Kind kind) {
     switch (kind) {
-      case Kind::kNone:
       case Kind::kInStack:
       case Kind::kInRegister:
+      case Kind::kInRegisterHigh:
       case Kind::kInFpuRegister:
+      case Kind::kInFpuRegisterHigh:
       case Kind::kConstant:
         return kind;
 
@@ -157,9 +173,10 @@ class DexRegisterLocation {
       case Kind::kConstantLargeValue:
         return Kind::kConstant;
 
-      default:
-        UNREACHABLE();
+      case Kind::kNone:
+        return kind;
     }
+    UNREACHABLE();
   }
 
   // Required by art::StackMapStream::LocationCatalogEntriesIndices.
@@ -305,55 +322,60 @@ class DexRegisterLocationCatalog {
 
   // Compute the compressed kind of `location`.
   static DexRegisterLocation::Kind ComputeCompressedKind(const DexRegisterLocation& location) {
-    switch (location.GetInternalKind()) {
-      case DexRegisterLocation::Kind::kNone:
-        DCHECK_EQ(location.GetValue(), 0);
-        return DexRegisterLocation::Kind::kNone;
+    DexRegisterLocation::Kind kind = location.GetInternalKind();
+    switch (kind) {
+      case DexRegisterLocation::Kind::kInStack:
+        return IsShortStackOffsetValue(location.GetValue())
+            ? DexRegisterLocation::Kind::kInStack
+            : DexRegisterLocation::Kind::kInStackLargeOffset;
 
       case DexRegisterLocation::Kind::kInRegister:
+      case DexRegisterLocation::Kind::kInRegisterHigh:
         DCHECK_GE(location.GetValue(), 0);
         DCHECK_LT(location.GetValue(), 1 << kValueBits);
-        return DexRegisterLocation::Kind::kInRegister;
+        return kind;
 
       case DexRegisterLocation::Kind::kInFpuRegister:
+      case DexRegisterLocation::Kind::kInFpuRegisterHigh:
         DCHECK_GE(location.GetValue(), 0);
         DCHECK_LT(location.GetValue(), 1 << kValueBits);
-        return DexRegisterLocation::Kind::kInFpuRegister;
-
-      case DexRegisterLocation::Kind::kInStack:
-        return IsShortStackOffsetValue(location.GetValue())
-            ? DexRegisterLocation::Kind::kInStack
-            : DexRegisterLocation::Kind::kInStackLargeOffset;
+        return kind;
 
       case DexRegisterLocation::Kind::kConstant:
         return IsShortConstantValue(location.GetValue())
             ? DexRegisterLocation::Kind::kConstant
             : DexRegisterLocation::Kind::kConstantLargeValue;
 
-      default:
-        LOG(FATAL) << "Unexpected location kind"
-                   << DexRegisterLocation::PrettyDescriptor(location.GetInternalKind());
-        UNREACHABLE();
+      case DexRegisterLocation::Kind::kConstantLargeValue:
+      case DexRegisterLocation::Kind::kInStackLargeOffset:
+      case DexRegisterLocation::Kind::kNone:
+        LOG(FATAL) << "Unexpected location kind " << DexRegisterLocation::PrettyDescriptor(kind);
     }
+    UNREACHABLE();
   }
 
   // Can `location` be turned into a short location?
   static bool CanBeEncodedAsShortLocation(const DexRegisterLocation& location) {
-    switch (location.GetInternalKind()) {
-      case DexRegisterLocation::Kind::kNone:
+    DexRegisterLocation::Kind kind = location.GetInternalKind();
+    switch (kind) {
+      case DexRegisterLocation::Kind::kInStack:
+        return IsShortStackOffsetValue(location.GetValue());
+
       case DexRegisterLocation::Kind::kInRegister:
+      case DexRegisterLocation::Kind::kInRegisterHigh:
       case DexRegisterLocation::Kind::kInFpuRegister:
+      case DexRegisterLocation::Kind::kInFpuRegisterHigh:
         return true;
 
-      case DexRegisterLocation::Kind::kInStack:
-        return IsShortStackOffsetValue(location.GetValue());
-
       case DexRegisterLocation::Kind::kConstant:
         return IsShortConstantValue(location.GetValue());
 
-      default:
-        UNREACHABLE();
+      case DexRegisterLocation::Kind::kConstantLargeValue:
+      case DexRegisterLocation::Kind::kInStackLargeOffset:
+      case DexRegisterLocation::Kind::kNone:
+        LOG(FATAL) << "Unexpected location kind " << DexRegisterLocation::PrettyDescriptor(kind);
     }
+    UNREACHABLE();
   }
 
   static size_t EntrySize(const DexRegisterLocation& location) {
@@ -501,8 +523,10 @@ class DexRegisterMap {
                              const StackMapEncoding& enc) const {
     DexRegisterLocation location =
         GetDexRegisterLocation(dex_register_number, number_of_dex_registers, code_info, enc);
-    DCHECK(location.GetInternalKind() == DexRegisterLocation::Kind::kInRegister
-           || location.GetInternalKind() == DexRegisterLocation::Kind::kInFpuRegister)
+    DCHECK(location.GetInternalKind() == DexRegisterLocation::Kind::kInRegister ||
+           location.GetInternalKind() == DexRegisterLocation::Kind::kInRegisterHigh ||
+           location.GetInternalKind() == DexRegisterLocation::Kind::kInFpuRegister ||
+           location.GetInternalKind() == DexRegisterLocation::Kind::kInFpuRegisterHigh)
         << DexRegisterLocation::PrettyDescriptor(location.GetInternalKind());
     return location.GetValue();
   }