[PATCH] arch/sparc/math-emu/math_32.c: drop stray break operator

[PATCH] arch/sparc/math-emu/math_32.c: drop stray break operator

[Andrey Utkin]

This commit is a guesswork, but it seems to make sense to drop this
break, as otherwise the following line is never executed and becomes
dead code. And that following line actually saves the result of
local calculation by the pointer given in function argument. So the
proposed change makes sense if this code in the whole makes sense (but I
am unable to analyze it in the whole).

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=81641
Reported-by: David Binderman <dcb314@hotmail.com>
Signed-off-by: Andrey Utkin <andrey.krieger.utkin@gmail.com>
---
 arch/sparc/math-emu/math_32.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/sparc/math-emu/math_32.c b/arch/sparc/math-emu/math_32.c
index aa4d55b..5ce8f2f 100644
--- a/arch/sparc/math-emu/math_32.c
+++ b/arch/sparc/math-emu/math_32.c
@@ -499,7 +499,7 @@ static int do_one_mathemu(u32 insn, unsigned long *pfsr, unsigned long *fregs)
 		case 0: fsr = *pfsr;
 			if (IR == -1) IR = 2;
 			/* fcc is always fcc0 */
-			fsr &= ~0xc00; fsr |= (IR << 10); break;
+			fsr &= ~0xc00; fsr |= (IR << 10);
 			*pfsr = fsr;
 			break;
 		case 1: rd->s = IR; break;
-- 
1.8.5.5

Re: [PATCH] arch/sparc/math-emu/math_32.c: drop stray break operator

[David Miller]

From: Andrey Utkin <andrey.krieger.utkin@gmail.com>
Date: Mon,  4 Aug 2014 23:47:41 +0300

> This commit is a guesswork, but it seems to make sense to drop this
> break, as otherwise the following line is never executed and becomes
> dead code. And that following line actually saves the result of
> local calculation by the pointer given in function argument. So the
> proposed change makes sense if this code in the whole makes sense (but I
> am unable to analyze it in the whole).
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=81641
> Reported-by: David Binderman <dcb314@hotmail.com>
> Signed-off-by: Andrey Utkin <andrey.krieger.utkin@gmail.com>

Yeah, not putting the condition code result into the register might
cause problems :-)

Thanks for catching this, applied and queued up for -stable.

Re: [PATCH] arch/sparc/math-emu/math_32.c: drop stray break operator

[walter harms]

Am 04.08.2014 22:47, schrieb Andrey Utkin:
> This commit is a guesswork, but it seems to make sense to drop this
> break, as otherwise the following line is never executed and becomes
> dead code. And that following line actually saves the result of
> local calculation by the pointer given in function argument. So the
> proposed change makes sense if this code in the whole makes sense (but I
> am unable to analyze it in the whole).
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=81641
> Reported-by: David Binderman <dcb314@hotmail.com>
> Signed-off-by: Andrey Utkin <andrey.krieger.utkin@gmail.com>
> ---
>  arch/sparc/math-emu/math_32.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/sparc/math-emu/math_32.c b/arch/sparc/math-emu/math_32.c
> index aa4d55b..5ce8f2f 100644
> --- a/arch/sparc/math-emu/math_32.c
> +++ b/arch/sparc/math-emu/math_32.c
> @@ -499,7 +499,7 @@ static int do_one_mathemu(u32 insn, unsigned long *pfsr, unsigned long *fregs)
>  		case 0: fsr = *pfsr;
>  			if (IR == -1) IR = 2;
>  			/* fcc is always fcc0 */

The patch looks ok, but can somebody comment on this comment ?
what "fcc" ? should it be a fsr ?


> -			fsr &= ~0xc00; fsr |= (IR << 10); break;
> +			fsr &= ~0xc00; fsr |= (IR << 10);

nitpicking:
   fsr &= ~0xc00;
   fsr |= (IR << 10);

It is better readable.

re,
 wh

