[GR-60623] Halve branch profile counter values on overflow.

woess · woess · commit 646b913417f3 · 2025-01-24T22:04:06.000Z
PullRequest: graal/19858
diff --git a/wasm/src/org.graalvm.wasm.benchcases/src/bench/wat/collatz.result b/wasm/src/org.graalvm.wasm.benchcases/src/bench/wat/collatz.result
@@ -0,0 +1 @@
+int 986
diff --git a/wasm/src/org.graalvm.wasm.benchcases/src/bench/wat/collatz.wat b/wasm/src/org.graalvm.wasm.benchcases/src/bench/wat/collatz.wat
@@ -0,0 +1,113 @@
+;;
+;; Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.
+;; DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+;;
+;; The Universal Permissive License (UPL), Version 1.0
+;;
+;; Subject to the condition set forth below, permission is hereby granted to any
+;; person obtaining a copy of this software, associated documentation and/or
+;; data (collectively the "Software"), free of charge and under any and all
+;; copyright rights in the Software, and any and all patent rights owned or
+;; freely licensable by each licensor hereunder covering either (i) the
+;; unmodified Software as contributed to or provided by such licensor, or (ii)
+;; the Larger Works (as defined below), to deal in both
+;;
+;; (a) the Software, and
+;;
+;; (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if
+;; one is included with the Software each a "Larger Work" to which the Software
+;; is contributed by such licensors),
+;;
+;; without restriction, including without limitation the rights to copy, create
+;; derivative works of, display, perform, and distribute the Software and make,
+;; use, sell, offer for sale, import, export, have made, and have sold the
+;; Software and the Larger Work(s), and to sublicense the foregoing rights on
+;; either these or other terms.
+;;
+;; This license is subject to the following condition:
+;;
+;; The above copyright notice and either this complete permission notice or at a
+;; minimum a reference to the UPL must be included in all copies or substantial
+;; portions of the Software.
+;;
+;; THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+;; IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+;; FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+;; AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+;; LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+;; OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+;; SOFTWARE.
+;;
+
+(;
+pub fn collatz_steps(mut n: u64) -> u64 {
+  let mut counter = 0;
+  while n != 1 {
+    if n % 2 == 0 {
+      n /= 2;
+    } else {
+      n = 3 * n + 1;
+    }
+    counter += 1;
+  }
+  counter
+}
+;)
+
+(module
+  (type $int_func (func (result i32)))
+  (type $setup_func (func))
+  (type $teardown_func (func (param i32)))
+
+  (global $iterations i64 (i64.const 670617279))
+
+  (memory $memory (export "memory") 0)
+
+  (func (export "benchmarkSetupEach") (type $setup_func))
+
+  (func (export "benchmarkTeardownEach") (type $teardown_func))
+
+  (func (export "benchmarkRun") (type $int_func)
+    global.get $iterations
+    call $collatz_steps
+    i32.wrap_i64
+  )
+
+  (func $collatz_steps (export "collatz_steps") (param i64) (result i64)
+    (local i64)
+    i64.const 0
+    local.set 1
+    block
+      local.get 0
+      i64.const 1
+      i64.eq
+      br_if 0
+      i64.const 0
+      local.set 1
+      loop $continue
+        local.get 1
+        i64.const 1
+        i64.add
+        local.set 1
+        local.get 0
+        i64.const 1
+        i64.shr_u
+        local.get 0
+        i64.const 3
+        i64.mul
+        i64.const 1
+        i64.add
+        local.get 0
+        i64.const 1
+        i64.and
+        i64.eqz
+        select
+        local.tee 0
+        i64.const 1
+        i64.ne
+        br_if $continue
+      end
+    end
+    local.get 1
+  )
+)
diff --git a/wasm/src/org.graalvm.wasm/src/org/graalvm/wasm/BinaryStreamParser.java b/wasm/src/org.graalvm.wasm/src/org/graalvm/wasm/BinaryStreamParser.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2019, 2025, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * The Universal Permissive License (UPL), Version 1.0
@@ -42,19 +42,28 @@
 
 import static com.oracle.truffle.api.nodes.ExplodeLoop.LoopExplosionKind.FULL_EXPLODE_UNTIL_RETURN;
 
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.VarHandle;
+import java.nio.ByteOrder;
+import java.util.Arrays;
+
 import org.graalvm.wasm.constants.GlobalModifier;
 import org.graalvm.wasm.exception.Failure;
 import org.graalvm.wasm.exception.WasmException;
 
+import com.oracle.truffle.api.CompilerDirectives;
 import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
+import com.oracle.truffle.api.memory.ByteArraySupport;
 import com.oracle.truffle.api.nodes.ExplodeLoop;
 
