diff mbox series

[rs6000] Correct return value of check_p9modulo_hw_available

Message ID 315e43e5-3167-50de-08ec-ab83202e55a0@linux.ibm.com
State New, archived
Headers
Series [rs6000] Correct return value of check_p9modulo_hw_available |

Commit Message

HAO CHEN GUI Aug. 4, 2022, 9:55 a.m. UTC 
  Hi,
  This patch corrects return value of check_p9modulo_hw_available. It should
return 0 when p9modulo is supported.

  Bootstrapped and tested on powerpc64-linux BE and LE with no regressions.
Is this okay for trunk? Any recommendations? Thanks a lot.

ChangeLog
2022-08-04  Haochen Gui  <guihaoc@linux.ibm.com>

gcc/testsuite/
	* lib/target-supports.exp (check_p9modulo_hw_available): Correct return
	value.


patch.diff

Comments

Kewen.Lin Aug. 4, 2022, 11:12 a.m. UTC | #1 
Hi Haochen,

on 2022/8/4 17:55, HAO CHEN GUI wrote:
> Hi,
>   This patch corrects return value of check_p9modulo_hw_available. It should
> return 0 when p9modulo is supported.

Good catch!  There is no case using p9modulo_hw for now, no coverage, sigh...

> 
>   Bootstrapped and tested on powerpc64-linux BE and LE with no regressions.
> Is this okay for trunk? Any recommendations? Thanks a lot.

This patch is OK, thanks!

BR,
Kewen

> 
> ChangeLog
> 2022-08-04  Haochen Gui  <guihaoc@linux.ibm.com>
> 
> gcc/testsuite/
> 	* lib/target-supports.exp (check_p9modulo_hw_available): Correct return
> 	value.
> 
> 
> patch.diff
> diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
> index 4ed7b25b9a4..04a2a8e8659 100644
> --- a/gcc/testsuite/lib/target-supports.exp
> +++ b/gcc/testsuite/lib/target-supports.exp
> @@ -2288,7 +2288,7 @@ proc check_p9modulo_hw_available { } {
>  		{
>  		    int i = 5, j = 3, r = -1;
>  		    asm ("modsw %0,%1,%2" : "+r" (r) : "r" (i), "r" (j));
> -		    return (r == 2);
> +		    return (r != 2);
>  		}
>  	    } $options
>  	}
Segher Boessenkool Aug. 4, 2022, 5:09 p.m. UTC | #2 
Hi!

On Thu, Aug 04, 2022 at 05:55:20PM +0800, HAO CHEN GUI wrote:
>   This patch corrects return value of check_p9modulo_hw_available. It should
> return 0 when p9modulo is supported.

It would be harder to make such mistakes if it used exit() explicitly,
so that the reader is reminded the shell semantics are used here instead
of the C conventions.

> -		    return (r == 2);
> +		    return (r != 2);

so that then would be smth like

	if (r == 2)
		exit (0);
	else
		exit (1);

(which makes the exit code for failure explicit as well).

Terse is good.  Explicit is good as well :-)

(You don't have to make this change here of course, but keep it in mind
for the future :-) )


Segher
HAO CHEN GUI Aug. 5, 2022, 3:05 a.m. UTC | #3 
Hi Segher,
  Thanks so much for your explanation. Now I have a clear picture about
the usage of return value. Patch was committed as r13-1971.

Thanks
Gui Haochen


On 5/8/2022 上午 1:09, Segher Boessenkool wrote:
> Hi!
> 
> On Thu, Aug 04, 2022 at 05:55:20PM +0800, HAO CHEN GUI wrote:
>>   This patch corrects return value of check_p9modulo_hw_available. It should
>> return 0 when p9modulo is supported.
> 
> It would be harder to make such mistakes if it used exit() explicitly,
> so that the reader is reminded the shell semantics are used here instead
> of the C conventions.
> 
>> -		    return (r == 2);
>> +		    return (r != 2);
> 
> so that then would be smth like
> 
> 	if (r == 2)
> 		exit (0);
> 	else
> 		exit (1);
> 
> (which makes the exit code for failure explicit as well).
> 
> Terse is good.  Explicit is good as well :-)
> 
> (You don't have to make this change here of course, but keep it in mind
> for the future :-) )
> 
> 
> Segher
diff mbox series

Patch

diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 4ed7b25b9a4..04a2a8e8659 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -2288,7 +2288,7 @@  proc check_p9modulo_hw_available { } {
 		{
 		    int i = 5, j = 3, r = -1;
 		    asm ("modsw %0,%1,%2" : "+r" (r) : "r" (i), "r" (j));
-		    return (r == 2);
+		    return (r != 2);
 		}
 	    } $options
 	}