8254793: [JVMCI] improve speculation encoding

Reviewed-by: kvn, dlong, never

src/hotspot/share/jvmci/jvmciRuntime.cpp

@@ -645,10 +645,11 @@ void JVMCINMethodData::initialize(
 }
 
 void JVMCINMethodData::add_failed_speculation(nmethod* nm, jlong speculation) {
-  uint index = (speculation >> 32) & 0xFFFFFFFF;
+  jlong index = speculation >> JVMCINMethodData::SPECULATION_LENGTH_BITS;
-  int length = (int) speculation;
+  guarantee(index >= 0 && index <= max_jint, "Encoded JVMCI speculation index is not a positive Java int: " INTPTR_FORMAT, index);
+  int length = speculation & JVMCINMethodData::SPECULATION_LENGTH_MASK;
   if (index + length > (uint) nm->speculations_size()) {
-    fatal(INTPTR_FORMAT "[index: %d, length: %d] out of bounds wrt encoded speculations of length %u", speculation, index, length, nm->speculations_size());
+    fatal(INTPTR_FORMAT "[index: " JLONG_FORMAT ", length: %d out of bounds wrt encoded speculations of length %u", speculation, index, length, nm->speculations_size());
   }
   address data = nm->speculations_begin() + index;
   FailedSpeculation::add_failed_speculation(nm, _failed_speculations, data, length);

src/hotspot/share/jvmci/jvmciRuntime.hpp

@@ -39,6 +39,7 @@ class MetadataHandles;
 // JVMCINMethodData objects are inlined into nmethods
 // at nmethod::_jvmci_data_offset.
 class JVMCINMethodData {
+  friend class JVMCIVMStructs;
   // Index for the HotSpotNmethod mirror in the nmethod's oops table.
   // This is -1 if there is no mirror in the oops table.
   int _nmethod_mirror_index;
@@ -51,6 +52,14 @@ class JVMCINMethodData {
   // is appended when it causes a deoptimization.
   FailedSpeculation** _failed_speculations;
 
+  // A speculation id is a length (low 5 bits) and an index into
+  // a jbyte array (i.e. 31 bits for a positive Java int).
+  enum {
+    // Keep in sync with HotSpotSpeculationEncoding.
+    SPECULATION_LENGTH_BITS = 5,
+    SPECULATION_LENGTH_MASK = (1 << SPECULATION_LENGTH_BITS) - 1
+  };
+
 public:
   // Computes the size of a JVMCINMethodData object
   static int compute_size(const char* nmethod_mirror_name) {

src/hotspot/share/jvmci/vmStructs_jvmci.cpp

@@ -391,6 +391,7 @@
   declare_constant(HeapWordSize)                                          \
   declare_constant(InvocationEntryBci)                                    \
   declare_constant(LogKlassAlignmentInBytes)                              \
+  declare_constant(JVMCINMethodData::SPECULATION_LENGTH_BITS)             \
                                                                           \
   declare_constant(JVM_ACC_WRITTEN_FLAGS)                                 \
   declare_constant(JVM_ACC_MONITOR_MATCH)                                 \

src/hotspot/share/runtime/thread.hpp

@@ -1160,7 +1160,8 @@ class JavaThread: public Thread {
   bool      _in_retryable_allocation;
 
   // An id of a speculation that JVMCI compiled code can use to further describe and
-  // uniquely identify the  speculative optimization guarded by the uncommon trap
+  // uniquely identify the speculative optimization guarded by an uncommon trap.
+  // See JVMCINMethodData::SPECULATION_LENGTH_BITS for further details.
   jlong     _pending_failed_speculation;
 
   // These fields are mutually exclusive in terms of live ranges.

src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotResolvedJavaFieldImpl.java

@@ -167,9 +167,9 @@ class HotSpotResolvedJavaFieldImpl implements HotSpotResolvedJavaField {
     }
 
     /**
-     * Checks if this field has the {@link Stable} annotation.
+     * Checks if this field has the {@code Stable} annotation.
      *
-     * @return true if field has {@link Stable} annotation, false otherwise
+     * @return true if field has {@code Stable} annotation, false otherwise
      */
     @Override
     public boolean isStable() {

src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotSpeculationEncoding.java

@@ -29,6 +29,7 @@ import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 import java.util.Arrays;
 
+import jdk.vm.ci.common.JVMCIError;
 import jdk.vm.ci.meta.ResolvedJavaMethod;
 import jdk.vm.ci.meta.ResolvedJavaType;
 import jdk.vm.ci.meta.SpeculationLog.SpeculationReasonEncoding;
@@ -36,11 +37,25 @@ import jdk.vm.ci.meta.SpeculationLog.SpeculationReasonEncoding;
 /**
  * Implements a {@link SpeculationReasonEncoding} that {@linkplain #getByteArray() produces} a byte
  * array. Data is added via a {@link DataOutputStream}. When producing the final byte array, if the
- * total length of data exceeds the length of a SHA-1 digest and a SHA-1 digest algorithm is
+ * total length of data exceeds {@value HotSpotSpeculationEncoding#MAX_LENGTH}, then a SHA-1 digest
- * available, then a SHA-1 digest of the data is produced instead.
+ * of the data is produced instead.
  */
 final class HotSpotSpeculationEncoding extends ByteArrayOutputStream implements SpeculationReasonEncoding {
 
+    /**
+     * Number of bits used for the length of an encoded speculation. The bit size of 5 is chosen to
+     * accommodate specifying // the length of a SHA-1 digest (i.e., 20 bytes).
+     */
+    // Also defined in C++ JVMCINMethodData class - keep in sync.
+    static final int LENGTH_BITS = 5;
+
+    /**
+     * The maximum length of an encoded speculation.
+     */
+    static final int MAX_LENGTH = (1 << LENGTH_BITS) - 1;
+
+    static final int LENGTH_MASK = MAX_LENGTH;
+
     private DataOutputStream dos = new DataOutputStream(this);
     private byte[] result;
 
@@ -160,7 +175,7 @@ final class HotSpotSpeculationEncoding extends ByteArrayOutputStream implements
      * time.
      */
     private static final boolean SHA1_IS_CLONEABLE;
-    private static final int SHA1_LENGTH;
+    private static final int SHA1_LENGTH = 20;
 
     static {
         MessageDigest sha1 = null;
@@ -171,13 +186,14 @@ final class HotSpotSpeculationEncoding extends ByteArrayOutputStream implements
             sha1IsCloneable = true;
         } catch (NoSuchAlgorithmException e) {
             // Should never happen given that SHA-1 is mandated in a
-            // compliant Java platform implementation. However, be
+            // compliant Java platform implementation.
-            // conservative and fall back to not using a digest.
+            throw new JVMCIError(e);
         } catch (CloneNotSupportedException e) {
         }
         SHA1 = sha1;
         SHA1_IS_CLONEABLE = sha1IsCloneable;
-        SHA1_LENGTH = SHA1 == null ? 20 : SHA1.getDigestLength();
+        assert SHA1.getDigestLength() == SHA1_LENGTH;
+        assert SHA1_LENGTH < MAX_LENGTH;
     }
 
     /**
@@ -186,7 +202,7 @@ final class HotSpotSpeculationEncoding extends ByteArrayOutputStream implements
      */
     byte[] getByteArray() {
         if (result == null) {
-            if (SHA1 != null && count > SHA1_LENGTH) {
+            if (count > MAX_LENGTH) {
                 try {
                     MessageDigest md = SHA1_IS_CLONEABLE ? (MessageDigest) SHA1.clone() : MessageDigest.getInstance("SHA-1");
                     md.update(buf, 0, count);

src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotSpeculationLog.java

@@ -30,6 +30,7 @@ import java.util.Formatter;
 import java.util.List;
 
 import jdk.vm.ci.code.BailoutException;
+import jdk.vm.ci.common.JVMCIError;
 import jdk.vm.ci.meta.JavaConstant;
 import jdk.vm.ci.meta.SpeculationLog;
 
@@ -119,9 +120,9 @@ public class HotSpotSpeculationLog implements SpeculationLog {
     public static final class HotSpotSpeculation extends Speculation {
 
         /**
-         * A speculation id is a long encoding an offset (high 32 bits) and a length (low 32 bts).
+         * A speculation id is a long encoding a length (low 5 bits) and an index into a
-         * Combined, the index and length denote where the {@linkplain #encoding encoded
+         * {@code byte[]}. Combined, the index and length denote where the {@linkplain #encoding
-         * speculation} is in a {@linkplain HotSpotSpeculationLog#getFlattenedSpeculations
+         * encoded speculation} is in a {@linkplain HotSpotSpeculationLog#getFlattenedSpeculations
          * flattened} speculations array.
          */
         private final JavaConstant id;
@@ -233,15 +234,21 @@ public class HotSpotSpeculationLog implements SpeculationLog {
     }
 
     private static long encodeIndexAndLength(int index, int length) {
-        return ((long) index) << 32 | length;
+        if (length > HotSpotSpeculationEncoding.MAX_LENGTH || length < 0) {
+            throw new InternalError(String.format("Invalid encoded speculation length: %d (0x%x)", length, length));
+        }
+        if (index < 0) {
+            throw new JVMCIError("Encoded speculation index is negative: %d (0x%x)", index, index);
+        }
+        return (index << HotSpotSpeculationEncoding.LENGTH_BITS) | length;
     }
 
     private static int decodeIndex(long indexAndLength) {
-        return (int) (indexAndLength >>> 32);
+        return (int) (indexAndLength >>> HotSpotSpeculationEncoding.LENGTH_BITS);
     }
 
     private static int decodeLength(long indexAndLength) {
-        return (int) indexAndLength & 0xFFFFFFFF;
+        return (int) (indexAndLength & HotSpotSpeculationEncoding.LENGTH_MASK);
     }
 
     @Override
@@ -358,4 +365,3 @@ public class HotSpotSpeculationLog implements SpeculationLog {
         final long address;
     }
 }
-

src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotVMConfig.java

@@ -25,6 +25,7 @@ package jdk.vm.ci.hotspot;
 import static jdk.vm.ci.hotspot.HotSpotJVMCIRuntime.runtime;
 import static jdk.vm.ci.hotspot.UnsafeAccess.UNSAFE;
 
+import jdk.vm.ci.common.JVMCIError;
 import jdk.vm.ci.services.Services;
 import jdk.internal.misc.Unsafe;
 
@@ -46,6 +47,9 @@ class HotSpotVMConfig extends HotSpotVMConfigAccess {
 
     HotSpotVMConfig(HotSpotVMConfigStore store) {
         super(store);
+
+        int speculationLengthBits = getConstant("JVMCINMethodData::SPECULATION_LENGTH_BITS", Integer.class);
+        JVMCIError.guarantee(HotSpotSpeculationEncoding.LENGTH_BITS == speculationLengthBits, "%d != %d", HotSpotSpeculationEncoding.LENGTH_BITS, speculationLengthBits);
     }
 
     /**

test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.hotspot.test/src/jdk/vm/ci/hotspot/test/TestHotSpotSpeculationLog.java

@@ -25,6 +25,7 @@
  * @test
  * @requires vm.jvmci
  * @modules jdk.internal.vm.ci/jdk.vm.ci.hotspot
+ *          jdk.internal.vm.ci/jdk.vm.ci.runtime
  *          jdk.internal.vm.ci/jdk.vm.ci.meta
  * @library /compiler/jvmci/jdk.vm.ci.hotspot.test/src
  * @run testng/othervm
@@ -41,8 +42,12 @@ import org.testng.SkipException;
 import org.testng.annotations.Test;
 
 import jdk.vm.ci.hotspot.HotSpotSpeculationLog;
+import jdk.vm.ci.meta.JavaConstant;
+import jdk.vm.ci.meta.JavaKind;
+import jdk.vm.ci.meta.MetaAccessProvider;
 import jdk.vm.ci.meta.SpeculationLog;
 import jdk.vm.ci.meta.SpeculationLog.SpeculationReasonEncoding;
+import jdk.vm.ci.runtime.JVMCI;
 
 public class TestHotSpotSpeculationLog {
 
@@ -88,6 +93,7 @@ public class TestHotSpotSpeculationLog {
 
     @Test
     public synchronized void testFailedSpeculations() {
+        MetaAccessProvider metaAccess = JVMCI.getRuntime().getHostJVMCIBackend().getMetaAccess();
         HotSpotSpeculationLog log = new HotSpotSpeculationLog();
         DummyReason reason1 = new DummyReason("dummy1");
         String longName = new String(new char[2000]).replace('\0', 'X');
@@ -98,6 +104,11 @@ public class TestHotSpotSpeculationLog {
         SpeculationLog.Speculation s1 = log.speculate(reason1);
         SpeculationLog.Speculation s2 = log.speculate(reason2);
 
+        JavaConstant encodedS1 = metaAccess.encodeSpeculation(s1);
+        JavaConstant encodedS2 = metaAccess.encodeSpeculation(s2);
+        Assert.assertEquals(JavaKind.Long, encodedS1.getJavaKind());
+        Assert.assertEquals(JavaKind.Long, encodedS2.getJavaKind());
+
         boolean added = log.addFailedSpeculation(s1);
         if (!added) {
             throw new SkipException("log.addFailedSpeculation(s1) is false");