Re: [Tarantool-patches] [PATCH luajit 19/19] MIPS: Add MIPS64 R6 port.

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

Hi, Maxim!
Thanks for the review!
Fixed your comments inline.

On 16.08.23, Maxim Kokryashkin wrote:
> Hi, Sergey!
> Thanks for the patch!
> LGTM, except for a few nits regarding the commit message.
> On Wed, Aug 09, 2023 at 06:36:08PM +0300, Sergey Kaplun via Tarantool-patches wrote:
> > From: Mike Pall <mike>
> > 
> > Contributed by Hua Zhang, YunQiang Su from Wave Computing,
> > and Radovan Birdic from RT-RK.
> > Sponsored by Wave Computing.
> > 
> > (cherry-picked from commit 94d0b53004a5fa368defa4307a17edcdb87fe727)
> > 
> > This patch adds support for MIPS Release 6 [1] for the 64-bit build.
> > This includes:
> > * Global `_map_def` value is set with <dynasm/dynasm.lua>. `MIPSR6` key
> >   specifies the corresponding instruction set support. Also, `MIPSR6` is
> >   defined in `DYNASM_FLAGS` (`DASM_AFLAGS`).
> > * New instructions are added within <dynasm/dasm_mips.lua>, they are
> >   used if the aforementioned key is set.
> > * Obsolete instructions (that are no more in use in r6) are used in the
> Typo: s/no more/no longer/

Fixed.

> >   opposite case (if `MIPSR6` isn't set).
> > * New opcode maps are added into  <src/jit/dis_mips.lua>.
> Typo: s/into/to/

Fixed.

> > * `map_arch` table in <jit/bcsave.lua> is refactored for more convenient
> >   usage. Now each arch key contains a table with the corresponding info
> >   about supported architecture:
> Typo: s/about/about the/

Fixed.

> >     - `e`: endianess; "le" or "be"
> >     - `b`: bit-width of the supported architecture; 32 or 64
> >     - `m`: machine specification (see `e_machine` in man elf)
> >     - `f`: processor-specific flags (see `e_flags` in man elf)
> >     - `p`: number that identifies the type of target machine [2] for
> >       Portable Executable format [3].
> > * New `LJ_TARGET_MIPSR6` define is set for MIPSR6 in <src/lj_arch.h>.
> > * The corresponding "MIPS32R6", "MIPS64R6" CPU strings are added to the
> >   <src/jit.h>
> > * MIPSR6 instructions are added to the <src/lj_target_mips.h>, some
> >   obsolete instructions are removed or defined only for the non-MIPSR6
> >   build.
> > * All release-dependent instructions in <src/lj_asm_mips.h> are
> >   instrumented with `LJ_TARGET_MIPSR6` macro.
> > * `f20`, `f21`, `f22` FP registers are defined as `FTMP0`, `FTMP1`,
> >   `FTMP2` correspondingly in the VM.
> > * All release-dependent instructions in <src/vm_mips64.dasm> are
> >   instrumented with `MIPSR6` macro.
> > * `sfmin_max` macro now takes the third operand for the MIPSR6 build.
> > * Fix implicit fallthrough warning for `LJ_SOFTFP && !LJ_NEED_FP64`
> Typo: s/Fix/Fix the/

Fixed.

> >   build in <src/lj_asm.c>.
> > 
> > Note, that 32-bit r6 targets still unsupported, because it is difficult
> Typo: s/targets/targets are/

Fixed.

> > and most available r6 CPUs are 64 bit.
> > 
> > [1]: https://www.mips.com/products/architectures/mips64/
> > [2]: https://learn.microsoft.com/en-us/windows/win32/debug/pe-format#machine-types
> > [3]: https://learn.microsoft.com/en-us/windows/win32/debug/pe-format
> > 
> > Sergey Kaplun:
> > * added the description for the feature
> > 
> > Part of tarantool/tarantool#8825
> > ---

<snipped>

> > 

-- 
Best regards,
Sergey Kaplun