| Message ID | f989c31bcf4bd8a80a50fa6899bcd239ed1bec69.1535609090.git.sandipan@linux.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | powerpc sstep: Extend instruction emulation support | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | next/apply_patch Successfully applied |
| snowpatch_ozlabs/checkpatch | fail | Test checkpatch on branch next |
On Mon, Sep 03, 2018 at 08:49:35PM +0530, Sandipan Das wrote: > + case 538: /* cnttzw */ > + if (!cpu_has_feature(CPU_FTR_ARCH_300)) > + return -1; > + val = (unsigned int) regs->gpr[rd]; > + op->val = ( val ? __builtin_ctz(val) : 32 ); > + goto logical_done; > +#ifdef __powerpc64__ > + case 570: /* cnttzd */ > + if (!cpu_has_feature(CPU_FTR_ARCH_300)) > + return -1; > + val = regs->gpr[rd]; > + op->val = ( val ? __builtin_ctzl(val) : 64 ); > + goto logical_done; __builtin_ctz(val) is undefined for val == 0. Segher
On Tue, Sep 04, 2018 at 04:12:07PM -0500, Segher Boessenkool wrote: > On Mon, Sep 03, 2018 at 08:49:35PM +0530, Sandipan Das wrote: > > + case 538: /* cnttzw */ > > + if (!cpu_has_feature(CPU_FTR_ARCH_300)) > > + return -1; > > + val = (unsigned int) regs->gpr[rd]; > > + op->val = ( val ? __builtin_ctz(val) : 32 ); > > + goto logical_done; > > +#ifdef __powerpc64__ > > + case 570: /* cnttzd */ > > + if (!cpu_has_feature(CPU_FTR_ARCH_300)) > > + return -1; > > + val = regs->gpr[rd]; > > + op->val = ( val ? __builtin_ctzl(val) : 64 ); > > + goto logical_done; > > __builtin_ctz(val) is undefined for val == 0. Which would be why he only calls it when val != 0, presumably, and uses 64 when val == 0. Apart from idiosyncratic whitespace his code looks correct to me. Are you saying there is a bug in his code, or that his patch description is incomplete, or what? Paul.
diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c index 18ac0a26c4fc..d57cb4e28621 100644 --- a/arch/powerpc/lib/sstep.c +++ b/arch/powerpc/lib/sstep.c @@ -1816,6 +1816,20 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs, case 506: /* popcntd */ do_popcnt(regs, op, regs->gpr[rd], 64); goto logical_done_nocc; +#endif + case 538: /* cnttzw */ + if (!cpu_has_feature(CPU_FTR_ARCH_300)) + return -1; + val = (unsigned int) regs->gpr[rd]; + op->val = ( val ? __builtin_ctz(val) : 32 ); + goto logical_done; +#ifdef __powerpc64__ + case 570: /* cnttzd */ + if (!cpu_has_feature(CPU_FTR_ARCH_300)) + return -1; + val = regs->gpr[rd]; + op->val = ( val ? __builtin_ctzl(val) : 64 ); + goto logical_done; #endif case 922: /* extsh */ op->val = (signed short) regs->gpr[rd];
This adds emulation support for the following integer instructions: * Count Trailing Zeros Word (cnttzw[.]) * Count Trailing Zeros Doubleword (cnttzd[.]) Signed-off-by: Sandipan Das <sandipan@linux.ibm.com> --- arch/powerpc/lib/sstep.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)