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.