Fix compilation of shift opcodes on x86_64 and i386 architectures (by…

…tecodealliance#2619) This change fixes the case where the right parameter of shift operator is negative, specifically, when both parameters of shift opcode are constants.
victoryang00 · Oct 7, 2023 · 0406f67 · 0406f67
1 parent f111e9a
commit 0406f67
Show file tree

Hide file tree

Showing 7 changed files with 159 additions and 51 deletions.
diff --git a/.github/workflows/compilation_on_android_ubuntu.yml b/.github/workflows/compilation_on_android_ubuntu.yml
@@ -65,6 +65,7 @@ env:
   THREADS_TEST_OPTIONS: "-s spec -b -p -P"
   X86_32_TARGET_TEST_OPTIONS: "-m x86_32 -P"
   WASI_TEST_OPTIONS: "-s wasi_certification -w"
+  WAMR_COMPILER_TEST_OPTIONS: "-s wamr_compiler -b -P"
 
 jobs:
   build_llvm_libraries_on_ubuntu_2204:
@@ -333,7 +334,7 @@ jobs:
         build_iwasm,
         build_llvm_libraries_on_ubuntu_2204,
         build_wamrc,
-      ]    
+      ]
     runs-on: ${{ matrix.os }}
     strategy:
       matrix:
@@ -482,6 +483,10 @@ jobs:
           - os: ubuntu-22.04
             llvm_cache_key: ${{ needs.build_llvm_libraries_on_ubuntu_2204.outputs.cache_key }}
             ubuntu_version: "22.04"
+          - os: ubuntu-22.04
+            llvm_cache_key: ${{ needs.build_llvm_libraries_on_ubuntu_2204.outputs.cache_key }}
+            running_mode: aot
+            test_option: $WAMR_COMPILER_TEST_OPTIONS
         exclude:
           # uncompatiable modes and features
           # classic-interp and fast-interp don't support simd

diff --git a/core/iwasm/compilation/aot_emit_numberic.c b/core/iwasm/compilation/aot_emit_numberic.c
@@ -171,6 +171,15 @@
         right = shift_count_mask;                                      \
     } while (0)
 
