Skip to content

Commit b115b7b

Browse files
loganekloganek
authored
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.
1 parent 3668093 commit b115b7b

File tree

7 files changed

+159
-51
lines changed

7 files changed

+159
-51
lines changed

‎.github/workflows/compilation_on_android_ubuntu.yml‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ env:
6565
THREADS_TEST_OPTIONS: "-s spec -b -p -P"
6666
X86_32_TARGET_TEST_OPTIONS: "-m x86_32 -P"
6767
WASI_TEST_OPTIONS: "-s wasi_certification -w"
68+
WAMR_COMPILER_TEST_OPTIONS: "-s wamr_compiler -b -P"
6869

6970
jobs:
7071
build_llvm_libraries_on_ubuntu_2204:
@@ -333,7 +334,7 @@ jobs:
333334
build_iwasm,
334335
build_llvm_libraries_on_ubuntu_2204,
335336
build_wamrc,
336-
]
337+
]
337338
runs-on: ${{ matrix.os }}
338339
strategy:
339340
matrix:
@@ -482,6 +483,10 @@ jobs:
482483
- os: ubuntu-22.04
483484
llvm_cache_key: ${{ needs.build_llvm_libraries_on_ubuntu_2204.outputs.cache_key }}
484485
ubuntu_version: "22.04"
486+
- os: ubuntu-22.04
487+
llvm_cache_key: ${{ needs.build_llvm_libraries_on_ubuntu_2204.outputs.cache_key }}
488+
running_mode: aot
489+
test_option: $WAMR_COMPILER_TEST_OPTIONS
485490
exclude:
486491
# uncompatiable modes and features
487492
# classic-interp and fast-interp don't support simd

‎core/iwasm/compilation/aot_emit_numberic.c‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,15 @@
171171
right = shift_count_mask; \
172172
} while (0)
173173

