On Mon, Aug 24, 2020 at 07:54:07PM +0800, Guohua Zhong wrote:
Yet, I have noticed that there is no checking of 'base' in these functions. But I am not sure how to check is better.As we know that the result is undefined when divisor is zero. It maybe good to print error and dump stack. Let the process to know that the divisor is zero by sending SIGFPE.
That is now what the PowerPC integer divide insns do: they just leave the result undefined (and they can set the overflow flag then, but no one uses that).
OK ,So just keep the patch as below. If this patch looks good for you, please help to review. I will send the new patch later.
Thanks for your reply.
diff --git a/arch/powerpc/boot/div64.S b/arch/powerpc/boot/div64.S index 4354928ed62e..1d3561cf16fa 100644 --- a/arch/powerpc/boot/div64.S +++ b/arch/powerpc/boot/div64.S @@ -13,8 +13,10 @@
.globl __div64_32 .globl __div64_32__div64_32:
- cmplwi r4,0 # check if divisor r4 is zero lwz r5,0(r3) # get the dividend into r5/r6 lwz r6,4(r3)
- beq 5f # jump to label 5 if r4(divisor) is zero
Just "beqlr".
This instruction scheduling hurts all CPUs that aren't 8xx, fwiw (but likely only in the case where r4 *is* zero, so who cares :-) )
So... What is the *goal* of this patch? It looks like the routine would not get into a loop if r4 is 0, just return the wrong result? But, it *always* will, there *is* no right result?
No caller should call it with zero as divisor ever, so in that sense, checking for it in the division routine is just pure wasted work.
Segher