+static bool
+is_shift_count_mask_needed(AOTCompContext *comp_ctx, LLVMValueRef left,
+                           LLVMValueRef right)
+{
+    return (strcmp(comp_ctx->target_arch, "x86_64") != 0
+            && strcmp(comp_ctx->target_arch, "i386") != 0)
+           || (LLVMIsEfficientConstInt(left) && LLVMIsEfficientConstInt(right));
+}
+
 /* Call llvm constrained floating-point intrinsic */
 static LLVMValueRef
 call_llvm_float_experimental_constrained_intrinsic(AOTCompContext *comp_ctx,
@@ -728,8 +737,7 @@ compile_int_shl(AOTCompContext *comp_ctx, LLVMValueRef left, LLVMValueRef right,
 {
     LLVMValueRef res;
 
-    if (strcmp(comp_ctx->target_arch, "x86_64") != 0
-        && strcmp(comp_ctx->target_arch, "i386") != 0)
+    if (is_shift_count_mask_needed(comp_ctx, left, right))
         SHIFT_COUNT_MASK;
 
     /* Build shl */
@@ -744,8 +752,7 @@ compile_int_shr_s(AOTCompContext *comp_ctx, LLVMValueRef left,
 {
     LLVMValueRef res;
 
-    if (strcmp(comp_ctx->target_arch, "x86_64") != 0
-        && strcmp(comp_ctx->target_arch, "i386") != 0)
+    if (is_shift_count_mask_needed(comp_ctx, left, right))
         SHIFT_COUNT_MASK;
 
     /* Build shl */
@@ -760,8 +767,7 @@ compile_int_shr_u(AOTCompContext *comp_ctx, LLVMValueRef left,
 {
     LLVMValueRef res;
 
-    if (strcmp(comp_ctx->target_arch, "x86_64") != 0
-        && strcmp(comp_ctx->target_arch, "i386") != 0)
+    if (is_shift_count_mask_needed(comp_ctx, left, right))
         SHIFT_COUNT_MASK;
 
     /* Build shl */

diff --git a/tests/wamr-compiler/.gitignore b/tests/wamr-compiler/.gitignore
@@ -0,0 +1,2 @@
+*.aot
+*.wasm
diff --git a/tests/wamr-compiler/README.md b/tests/wamr-compiler/README.md
@@ -0,0 +1,3 @@
+# WAMR test benchmarks
+
+This folder contains tests for WAMR AOT compiler and its generated code.
diff --git a/tests/wamr-compiler/test_shift_negative_constants.wat b/tests/wamr-compiler/test_shift_negative_constants.wat
@@ -0,0 +1,43 @@
+;; Copyright (C) 2023 Amazon Inc.  All rights reserved.
+;; SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+;;
+;; Those tests verify if passing constant negative value
+;; as a right parameter of the shift operator (along
+;; with a constant value of the left operator) causes
+;; any problems. See: https://github.com/bytecodealliance/wasm-micro-runtime/pull/2619
+(module
+  (memory (export "memory") 1 1)
+  (func $assert_eq (param i32 i32)
+    (i32.ne (local.get 0) (local.get 1))
+    if
+      unreachable
+    end
+  )
+
+  (func $i32_shr_u
+    (call $assert_eq
+      (i32.shr_u (i32.const -1) (i32.const -5))
+      (i32.const 31)
+    )
+  )
+
+  (func $i32_shr_s
+    (call $assert_eq
+      (i32.shr_u (i32.const 32) (i32.const -30))
+      (i32.const 8)
+    )
+  )
+
+  (func $i32_shl
+    (call $assert_eq
+      (i32.shl (i32.const -1) (i32.const -30))
+      (i32.const -4)
+    )
+  )
+
+  (func (export "_start")
+    call $i32_shr_u
+    call $i32_shr_s
+    call $i32_shl
+  )
+)
diff --git a/tests/wamr-test-suites/test_wamr.sh b/tests/wamr-test-suites/test_wamr.sh
@@ -14,7 +14,7 @@ function help()
 {
     echo "test_wamr.sh [options]"
     echo "-c clean previous test results, not start test"
-    echo "-s {suite_name} test only one suite (spec|wasi_certification)"
+    echo "-s {suite_name} test only one suite (spec|wasi_certification|wamr_compiler)"
     echo "-m set compile target of iwasm(x86_64|x86_32|armv7_vfp|thumbv7_vfp|riscv64_lp64d|riscv64_lp64|aarch64)"
     echo "-t set compile type of iwasm(classic-interp|fast-interp|jit|aot|fast-jit|multi-tier-jit)"
     echo "-M enable multi module feature"
@@ -309,6 +309,53 @@ function sightglass_test()
     echo "Finish sightglass benchmark tests"
 }
 
+function setup_wabt()
+{
+    if [ ${WABT_BINARY_RELEASE} == "YES" ]; then
+        echo "download a binary release and install"
+        local WAT2WASM=${WORK_DIR}/wabt/out/gcc/Release/wat2wasm
+        if [ ! -f ${WAT2WASM} ]; then
+            case ${PLATFORM} in
+                cosmopolitan)
+                    ;&
+                linux)
+                    WABT_PLATFORM=ubuntu
+                    ;;
+                darwin)
+                    WABT_PLATFORM=macos
+                    ;;
+                *)
+                    echo "wabt platform for ${PLATFORM} in unknown"
+                    exit 1
+                    ;;
+            esac
+            if [ ! -f /tmp/wabt-1.0.31-${WABT_PLATFORM}.tar.gz ]; then
+                wget \
+                    https://github.com/WebAssembly/wabt/releases/download/1.0.31/wabt-1.0.31-${WABT_PLATFORM}.tar.gz \
+                    -P /tmp
+            fi
+
+            cd /tmp \
+            && tar zxf wabt-1.0.31-${WABT_PLATFORM}.tar.gz \
+            && mkdir -p ${WORK_DIR}/wabt/out/gcc/Release/ \
+            && install wabt-1.0.31/bin/wa* ${WORK_DIR}/wabt/out/gcc/Release/ \
+            && cd -
+        fi
+    else
+        echo "download source code and compile and install"
+        if [ ! -d "wabt" ];then
+            echo "wabt not exist, clone it from github"
+            git clone --recursive https://github.com/WebAssembly/wabt
+        fi
+        echo "upate wabt"
+        cd wabt
+        git pull
+        git reset --hard origin/main
+        cd ..
+        make -C wabt gcc-release -j 4
+    fi
+}
+
 # TODO: with iwasm only
 function spec_test()
 {
@@ -383,49 +430,7 @@ function spec_test()
     popd
     echo $(pwd)
 
-    if [ ${WABT_BINARY_RELEASE} == "YES" ]; then
-        echo "download a binary release and install"
-        local WAT2WASM=${WORK_DIR}/wabt/out/gcc/Release/wat2wasm
-        if [ ! -f ${WAT2WASM} ]; then
-            case ${PLATFORM} in
-                cosmopolitan)
-                    ;&
-                linux)
-                    WABT_PLATFORM=ubuntu
-                    ;;
-                darwin)
-                    WABT_PLATFORM=macos
-                    ;;
-                *)
-                    echo "wabt platform for ${PLATFORM} in unknown"
-                    exit 1
-                    ;;
-            esac
-            if [ ! -f /tmp/wabt-1.0.31-${WABT_PLATFORM}.tar.gz ]; then
-                wget \
-                    https://github.com/WebAssembly/wabt/releases/download/1.0.31/wabt-1.0.31-${WABT_PLATFORM}.tar.gz \
-                    -P /tmp
-            fi
-
-            cd /tmp \
-            && tar zxf wabt-1.0.31-${WABT_PLATFORM}.tar.gz \
-            && mkdir -p ${WORK_DIR}/wabt/out/gcc/Release/ \
-            && install wabt-1.0.31/bin/wa* ${WORK_DIR}/wabt/out/gcc/Release/ \
-            && cd -
-        fi
-    else
-        echo "download source code and compile and install"
-        if [ ! -d "wabt" ];then
-            echo "wabt not exist, clone it from github"
-            git clone --recursive https://github.com/WebAssembly/wabt
-        fi
-        echo "upate wabt"
-        cd wabt
-        git pull
-        git reset --hard origin/main
-        cd ..
-        make -C wabt gcc-release -j 4
-    fi
+    setup_wabt
 
     ln -sf ${WORK_DIR}/../spec-test-script/all.py .
     ln -sf ${WORK_DIR}/../spec-test-script/runtest.py .
