Re: [try 2nd 4/8] Displaced stepping for Thumb 16-bit insn

["Ulrich Weigand" <uweigand at de dot ibm dot com>]

Yao Qi wrote:

> In my new patch, there are three different cases to handle POP instruction,
> 1.  register list is full, no free register.  The code sequence I am
> using is like
> 
>      POP {r0, r1, ...., r6};
>      POP {r7};

The above can use just a single POP {r0, ..., r7}, can't it?

>      MOV r8, r7;
>      POP {r7};
> 
> after execution of this sequence, PC's value is stored in r7, and r7's
> value is stored in r8.  In cleanup, we can set PC, r7, and r8 accordingly.
> 
> 2.  register list is not full, and not empty.  In this case, we scan the
> code to find a free register, rN.  Run the follow code sequence,
> 
>      POP {rX, rY, ...., rZ};
>      POP {rN};
> 
> After execution of this sequence, PC's value is stored in rN.  In
> cleanup, we can set PC from rN.

Have you looked at how the ARM case does it?  There, we still have just
a single POP { r0, ..., rN } that pops the right number of registers,
and then the cleanup function (cleanup_block_load_pc) reshuffles them.
It seems to me we could do the same (and actually use the same cleanup
function) for the Thumb case too ...

> 3.  register list is empty.  This case is relative simple.
> 
>      POP {r0}
> 
> In cleanup, we store r0's value to PC.

If we used cleanup_block_load_pc, this would handle the same case as well.

(Unfortunately, handling case 1 the same way looks somewhat difficult,
since cleanup_block_load_pc would expect the PC in register r8 ...)

> +cleanup_pop_pc_16bit(struct gdbarch *gdbarch, struct regcache *regs,
> +		     struct displaced_step_closure *dsc)

One more space before ( ...

> +  else /* Cleanup procedure of case #2 and case #3 can be unified.  */
> +    {
> +      int rx = 0;
> +      int rx_val = 0;
> +
> +      if (dsc->u.block.regmask)
> +	{
> +	  for (rx = 0; rx < 8; rx++)
> +	    if ((dsc->u.block.regmask & (1 << rx)) == 0)
> +	      break;
> +	}
> +      else
> +	rx = 0;

(This is irrelevant if we decide to use cleanup_block_load_pc, but:
the "if (dsc->u.block.regmask)" and "else rx = 0" are superfluous,
since the for loop will terminate with rx == 0 anyway if regmask
is zero.)

Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com