Re: [Tarantool-patches] [PATCH luajit v2 2/2] test: add debug extension tests for FPMATHOP

[Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi, Mikhail!
Thanks for the patch!
Please consider my comments below.

On 15.09.26, Mikhail Elhimov wrote:
> These tests extends set of tests that check mapping of FPMATHOP code

Typo: s/extends/extend/
Typo: s/set/the set/
Typo: s/mapping/the mapping/

> into a human-readable form while dumping FPMATH IR with the debug
> extension.
> ---
>  .../debug-extension-tests.py                  | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/test/tarantool-debugger-tests/debug-extension-tests.py b/test/tarantool-debugger-tests/debug-extension-tests.py
> index 32701fd5..9989032b 100644
> --- a/test/tarantool-debugger-tests/debug-extension-tests.py
> +++ b/test/tarantool-debugger-tests/debug-extension-tests.py
> @@ -711,6 +711,25 @@ class TestLJIRFPMathOpBase(TestCaseBase):
>          super(TestLJIRFPMathOpBase, cls).setUpClass()
>  
>  
> +@unittest.skipIf(machine in ('arm64', 'aarch64'),
> +                 "not used as there is no corresponding hardware instruction")

Instead of skipping the case, it is better to adjust the
LJIRFPMathOpBase. The issue with missing IR is due to DUALNUM mode,
which coerces the 1 to int, so there is no need to floor or ceil it. If
we use 1.1 instead for all FPMMath tests, it fixes the issue.

> +class TestLJIRFPMathFloor(TestLJIRFPMathOpBase):
> +    lua_expr = 'math.floor(a)'
> +    op = 'floor'
> +
> +
> +@unittest.skipIf(machine in ('arm64', 'aarch64'),
> +                 "not used as there is no corresponding hardware instruction")
> +class TestLJIRFPMathCeil(TestLJIRFPMathOpBase):
> +    lua_expr = 'math.ceil(a)'
> +    op = 'ceil'
> +
> +
> +class TestLJIRFPMathSqrt(TestLJIRFPMathOpBase):
> +    lua_expr = 'math.sqrt(a)'
> +    op = 'sqrt'
> +
> +
>  class TestLJIRFPMathLog(TestLJIRFPMathOpBase):
>      lua_expr = 'math.log(a)'
>      op = 'log'
> -- 
> 2.43.0
> 

-- 
Best regards,
Sergey Kaplun