From b115b7baac4d7c1a2f4d92278ad18cd53d265ff1 Mon Sep 17 00:00:00 2001 From: Marcin Kolny Date: Sat, 7 Oct 2023 12:55:14 +0100 Subject: [PATCH] Fix compilation of shift opcodes on x86_64 and i386 architectures (#2619) This change fixes the case where the right parameter of shift operator is negative, specifically, when both parameters of shift opcode are constants. --- .../compilation_on_android_ubuntu.yml | 7 +- core/iwasm/compilation/aot_emit_numberic.c | 18 ++- tests/wamr-compiler/.gitignore | 2 + tests/wamr-compiler/README.md | 3 + .../test_shift_negative_constants.wat | 43 +++++++ tests/wamr-test-suites/test_wamr.sh | 115 +++++++++++------- .../run_wamr_compiler_tests.sh | 22 ++++ 7 files changed, 159 insertions(+), 51 deletions(-) create mode 100644 tests/wamr-compiler/.gitignore create mode 100644 tests/wamr-compiler/README.md create mode 100644 tests/wamr-compiler/test_shift_negative_constants.wat create mode 100755 tests/wamr-test-suites/wamr-compiler-test-script/run_wamr_compiler_tests.sh diff --git a/.github/workflows/compilation_on_android_ubuntu.yml b/.github/workflows/compilation_on_android_ubuntu.yml index 3c5b4e35..3d6e44c6 100644 --- 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 index f540dbe0..bda64f83 100644 --- 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 new file mode 100644 index 00000000..5425c41b --- /dev/null +++ b/tests/wamr-compiler/.gitignore @@ -0,0 +1,2 @@ +*.aot +*.wasm \ No newline at end of file diff --git a/tests/wamr-compiler/README.md b/tests/wamr-compiler/README.md new file mode 100644 index 00000000..d48ea982 --- /dev/null +++ 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 new file mode 100644 index 00000000..030cc898 --- /dev/null +++ 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 index a0803133..81844f0e 100755 --- 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 new file mode 100755 index 00000000..19c8030d --- /dev/null +++ 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