On Thu, Jan 09, 2020 at 09:23:14AM +1100, Paul Mackerras wrote:
> On Tue, Dec 10, 2019 at 12:49:03PM +0530, Balamuruhan S wrote:
> > This patch adds emulation support for divde, divdeu instructions,
> > * Divide Doubleword Extended (divde[.])
> > * Divide Doubleword Extended Unsigned (divdeu[.])
> >
> > Signed-off-by: Balamuruhan S <bal...@linux.ibm.com>
> > ---
> > arch/powerpc/lib/sstep.c | 27 ++++++++++++++++++++++++++-
> > 1 file changed, 26 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
> > index c077acb983a1..4b4119729e59 100644
> > --- a/arch/powerpc/lib/sstep.c
> > +++ b/arch/powerpc/lib/sstep.c
> > @@ -1736,7 +1736,32 @@ int analyse_instr(struct instruction_op *op, const
> > struct pt_regs *regs,
> > op->val = (int) regs->gpr[ra] /
> > (int) regs->gpr[rb];
> > goto arith_done;
> > -
> > +#ifdef __powerpc64__
> > + case 425: /* divde[.] */
> > + if (instr & 1) {
> > + asm volatile(PPC_DIVDE_DOT(%0, %1, %2) :
> > + "=r" (op->val) : "r" (regs->gpr[ra]),
> > + "r" (regs->gpr[rb]));
> > + set_cr0(regs, op);
>
> This seems unneccesarily complicated. You take the trouble to do a
> "divde." instruction rather than a "divde" instruction but then don't
> use the CR0 setting that the instruction did, but instead go and work
> out what happens to CR0 manually in set_cr0(). Also you don't tell
> the compiler that CR0 has been modified, which could lead to problems.
>
> This case could be done much more simply like this:
>
>
>
> case 425: /* divde[.] */
> asm volatile(PPC_DIVDE(%0, %1, %2) :
> "=r" (op->val) : "r" (regs->gpr[ra]),
> "r" (regs->gpr[rb]));
> goto arith_done;
>
> (note, goto arith_done rather than compute_done) and similarly for the
> divdeu case.
Thanks Paul for review, I will fix it as suggested and post the v2 version.
-- Bala
>
> Paul.