174+
static bool
175+
is_shift_count_mask_needed(AOTCompContext *comp_ctx, LLVMValueRef left,
176+
LLVMValueRef right)
177+
{
178+
return (strcmp(comp_ctx->target_arch, "x86_64") != 0
179+
&& strcmp(comp_ctx->target_arch, "i386") != 0)
180+
|| (LLVMIsEfficientConstInt(left) && LLVMIsEfficientConstInt(right));
181+
}
182+
174183
/* Call llvm constrained floating-point intrinsic */
175184
static LLVMValueRef
176185
call_llvm_float_experimental_constrained_intrinsic(AOTCompContext *comp_ctx,
@@ -728,8 +737,7 @@ compile_int_shl(AOTCompContext *comp_ctx, LLVMValueRef left, LLVMValueRef right,
728737
{
729738
LLVMValueRef res;
730739

731-
if (strcmp(comp_ctx->target_arch, "x86_64") != 0
732-
&& strcmp(comp_ctx->target_arch, "i386") != 0)
740+
if (is_shift_count_mask_needed(comp_ctx, left, right))
733741
SHIFT_COUNT_MASK;
734742

735743
/* Build shl */
@@ -744,8 +752,7 @@ compile_int_shr_s(AOTCompContext *comp_ctx, LLVMValueRef left,
744752
{
745753
LLVMValueRef res;
746754

747-
if (strcmp(comp_ctx->target_arch, "x86_64") != 0
748-
&& strcmp(comp_ctx->target_arch, "i386") != 0)
755+
if (is_shift_count_mask_needed(comp_ctx, left, right))
749756
SHIFT_COUNT_MASK;
750757

751758
/* Build shl */
@@ -760,8 +767,7 @@ compile_int_shr_u(AOTCompContext *comp_ctx, LLVMValueRef left,
760767
{
761768
LLVMValueRef res;
762769

763-
if (strcmp(comp_ctx->target_arch, "x86_64") != 0
764-
&& strcmp(comp_ctx->target_arch, "i386") != 0)
770+
if (is_shift_count_mask_needed(comp_ctx, left, right))
765771
SHIFT_COUNT_MASK;
766772

767773
/* Build shl */

‎tests/wamr-compiler/.gitignore‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
*.aot
2+
*.wasm

‎tests/wamr-compiler/README.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
# WAMR test benchmarks
2+
3+
This folder contains tests for WAMR AOT compiler and its generated code.

‎tests/wamr-compiler/test_shift_negative_constants.wat‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
;; Copyright (C) 2023 Amazon Inc. All rights reserved.
2+
;; SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
3+
;;
4+
;; Those tests verify if passing constant negative value
5+
;; as a right parameter of the shift operator (along
6+
;; with a constant value of the left operator) causes
7+
;; any problems. See: https://github.com/bytecodealliance/wasm-micro-runtime/pull/2619
8+
(module
9+
(memory (export "memory") 1 1)
10+
(func $assert_eq (param i32 i32)
11+
(i32.ne (local.get 0) (local.get 1))
12+
if
13+
unreachable
14+
end
15+
)
16+
17+
(func $i32_shr_u
18+
(call $assert_eq
19+
(i32.shr_u (i32.const -1) (i32.const -5))
20+
(i32.const 31)
21+
)
22+
)
23+
24+
(func $i32_shr_s
25+
(call $assert_eq
26+
(i32.shr_u (i32.const 32) (i32.const -30))
27+
(i32.const 8)
28+
)
29+
)
30+
31+
(func $i32_shl
32+
(call $assert_eq
33+
(i32.shl (i32.const -1) (i32.const -30))
34+
(i32.const -4)
35+
)
36+
)
37+
38+
(func (export "_start")
39+
call $i32_shr_u
40+
call $i32_shr_s
41+
call $i32_shl
42+
)
43+
)

‎tests/wamr-test-suites/test_wamr.sh‎

Lines changed: 71 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ function help()
1414
{
1515
echo "test_wamr.sh [options]"
1616
echo "-c clean previous test results, not start test"
17-
echo "-s {suite_name} test only one suite (spec|wasi_certification)"
17+
echo "-s {suite_name} test only one suite (spec|wasi_certification|wamr_compiler)"
1818
echo "-m set compile target of iwasm(x86_64|x86_32|armv7_vfp|thumbv7_vfp|riscv64_lp64d|riscv64_lp64|aarch64)"
1919
echo "-t set compile type of iwasm(classic-interp|fast-interp|jit|aot|fast-jit|multi-tier-jit)"
2020
echo "-M enable multi module feature"
@@ -309,6 +309,53 @@ function sightglass_test()
309309
echo "Finish sightglass benchmark tests"
310310
}
311311

312+
function setup_wabt()
313+
{
314+
if [ ${WABT_BINARY_RELEASE} == "YES" ]; then
315+
echo "download a binary release and install"
316+
local WAT2WASM=${WORK_DIR}/wabt/out/gcc/Release/wat2wasm
317+
if [ ! -f ${WAT2WASM} ]; then
318+
case ${PLATFORM} in
319+
cosmopolitan)
320+
;&
321+
linux)
322+
WABT_PLATFORM=ubuntu
323+
;;
324+
darwin)
325+
WABT_PLATFORM=macos
326+
;;
327+
*)
328+
echo "wabt platform for ${PLATFORM} in unknown"
329+
exit 1
330+
;;
331+
esac
332+
if [ ! -f /tmp/wabt-1.0.31-${WABT_PLATFORM}.tar.gz ]; then
333+
wget \
334+
https://github.com/WebAssembly/wabt/releases/download/1.0.31/wabt-1.0.31-${WABT_PLATFORM}.tar.gz \
335+
-P /tmp
336+
fi
337+
338+
cd /tmp \
339+
&& tar zxf wabt-1.0.31-${WABT_PLATFORM}.tar.gz \
340+
&& mkdir -p ${WORK_DIR}/wabt/out/gcc/Release/ \
341+
&& install wabt-1.0.31/bin/wa* ${WORK_DIR}/wabt/out/gcc/Release/ \
342+
&& cd -
343+
fi
344+
else
345+
echo "download source code and compile and install"
346+
if [ ! -d "wabt" ];then
347+
echo "wabt not exist, clone it from github"
348+
git clone --recursive https://github.com/WebAssembly/wabt
349+
fi
350+
echo "upate wabt"
351+
cd wabt
352+
git pull
353+
git reset --hard origin/main
354+
cd ..
355+
make -C wabt gcc-release -j 4
356+
fi
357+
}
358+
312359
# TODO: with iwasm only
313360
function spec_test()
314361
{
@@ -383,49 +430,7 @@ function spec_test()
383430
popd
384431
echo $(pwd)
385432

386-
if [ ${WABT_BINARY_RELEASE} == "YES" ]; then
387-
echo "download a binary release and install"
388-
local WAT2WASM=${WORK_DIR}/wabt/out/gcc/Release/wat2wasm
389-
if [ ! -f ${WAT2WASM} ]; then
390-
case ${PLATFORM} in
391-
cosmopolitan)
392-
;&
393-
linux)
394-
WABT_PLATFORM=ubuntu
395-
;;
396-
darwin)
397-
WABT_PLATFORM=macos
398-
;;
399-
*)
400-
echo "wabt platform for ${PLATFORM} in unknown"
401-
exit 1
402-
;;
403-
esac
404-
if [ ! -f /tmp/wabt-1.0.31-${WABT_PLATFORM}.tar.gz ]; then
405-
wget \
406-
https://github.com/WebAssembly/wabt/releases/download/1.0.31/wabt-1.0.31-${WABT_PLATFORM}.tar.gz \
407-
-P /tmp
408-
fi
409-
410-
cd /tmp \
411-
&& tar zxf wabt-1.0.31-${WABT_PLATFORM}.tar.gz \
412-
&& mkdir -p ${WORK_DIR}/wabt/out/gcc/Release/ \
413-
&& install wabt-1.0.31/bin/wa* ${WORK_DIR}/wabt/out/gcc/Release/ \
414-
&& cd -
415-
fi
416-
else
417-
echo "download source code and compile and install"
418-
if [ ! -d "wabt" ];then
419-
echo "wabt not exist, clone it from github"
420-
git clone --recursive https://github.com/WebAssembly/wabt
421-
fi
422-
echo "upate wabt"
423-
cd wabt
424-
git pull
425-
git reset --hard origin/main
426-
cd ..
427-
make -C wabt gcc-release -j 4
428-
fi
433+
setup_wabt
429434

