Extend SIMD benchmark tests #162

GorogPeter · 2023-09-07T12:45:18Z

No description provided.

zherczeg · 2023-09-07T12:51:14Z

test/wasmBenchmarker/benchmark.py

        print("compiling " + name)
-        bob_the_stringbuilder = "./emsdk/upstream/emscripten/emcc " + path + "/" + file + " --no-entry -s WASM=1 -s EXPORTED_FUNCTIONS=_runtime -s EXPORTED_RUNTIME_METHODS=ccall,cwrap -o " + path + "/wasm/" + name + ".wasm"
+        bob_the_stringbuilder = "./emsdk/upstream/emscripten/emcc " + path + "/" + file + " " + extraFlags + " --no-entry -s WASM=1 -s EXPORTED_FUNCTIONS=_runtime -s EXPORTED_RUNTIME_METHODS=ccall,cwrap -o " + path + "/wasm/" + name + ".wasm"


It would be good if simd tests could be enabled by a command line argument

Okay. I'm on it.

zherczeg · 2023-09-07T12:52:03Z

test/wasmBenchmarker/ctests/simdMandelbrotCount.c

+
+uint32_t runtime();
+
+int main() {


Man could be moved below, and runtime forward definition is not needed.

zherczeg · 2023-09-07T12:52:35Z

test/wasmBenchmarker/ctests/simdMandelbrotCount.c

+uint32_t runtime();
+
+int main() {
+    if (WIDTH % 4 != 0) {


Is this useful? Add a comment to WIDTH instead.

zherczeg · 2023-09-07T12:53:33Z

test/wasmBenchmarker/ctests/simdMandelbrotCount.c

+
+#define getNthBit(b, n) ((b & (0b00000001 << (7 - n))) > 0)
+
+#define setNthBitZero(b, n) b = b & (0b11111111 - (0b00000001 << (7 - n)))


clearNthBit

zherczeg · 2023-09-07T12:53:55Z

test/wasmBenchmarker/ctests/simdMandelbrotCount.c

+
+#define setNthBitZero(b, n) b = b & (0b11111111 - (0b00000001 << (7 - n)))
+
+#define setNthBitOne(b, n) b = b | (0b00000001 << (7 - n))


setNthBit
set / clear tells the bit value

zherczeg · 2023-09-07T12:54:57Z

test/wasmBenchmarker/ctests/simdMandelbrotCount.c

+            __m128 real = _mm_add_ps(_mm_mul_ps(_mm_set_ps(j+3, j+2, j+1, j), _mm_set1_ps(ZOOM)), _mm_set1_ps(REAL_AXIS_SHIFT));
+            __m128 imaginary = _mm_add_ps(_mm_mul_ps(_mm_set1_ps(i), _mm_set1_ps(ZOOM)), _mm_set1_ps(IMAGINARY_AXIS_SHIFT));
+            byte pixels = areInMandelbrotSet(real, imaginary);
+            for (int i = 0; i < 4; i++) {


Add newline before

zherczeg · 2023-09-07T12:55:06Z

test/wasmBenchmarker/ctests/simdMandelbrotCount.c

+    __m128 z_imaginary = _mm_set1_ps(0);
+    for (size_t i = 0; i < N; i++) {
+        __m128 cmp_result = _mm_cmpgt_ps(ABS_COMPLEX(z_real, z_imaginary), _mm_set1_ps(2));
+        for (size_t j = 0; j < 4; j++) {


Add newline before

Still not added

zherczeg · 2023-09-07T12:55:13Z

test/wasmBenchmarker/ctests/simdMandelbrotCount.c

+                setNthBitZero(result, j);
+            }
+        }
+        __m128 next_z_real = _mm_add_ps(_mm_sub_ps(SQUARE(z_real), SQUARE(z_imaginary)), c_real);


Add newline before

zherczeg · 2023-09-07T12:55:53Z

test/wasmBenchmarker/ctests/simdMandelbrotCount.c

+        z_real = next_z_real;
+        z_imaginary = next_z_imaginary;
+        if (result == 0b00000000) {
+            break;


Is it break or return? Break just stops the inner loop.

Okay. I'll change it.

zherczeg · 2023-09-11T10:40:02Z

I realized the base branch is not good. This should be based on the interpreter

zherczeg · 2023-09-11T10:41:34Z

test/wasmBenchmarker/benchmark.py

@@ -58,6 +61,7 @@ def prepare_arg_pars():
    parser.add_argument('--iterations', metavar='NUMBER', help='how many times run the tests', nargs='?',
                        const='10', default=10, type=int)
    parser.add_argument('--compile-anyway', help='compile the tests even if they are compiled', action='store_true')
+    parser.add_argument('--simd-tests', help='run SIMD tests too', action='store_true')


--enable-simd would sound better

Okay. I'll change it.

zherczeg

LGTM after the newline is added

GorogPeter · 2023-09-12T13:16:14Z

Please don't merge.

I will change the "no-SIMD" Mandelbrot test to calculate the same thing.

zherczeg · 2023-09-25T07:58:24Z

test/wasmBenchmarker/ctests/mandelbrot.c

+            z_real = next_z_real;
+            z_imaginary = next_z_imaginary;
+        }
+        if (result == 0) {


Add newline before if

zherczeg · 2023-09-25T07:58:48Z

test/wasmBenchmarker/ctests/mandelbrot.c

 }

 int main() {
-    printf("%d\n", runtime());
+    printf("%u\n", runtime());
    return 0;
 }


Add newline at the end

zherczeg · 2023-09-25T07:59:05Z

test/wasmBenchmarker/ctests/simdMandelbrot.c

+int main() {
+    printf("%u\n", runtime());
+    return 0;
+}


Add newline at the end.

zherczeg · 2023-09-25T08:02:53Z

test/wasmBenchmarker/ctests/simdMandelbrot.c

+        v128_t next_z_imaginary = wasm_f32x4_add(wasm_f32x4_mul(wasm_f32x4_mul(z_real, z_imaginary), wasm_f32x4_const_splat(2)), c_imaginary);
+        z_real = next_z_real;
+        z_imaginary = next_z_imaginary;
+        if (result == 0b00000000) {


Add newline before if. The other code use == 0, I would prefer that here.

zherczeg

LGTM

clover2123 · 2023-10-04T02:21:58Z

test/wasmBenchmarker/ctests/mandelbrot.c

-/*
- * Copyright (c) 2023-present Samsung Electronics Co., Ltd
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */


Why did you remove these license comments?
I also wonder the origin of this test code.

I accidently removed it. Thank you for noticing it.

When I started to write the SIMD Mandelbrot test, at first I made a non-SIMD version, then I made the SIMD version based on it. To ensure that both of the Mandelbrot tests calculate the same thing, I copied my non-SIMD version to the benchmark tests (replacing the old one). Now, their results can be compared.

clover2123 · 2023-10-04T02:26:29Z

One more,
This patch just adds simdMandelbrot test, but not enabled in default.
If possible, please enable this test on available platforms in our CI (actions)
(interpreter mode supports basic SIMD features now)

zherczeg

Good patch. Could you squash the patches and rebase the patch?

SIMD versions of Mandelbrot and NBody tests have been added. Their non-SIMD pairs have been change to work similarly. Thus, the SIMD and non-SIMD results can be compared.

zherczeg

LGTM

GorogPeter requested review from ksh8281 and clover2123 as code owners September 7, 2023 12:45

zherczeg requested changes Sep 7, 2023

View reviewed changes

zherczeg force-pushed the jit_next branch from de7eb5f to 583cb15 Compare September 8, 2023 06:16

GorogPeter force-pushed the addSimdTest branch from 46c03a5 to 882a3ef Compare September 10, 2023 08:37

zherczeg force-pushed the jit_next branch from 583cb15 to 9e7977d Compare September 11, 2023 10:28

zherczeg reviewed Sep 11, 2023

View reviewed changes

GorogPeter force-pushed the addSimdTest branch 2 times, most recently from 6614277 to 27a0ee7 Compare September 11, 2023 13:17

GorogPeter changed the base branch from jit_next to interp September 11, 2023 13:17

zherczeg approved these changes Sep 12, 2023

View reviewed changes

GorogPeter force-pushed the addSimdTest branch 3 times, most recently from 35c5fe0 to 5d9e9d3 Compare September 12, 2023 11:30

GorogPeter force-pushed the addSimdTest branch from 5d9e9d3 to e975a36 Compare September 22, 2023 08:46

zherczeg requested changes Sep 25, 2023

View reviewed changes

GorogPeter force-pushed the addSimdTest branch from e975a36 to d3f02b5 Compare September 25, 2023 08:33

zherczeg approved these changes Sep 25, 2023

View reviewed changes

clover2123 reviewed Oct 4, 2023

View reviewed changes

GorogPeter changed the title ~~Add Mandelbrot SIMD test to the benchmark tests~~ Extend SIMD benchmark tests Oct 6, 2023

GorogPeter force-pushed the addSimdTest branch 2 times, most recently from a612301 to fef3850 Compare October 8, 2023 11:35

zherczeg reviewed Oct 8, 2023

View reviewed changes

Add SIMD tests

5d83957

SIMD versions of Mandelbrot and NBody tests have been added. Their non-SIMD pairs have been change to work similarly. Thus, the SIMD and non-SIMD results can be compared.

GorogPeter force-pushed the addSimdTest branch from fef3850 to 5d83957 Compare October 9, 2023 15:57

zherczeg approved these changes Oct 10, 2023

View reviewed changes

clover2123 approved these changes Oct 11, 2023

View reviewed changes

clover2123 merged commit 54c0b6a into Samsung:interp Oct 11, 2023
12 checks passed

GorogPeter deleted the addSimdTest branch November 2, 2023 14:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extend SIMD benchmark tests #162

Extend SIMD benchmark tests #162

GorogPeter commented Sep 7, 2023

zherczeg Sep 7, 2023

GorogPeter Sep 7, 2023

zherczeg Sep 7, 2023

GorogPeter Sep 7, 2023

zherczeg Sep 7, 2023

GorogPeter Sep 7, 2023

zherczeg Sep 7, 2023

GorogPeter Sep 7, 2023

zherczeg Sep 7, 2023

GorogPeter Sep 7, 2023

zherczeg Sep 7, 2023

GorogPeter Sep 7, 2023

zherczeg Sep 7, 2023

GorogPeter Sep 7, 2023

zherczeg Sep 12, 2023

zherczeg Sep 7, 2023

GorogPeter Sep 7, 2023

zherczeg Sep 7, 2023

GorogPeter Sep 7, 2023

zherczeg commented Sep 11, 2023

zherczeg Sep 11, 2023

GorogPeter Sep 11, 2023

zherczeg left a comment

GorogPeter commented Sep 12, 2023

zherczeg Sep 25, 2023

zherczeg Sep 25, 2023

zherczeg Sep 25, 2023

zherczeg Sep 25, 2023

zherczeg left a comment

clover2123 Oct 4, 2023

GorogPeter Oct 6, 2023

clover2123 commented Oct 4, 2023 •

edited

Loading

zherczeg left a comment

zherczeg left a comment


		#define getNthBit(b, n) ((b & (0b00000001 << (7 - n))) > 0)

		#define setNthBitZero(b, n) b = b & (0b11111111 - (0b00000001 << (7 - n)))


		#define setNthBitZero(b, n) b = b & (0b11111111 - (0b00000001 << (7 - n)))

		#define setNthBitOne(b, n) b = b \| (0b00000001 << (7 - n))

Extend SIMD benchmark tests #162

Extend SIMD benchmark tests #162

Conversation

GorogPeter commented Sep 7, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zherczeg commented Sep 11, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zherczeg left a comment

Choose a reason for hiding this comment

GorogPeter commented Sep 12, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zherczeg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clover2123 commented Oct 4, 2023 • edited Loading

zherczeg left a comment

Choose a reason for hiding this comment

zherczeg left a comment

Choose a reason for hiding this comment

clover2123 commented Oct 4, 2023 •

edited

Loading