Hi,
Apologies if this has come up in a previous revision.
> case 1:
> + if (!cpu_has_feature(CPU_FTR_ARCH_31))
> + return -1;
> +
> prefix_r = GET_PREFIX_R(word);
> ra = GET_PREFIX_RA(suffix);
The comment above analyse_instr reads in part:
* Return value is 1 if the instruction can be emulated just by
* updating *regs with the information in *op, -1 if we need the
* GPRs but *regs doesn't contain the full register set, or 0
* otherwise.
I was wondering why returning -1 if the instruction isn't supported the
right thing to do - it seemed to me that it should return 0?
I did look and see that there are other cases where the code returns -1
for an unsupported operation, e.g.:
#ifdef __powerpc64__
case 4:
if (!cpu_has_feature(CPU_FTR_ARCH_300))
return -1;
switch (word & 0x3f) {
case 48: /* maddhd */
That's from commit 930d6288a267 ("powerpc: sstep: Add support for
maddhd, maddhdu, maddld instructions"), but it's not explained there either
I see the same pattern in a number of commits: commit 6324320de609
("powerpc sstep: Add support for modsd, modud instructions"), commit
6c180071509a ("powerpc sstep: Add support for modsw, moduw
instructions"), commit a23987ef267a ("powerpc: sstep: Add support for
darn instruction") and a few others, all of which seem to have come
through Sandipan in February 2019. I haven't spotted any explanation for
why -1 was chosen, but I haven't checked the mailing list archives.
If -1 is OK, would it be possible to update the comment to explain why?
Kind regards,
Daniel