430435
ln -sf ${WORK_DIR}/../spec-test-script/all.py .
431436
ln -sf ${WORK_DIR}/../spec-test-script/runtest.py .
@@ -513,6 +518,28 @@ function wasi_test()
513518
echo "Finish wasi tests"
514519
}
515520

521+
function wamr_compiler_test()
522+
{
523+
if [[ $1 != "aot" ]]; then
524+
echo "WAMR compiler tests only support AOT mode"
525+
exit 1
526+
fi
527+
528+
echo "Now start WAMR compiler tests"
529+
setup_wabt
530+
cd ${WORK_DIR}/../wamr-compiler-test-script
531+
./run_wamr_compiler_tests.sh ${WORK_DIR}/wabt/out/gcc/Release/wat2wasm $WAMRC_CMD $IWASM_CMD \
532+
| tee -a ${REPORT_DIR}/wamr_compiler_test_report.txt
533+
534+
ret=${PIPESTATUS[0]}
535+
536+
if [[ ${ret} -ne 0 ]];then
537+
echo -e "\nWAMR compiler tests FAILED" | tee -a ${REPORT_DIR}/wamr_compiler_test_report.txt
538+
exit 1
539+
fi
540+
echo -e "\nFinish WAMR compiler tests" | tee -a ${REPORT_DIR}/wamr_compiler_test_report.txt
541+
}
542+
516543
function wasi_certification_test()
517544
{
518545
echo "Now start wasi certification tests"

‎tests/wamr-test-suites/wamr-compiler-test-script/run_wamr_compiler_tests.sh‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
#!/bin/bash
2+
3+
# Copyright (C) 2023 Amazon Inc. All rights reserved.
4+
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
5+
6+
set -e
7+
8+
WAT2WASM_CMD=$1
9+
WAMRC_CMD=$2
10+
IWASM_CMD=$3
11+
12+
for wat_file in ../../wamr-compiler/*.wat; do
13+
wasm_file="${wat_file%.wat}.wasm"
14+
aot_file="${wat_file%.wat}.aot"
15+
16+
echo "Compiling $wat_file to $wasm_file"
17+
$WAT2WASM_CMD "$wat_file" -o "$wasm_file"
18+
echo "Compiling $wasm_file to $aot_file"
19+
$WAMRC_CMD -o $aot_file $wasm_file
20+
echo "Testing $aot_file"
21+
$IWASM_CMD "$aot_file"
22+
done

0 commit comments

Comments
 (0)