Re: [PATCH v2 0/9] mips: Fill delay slots v3

[Paulo César Pereira de Andrade]

Em sáb., 14 de jan. de 2023 às 12:11, Paul Cercueil
<paul@crapouillou.net> escreveu:
>
> Hi Paulo,

  Hi Paul,

  Patches pushed as all tests pass.

> Here's the V3 of my patchset that attempts to fill branch delay slots on
> MIPS.
>
> All tests do pass now, except the "float.nodata" check, but this one
> fails in master as well.

  What environment are you using for testing? It works in the mips
environments I am testing.

  Please let me know the output of:

$ cpp -dM </dev/null

> I implemented your suggestion and it worked just fine. There was one
> last problem though, which caused the "clobber" check to fail once
> again; code emitters that generated branches to the next
> instruction (to conditionally execute some opcodes) caused problems, as
> the next instruction was not detected as a jump target (no
> "jit_flag_patch" set). I solved this by just checking the previous
> node's code against a table of accepted values.

  Not certain if I fully understand this. It also appears to be a somewhat
fragile logic.

  Everytime the delay slot is used, there should be a comment
telling that instruction is in the delay slot, for example:

static void
_ltr_f(jit_state_t *_jit, jit_int32_t r0, jit_int32_t r1, jit_int32_t r2)
{
    jit_word_t        w;
    C_OLT_S(r1, r2);
    w = _jit->pc.w;
    BC1T(0);
    /* delay slot */
    movi(r0, 1);
    movi(r0, 0);
    patch_at(w, _jit->pc.w);
}

so, you check the previous opcode for float comparison and cas{r,i}.
Shouldn't it also check for b{o,x}{add,sub}{r,i} codes? These also
end with a delay slot usage, and based on the patches and your
description, not checking them might create very difficult to debug
bugs in jit doing complex branch logic on carry/overflow.

> Cheers,
> -Paul
>
> Paul Cercueil (8):
>   mips: Optimize jit_eqr / jit_eqi
>   mips: Fill delay slots of JR opcodes in jit_jmpr
>   mips: Fill delay slots of JALR opcodes in jit_callr
>   mips: Fill delay slots of J in jit_jmpi
>   mips: Fill delay slots in jit_beqr / jit_beqi
>   mips: Fill delay slots in jit_bner / jit_bnei
>   mips: Fill delay slots in jit_bgtr, jit_bgti, jit_bler, jit_blei
>   mips: Fill delay slots in jit_bger, jit_bgei, jit_bltr, jit_blti
>
>  lib/jit_mips-cpu.c | 913 +++++++++++++++++++++++++++++----------------
>  lib/jit_mips.c     |  89 +++--
>  2 files changed, 647 insertions(+), 355 deletions(-)
>
> --
> 2.39.0

Thanks!
Paulo