@@ -513,6 +518,28 @@ function wasi_test()
     echo "Finish wasi tests"
 }
 
+function wamr_compiler_test()
+{
+    if [[ $1 != "aot" ]]; then
+        echo "WAMR compiler tests only support AOT mode"
+        exit 1
+    fi
+
+    echo  "Now start WAMR compiler tests"
+    setup_wabt
+    cd ${WORK_DIR}/../wamr-compiler-test-script
+    ./run_wamr_compiler_tests.sh ${WORK_DIR}/wabt/out/gcc/Release/wat2wasm $WAMRC_CMD $IWASM_CMD \
+        | tee -a ${REPORT_DIR}/wamr_compiler_test_report.txt
+
+    ret=${PIPESTATUS[0]}
+
+    if [[ ${ret} -ne 0 ]];then
+        echo -e "\nWAMR compiler tests FAILED" | tee -a ${REPORT_DIR}/wamr_compiler_test_report.txt
+        exit 1
+    fi
+    echo -e "\nFinish WAMR compiler tests" | tee -a ${REPORT_DIR}/wamr_compiler_test_report.txt
+}
+
 function wasi_certification_test()
 {
     echo  "Now start wasi certification tests"

diff --git a/tests/wamr-test-suites/wamr-compiler-test-script/run_wamr_compiler_tests.sh b/tests/wamr-test-suites/wamr-compiler-test-script/run_wamr_compiler_tests.sh
@@ -0,0 +1,22 @@
+#!/bin/bash
+
+# Copyright (C) 2023 Amazon Inc.  All rights reserved.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+set -e
+
+WAT2WASM_CMD=$1
+WAMRC_CMD=$2
+IWASM_CMD=$3
+
+for wat_file in ../../wamr-compiler/*.wat; do
+    wasm_file="${wat_file%.wat}.wasm"
+    aot_file="${wat_file%.wat}.aot"
+
+    echo "Compiling $wat_file to $wasm_file"
+    $WAT2WASM_CMD "$wat_file" -o "$wasm_file"
+    echo "Compiling $wasm_file to $aot_file"
+    $WAMRC_CMD -o $aot_file $wasm_file
+    echo "Testing $aot_file"
+    $IWASM_CMD "$aot_file"
+done
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,3 @@
		# WAMR test benchmarks

		This folder contains tests for WAMR AOT compiler and its generated code.