linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] powerpc/lib/sstep: Fix count leading zeros instructions
@ 2017-10-10  6:45 Sandipan Das
  2017-10-10  6:54 ` Naveen N. Rao
  2017-10-12  0:20 ` [v2] " Michael Ellerman
  0 siblings, 2 replies; 3+ messages in thread
From: Sandipan Das @ 2017-10-10  6:45 UTC (permalink / raw)
  To: mpe; +Cc: naveen.n.rao, paulus, anton, segher, linuxppc-dev

According to the GCC documentation, the behaviour of __builtin_clz()
and __builtin_clzl() is undefined if the value of the input argument
is zero. Without handling this special case, these builtins have been
used for emulating the following instructions:
  * Count Leading Zeros Word (cntlzw[.])
  * Count Leading Zeros Doubleword (cntlzd[.])

This fixes the emulated behaviour of these instructions by adding an
additional check for this special case.

Fixes: 3cdfcbfd32b9d ("powerpc: Change analyse_instr so it doesn't modify *regs")
Signed-off-by: Sandipan Das <sandipan@linux.vnet.ibm.com>
---
v2: Make zero-checking condition more compact.
    Add details of original commit that is being fixed here.
---
 arch/powerpc/lib/sstep.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index 5118110c3983..8c3955e183d4 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -1699,11 +1699,13 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
  * Logical instructions
  */
 		case 26:	/* cntlzw */
-			op->val = __builtin_clz((unsigned int) regs->gpr[rd]);
+			val = (unsigned int) regs->gpr[rd];
+			op->val = ( val ? __builtin_clz(val) : 32 );
 			goto logical_done;
 #ifdef __powerpc64__
 		case 58:	/* cntlzd */
-			op->val = __builtin_clzl(regs->gpr[rd]);
+			val = regs->gpr[rd];
+			op->val = ( val ? __builtin_clzl(val) : 64 );
 			goto logical_done;
 #endif
 		case 28:	/* and */
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] powerpc/lib/sstep: Fix count leading zeros instructions
  2017-10-10  6:45 [PATCH v2] powerpc/lib/sstep: Fix count leading zeros instructions Sandipan Das
@ 2017-10-10  6:54 ` Naveen N. Rao
  2017-10-12  0:20 ` [v2] " Michael Ellerman
  1 sibling, 0 replies; 3+ messages in thread
From: Naveen N. Rao @ 2017-10-10  6:54 UTC (permalink / raw)
  To: Sandipan Das; +Cc: mpe, paulus, anton, segher, linuxppc-dev

On 2017/10/10 06:45AM, Sandipan Das wrote:
> According to the GCC documentation, the behaviour of __builtin_clz()
> and __builtin_clzl() is undefined if the value of the input argument
> is zero. Without handling this special case, these builtins have been
> used for emulating the following instructions:
>   * Count Leading Zeros Word (cntlzw[.])
>   * Count Leading Zeros Doubleword (cntlzd[.])
> 
> This fixes the emulated behaviour of these instructions by adding an
> additional check for this special case.
> 
> Fixes: 3cdfcbfd32b9d ("powerpc: Change analyse_instr so it doesn't modify *regs")
> Signed-off-by: Sandipan Das <sandipan@linux.vnet.ibm.com>

Reviewed-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>

> ---
> v2: Make zero-checking condition more compact.
>     Add details of original commit that is being fixed here.
> ---
>  arch/powerpc/lib/sstep.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
> index 5118110c3983..8c3955e183d4 100644
> --- a/arch/powerpc/lib/sstep.c
> +++ b/arch/powerpc/lib/sstep.c
> @@ -1699,11 +1699,13 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
>   * Logical instructions
>   */
>  		case 26:	/* cntlzw */
> -			op->val = __builtin_clz((unsigned int) regs->gpr[rd]);
> +			val = (unsigned int) regs->gpr[rd];
> +			op->val = ( val ? __builtin_clz(val) : 32 );
>  			goto logical_done;
>  #ifdef __powerpc64__
>  		case 58:	/* cntlzd */
> -			op->val = __builtin_clzl(regs->gpr[rd]);
> +			val = regs->gpr[rd];
> +			op->val = ( val ? __builtin_clzl(val) : 64 );
>  			goto logical_done;
>  #endif
>  		case 28:	/* and */
> -- 
> 2.13.6
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [v2] powerpc/lib/sstep: Fix count leading zeros instructions
  2017-10-10  6:45 [PATCH v2] powerpc/lib/sstep: Fix count leading zeros instructions Sandipan Das
  2017-10-10  6:54 ` Naveen N. Rao
@ 2017-10-12  0:20 ` Michael Ellerman
  1 sibling, 0 replies; 3+ messages in thread
From: Michael Ellerman @ 2017-10-12  0:20 UTC (permalink / raw)
  To: Sandipan Das; +Cc: linuxppc-dev, naveen.n.rao, paulus, anton

On Tue, 2017-10-10 at 06:45:30 UTC, Sandipan Das wrote:
> According to the GCC documentation, the behaviour of __builtin_clz()
> and __builtin_clzl() is undefined if the value of the input argument
> is zero. Without handling this special case, these builtins have been
> used for emulating the following instructions:
>   * Count Leading Zeros Word (cntlzw[.])
>   * Count Leading Zeros Doubleword (cntlzd[.])
> 
> This fixes the emulated behaviour of these instructions by adding an
> additional check for this special case.
> 
> Fixes: 3cdfcbfd32b9d ("powerpc: Change analyse_instr so it doesn't modify *regs")
> Signed-off-by: Sandipan Das <sandipan@linux.vnet.ibm.com>
> Reviewed-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>

Applied to powerpc fixes, thanks.

https://git.kernel.org/powerpc/c/b0490a04e736356e427e227902b17f

cheers

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-10-12  0:20 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-10  6:45 [PATCH v2] powerpc/lib/sstep: Fix count leading zeros instructions Sandipan Das
2017-10-10  6:54 ` Naveen N. Rao
2017-10-12  0:20 ` [v2] " Michael Ellerman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).