>  			*pfsr = fsr;
>  			break;
>  		case 1: rd->s = IR; break;

Re: [PATCH] arch/sparc/math-emu/math_32.c: drop stray break operator

[David Miller]

From: walter harms <wharms@bfs.de>
Date: Tue, 05 Aug 2014 09:11:52 +0200

> 
> 
> Am 04.08.2014 22:47, schrieb Andrey Utkin:
>> This commit is a guesswork, but it seems to make sense to drop this
>> break, as otherwise the following line is never executed and becomes
>> dead code. And that following line actually saves the result of
>> local calculation by the pointer given in function argument. So the
>> proposed change makes sense if this code in the whole makes sense (but I
>> am unable to analyze it in the whole).
>> 
>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=81641
>> Reported-by: David Binderman <dcb314@hotmail.com>
>> Signed-off-by: Andrey Utkin <andrey.krieger.utkin@gmail.com>
>> ---
>>  arch/sparc/math-emu/math_32.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/arch/sparc/math-emu/math_32.c b/arch/sparc/math-emu/math_32.c
>> index aa4d55b..5ce8f2f 100644
>> --- a/arch/sparc/math-emu/math_32.c
>> +++ b/arch/sparc/math-emu/math_32.c
>> @@ -499,7 +499,7 @@ static int do_one_mathemu(u32 insn, unsigned long *pfsr, unsigned long *fregs)
>>  		case 0: fsr = *pfsr;
>>  			if (IR == -1) IR = 2;
>>  			/* fcc is always fcc0 */
> 
> The patch looks ok, but can somebody comment on this comment ?
> what "fcc" ? should it be a fsr ?

It's the condition code field inside of the %fsr register.

In 32-bit chips there is only one set of condition codes, whereas
on 64-bit chips there are 4 sets referred to as fcc0, fcc1, fcc2,
and fcc3.

That's what this comment is talking about.

Re: [PATCH] arch/sparc/math-emu/math_32.c: drop stray break operator

[walter harms]

Am 05.08.2014 21:49, schrieb David Miller:
> From: walter harms <wharms@bfs.de>
> Date: Tue, 05 Aug 2014 09:11:52 +0200
> 
>>
>>
>> Am 04.08.2014 22:47, schrieb Andrey Utkin:
>>> This commit is a guesswork, but it seems to make sense to drop this
>>> break, as otherwise the following line is never executed and becomes
>>> dead code. And that following line actually saves the result of
>>> local calculation by the pointer given in function argument. So the
>>> proposed change makes sense if this code in the whole makes sense (but I
>>> am unable to analyze it in the whole).
>>>
>>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=81641
>>> Reported-by: David Binderman <dcb314@hotmail.com>
>>> Signed-off-by: Andrey Utkin <andrey.krieger.utkin@gmail.com>
>>> ---
>>>  arch/sparc/math-emu/math_32.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/sparc/math-emu/math_32.c b/arch/sparc/math-emu/math_32.c
>>> index aa4d55b..5ce8f2f 100644
>>> --- a/arch/sparc/math-emu/math_32.c
>>> +++ b/arch/sparc/math-emu/math_32.c
>>> @@ -499,7 +499,7 @@ static int do_one_mathemu(u32 insn, unsigned long *pfsr, unsigned long *fregs)
>>>  		case 0: fsr = *pfsr;
>>>  			if (IR == -1) IR = 2;
>>>  			/* fcc is always fcc0 */
>>
>> The patch looks ok, but can somebody comment on this comment ?
>> what "fcc" ? should it be a fsr ?
> 
> It's the condition code field inside of the %fsr register.
> 
> In 32-bit chips there is only one set of condition codes, whereas
> on 64-bit chips there are 4 sets referred to as fcc0, fcc1, fcc2,
> and fcc3.
> 
> That's what this comment is talking about.
> 
thx for info,

would you mind to add your explanation ?
It is much mor helpful that the comment right now.

re,
 wh