-import java.util.Arrays;
-
 public abstract class BinaryStreamParser {
     protected static final int SINGLE_RESULT_VALUE = 0;
     protected static final int MULTI_RESULT_VALUE = 1;
 
+    private static final VarHandle I16LE = MethodHandles.byteArrayViewVarHandle(short[].class, ByteOrder.LITTLE_ENDIAN);
+    private static final VarHandle I32LE = MethodHandles.byteArrayViewVarHandle(int[].class, ByteOrder.LITTLE_ENDIAN);
+    private static final VarHandle I64LE = MethodHandles.byteArrayViewVarHandle(long[].class, ByteOrder.LITTLE_ENDIAN);
+
     @CompilationFinal(dimensions = 1) protected byte[] data;
     protected int offset;
 
@@ -353,7 +362,7 @@ public static byte peekLeb128Length(byte[] data, int initialOffset) {
 
     /**
      * Reads the unsigned byte value at the given bytecode offset.
-     * 
+     *
      * @param bytecode The bytecode
      * @param offset The offset in the bytecode
      * @return the unsigned byte value at the given bytecode offset.
@@ -364,7 +373,7 @@ public static int rawPeekU8(byte[] bytecode, int offset) {
 
     /**
      * Reads the signed byte value at the given bytecode offset.
-     * 
+     *
      * @param bytecode The bytecode
      * @param offset The offset in the bytecode
      * @return The signed byte value at the given bytecode offset.
@@ -375,27 +384,38 @@ public static byte rawPeekI8(byte[] bytecode, int offset) {
 
     /**
      * Reads the unsigned short value at the given bytecode offset.
-     * 
+     *
      * @param bytecode The bytecode
      * @param offset The offset in the bytecode
      * @return The unsigned short value at the given bytecode offset.
      */
     public static int rawPeekU16(byte[] bytecode, int offset) {
-        return ((bytecode[offset] & 0xFF) | ((bytecode[offset + 1] & 0xFF) << 8));
+        return Short.toUnsignedInt(rawPeekI16(bytecode, offset));
+    }
+
+    /**
+     * Reads the signed short value at the given bytecode offset.
+     *
+     * @param bytecode The bytecode
+     * @param offset The offset in the bytecode
+     * @return The signed short value at the given bytecode offset.
+     */
+    public static short rawPeekI16(byte[] bytecode, int offset) {
+        if (CompilerDirectives.inCompiledCode()) {
+            return ByteArraySupport.littleEndian().getShortUnaligned(bytecode, offset);
+        }
+        return (short) I16LE.get(bytecode, offset);
     }
 
     /**
      * Writes the unsigned short value to the given bytecode offset.
-     * 
+     *
      * @param bytecode The bytecode
      * @param offset The offset in the bytecode
      * @param value The value that should be written
      */
     public static void writeU16(byte[] bytecode, int offset, int value) {
-        final byte low = (byte) (value & 0xFF);
-        final byte high = (byte) ((value >> 8) & 0xFF);
-        bytecode[offset] = low;
-        bytecode[offset + 1] = high;
+        I16LE.set(bytecode, offset, (short) value);
     }
 
     /**
@@ -406,60 +426,46 @@ public static void writeU16(byte[] bytecode, int offset, int value) {
      * @return The unsigned integer value at the given bytecode offset.
      */
     public static long rawPeekU32(byte[] bytecode, int offset) {
-        return (bytecode[offset] & 0xFFL) |
-                        ((bytecode[offset + 1] & 0xFFL) << 8) |
-                        ((bytecode[offset + 2] & 0xFFL) << 16) |
-                        ((bytecode[offset + 3] & 0xFFL) << 24);
+        return Integer.toUnsignedLong(rawPeekI32(bytecode, offset));
     }
 
     /**
      * Reads the signed integer value at the given bytecode offset.
-     * 
+     *
      * @param bytecode The bytecode
      * @param offset The offset in the bytecode.
      * @return The signed integer value at the given bytecode offset.
      */
     public static int rawPeekI32(byte[] bytecode, int offset) {
-        return (bytecode[offset] & 0xFF) |
-                        ((bytecode[offset + 1] & 0xFF) << 8) |
-                        ((bytecode[offset + 2] & 0xFF) << 16) |
-                        ((bytecode[offset + 3] & 0xFF) << 24);
+        if (CompilerDirectives.inCompiledCode()) {
+            return ByteArraySupport.littleEndian().getIntUnaligned(bytecode, offset);
+        }
+        return (int) I32LE.get(bytecode, offset);
     }
 
     /**
      * Reads the signed long value at the given bytecode offset.
-     * 
+     *
      * @param bytecode The bytecode
      * @param offset The offset in the bytecode.
      * @return The signed long value at the given bytecode offset.
      */
     public static long rawPeekI64(byte[] bytecode, int offset) {
-        return (bytecode[offset] & 0xFFL) |
-                        ((bytecode[offset + 1] & 0xFFL) << 8) |
-                        ((bytecode[offset + 2] & 0xFFL) << 16) |
-                        ((bytecode[offset + 3] & 0xFFL) << 24) |
-                        ((bytecode[offset + 4] & 0xFFL) << 32) |
-                        ((bytecode[offset + 5] & 0xFFL) << 40) |
-                        ((bytecode[offset + 6] & 0xFFL) << 48) |
-                        ((bytecode[offset + 7] & 0xFFL) << 56);
+        if (CompilerDirectives.inCompiledCode()) {
+            return ByteArraySupport.littleEndian().getLongUnaligned(bytecode, offset);
+        }
+        return (long) I64LE.get(bytecode, offset);
     }
 
     /**
      * Writes the signed long value to the given bytecode offset.
-     * 
+     *
      * @param bytecode The bytecode
      * @param offset The offset in the bytecode
      * @param value The value that should be written
      */
     public static void writeI64(byte[] bytecode, int offset, long value) {
-        bytecode[offset] = (byte) (value & 0xFF);
-        bytecode[offset + 1] = (byte) ((value >> 8) & 0xFF);
-        bytecode[offset + 2] = (byte) ((value >> 16) & 0xFF);
-        bytecode[offset + 3] = (byte) ((value >> 24) & 0xFF);
-        bytecode[offset + 4] = (byte) ((value >> 32) & 0xFF);
-        bytecode[offset + 5] = (byte) ((value >> 40) & 0xFF);
-        bytecode[offset + 6] = (byte) ((value >> 48) & 0xFF);
-        bytecode[offset + 7] = (byte) ((value >> 56) & 0xFF);
+        I64LE.set(bytecode, offset, value);
     }
 
     /**
diff --git a/wasm/src/org.graalvm.wasm/src/org/graalvm/wasm/nodes/WasmFunctionNode.java b/wasm/src/org.graalvm.wasm/src/org/graalvm/wasm/nodes/WasmFunctionNode.java
@@ -4646,45 +4646,54 @@ private static void dropStack(VirtualFrame frame, int stackPointer, int targetSt
     private static final int MAX_PROFILE_VALUE = 0x0000_00ff;
     private static final int MAX_TABLE_PROFILE_VALUE = 0x0000_ffff;
 
+    @SuppressWarnings("all") // "The parameter condition should not be assigned."
     private static boolean profileCondition(byte[] data, final int profileOffset, boolean condition) {
         int t = rawPeekU8(data, profileOffset);
         int f = rawPeekU8(data, profileOffset + 1);
-        boolean val = condition;
-        if (val) {
+        if (condition) {
             if (t == 0) {
                 CompilerDirectives.transferToInterpreterAndInvalidate();
             }
-            if (!CompilerDirectives.inInterpreter()) {
-                if (f == 0) {
-                    // Make this branch fold during PE
-                    val = true;
+            if (CompilerDirectives.inInterpreter()) {
+                if (t < MAX_PROFILE_VALUE) {
+                    t++;
+                } else {
+                    // halve count rounding up, must never go from 1 to 0.
+                    f = (f >>> 1) + (f & 0x1);
+                    t = (MAX_PROFILE_VALUE >>> 1) + 1;
+                    data[profileOffset + 1] = (byte) f;
                 }
+                data[profileOffset] = (byte) t;
+                return condition;
             } else {
-                if (t < MAX_PROFILE_VALUE) {
-                    data[profileOffset] = (byte) (t + 1);
+                if (f == 0) {
+                    // Make this branch fold during PE
+                    condition = true;
                 }
             }
         } else {
             if (f == 0) {
                 CompilerDirectives.transferToInterpreterAndInvalidate();
             }
-            if (!CompilerDirectives.inInterpreter()) {
-                if (t == 0) {
-                    // Make this branch fold during PE
-                    val = false;
+            if (CompilerDirectives.inInterpreter()) {
+                if (f < MAX_PROFILE_VALUE) {
+                    f++;
+                } else {
+                    // halve count rounding up, must never go from 1 to 0.
+                    t = (t >>> 1) + (t & 0x1);
+                    f = (MAX_PROFILE_VALUE >>> 1) + 1;
+                    data[profileOffset] = (byte) t;
                 }
+                data[profileOffset + 1] = (byte) f;
+                return condition;
             } else {
-                if (f < MAX_PROFILE_VALUE) {
-                    data[profileOffset + 1] = (byte) (f + 1);
+                if (t == 0) {
+                    // Make this branch fold during PE
+                    condition = false;
                 }
             }
         }
-        if (CompilerDirectives.inInterpreter()) {
-            return val;
-        } else {
-            int sum = t + f;
-            return CompilerDirectives.injectBranchProbability((double) t / (double) sum, val);
-        }
+        return CompilerDirectives.injectBranchProbability((double) t / (double) (t + f), condition);
     }
 
     private static void updateBranchTableProfile(byte[] data, final int counterOffset, final int profileOffset) {
