Re: [PATCH] simulator for mips3264

[cgd at broadcom dot com]

echristo@redhat.com ("Eric M. Christopher") writes:
> Here's a patch I've been using for quite some time for a mips32 and
> mips64 simulator. I don't think I have write after approval so if
> approved either I'll need that or someone else (Andrew?) will need to
> check it in for me.

And now the tables are turned!  Muhahahahaha!  8-)

Andrew, is it really the case that there's no write-after-approval to
sim?  If not, can we allow it?  8-) If not, after resolution of the
issues below I'd be glad to check these changes in for you Eric.


So, the issues / questions:

(1) "using how?" / "tested how?"

(2) why bother define the do_foo functions in m3264.igen, when they're
    only used in one function?

(3) why did you add clo/clz in mips.igen?  I know there are some
    non-true-mips32/mips64 CPUs which implement them...  but they also
    implement madd/maddu/msub/msubu.  Any argument that puts these in
    mips.igen also puts the rest, as far as I can see.

    Personally, if I were doing this ("oh wait, we did, but I've not
    had time to go back and submit it all yet!" 8-) I'd put the new
    mips32/mips64 instructions into mips.igen, since they're neither
    particular chip-specific and they're not an ASE either.

    I think the patch should be changed to do one or the other.

(4) you're missing dclo + dclz.

(5) the diff looks like you're implement {mf,mt}{lo,hi} w/ hazards as
    needed for early MIPS.  Is that correct?  (MIPS32/MIPS64 and
    indeed back to MIPS IV, didn't have that restriction.  Looks like
    the code was incorrect for MIPS IV previously, according to the
    MIPS64 BIS doc.)

I didn't read all of the diffs which add mips32/mips64 to existing
instructions, but I believe you did the right thing w/ them.  (If not,
we'll undoubtedly notice when we sync up our tree.)


Anyway, if you fix (3)-(5) and explain the rest, these are approved.


chris