Re: [Tarantool-patches] [PATCH v4 1/2] core: introduce various platform metrics

[Igor Munkin <imun@tarantool.org>]

Sergey,

On 07.10.20, Igor Munkin wrote:
> Sergey,
> 
> Thanks for your fixes! There is still a comment regarding CNEW
> assembling and a couple minors below.
> 

<snipped>

> 
> > diff --git a/src/lj_asm_mips.h b/src/lj_asm_mips.h
> > index f4b4b5d..0341701 100644
> > --- a/src/lj_asm_mips.h
> > +++ b/src/lj_asm_mips.h
> > @@ -1430,7 +1430,9 @@ static void asm_cnew(ASMState *as, IRIns *ir)
> >    CTInfo info = lj_ctype_info(cts, id, &sz);
> >    const CCallInfo *ci = &lj_ir_callinfo[IRCALL_lj_mem_newgco];
> >    IRRef args[4];
> > +  RegSet allow = (RSET_GPR & ~RSET_SCRATCH);
> >    RegSet drop = RSET_SCRATCH;
> > +  Reg tmp;
> >    lua_assert(sz != CTSIZE_INVALID || (ir->o == IR_CNEW && ir->op2 != REF_NIL));
> >  
> >    as->gcsteps++;
> > @@ -1442,7 +1444,6 @@ static void asm_cnew(ASMState *as, IRIns *ir)
> >  
> >    /* Initialize immutable cdata object. */ >    if (ir->o == IR_CNEWI) {
> > -    RegSet allow = (RSET_GPR & ~RSET_SCRATCH);
> >  #if LJ_32
> >      int32_t ofs = sizeof(GCcdata);
> >      if (sz == 8) {
> > @@ -1473,15 +1474,16 @@ static void asm_cnew(ASMState *as, IRIns *ir)
> >      return;
> >    }
> >  
> > +  tmp = ra_scratch(as, allow);
> 
> Since there are registers allocated in scope of IR_CNEWI assembling
> above, you need to exclude those registers from <allow> set prior to
> scratching a new one.

I glanced <ra_scratch> once more: the register to be yielded is picked
from the intersection between the not used set and allowed set (which is
quite obvious), so there is no problem. Disregard the comment above.

> 
> >    /* Code incrementing cdatanum is sparse to avoid mips data hazards. */

<snipped>

-- 
Best regards,
IM