rs6000: Change bitwise xor to inequality operator [PR106907]

Message ID a09039c6-3535-c7c9-8755-acaaabc1278a@linux.vnet.ibm.com
State Accepted
Headers
Series rs6000: Change bitwise xor to inequality operator [PR106907] |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

jeevitha June 12, 2023, 11:18 a.m. UTC
  PR106907 has few warnings spotted from cppcheck. Here we have
warnings for precedence clarification since boolean results are
used in bitwise operation. Bitwise xor performed on bool
is similar to checking inequality. So changed to inequality
operator (!=) instead of bitwise xor (^). And fixed comment indentation

2023-06-12  Jeevitha Palanisamy  <jeevitha@linux.ibm.com>

gcc/
	PR target/106907
	* config/rs6000/rs6000.cc (altivec_expand_vec_perm_const): Change bitwise
	xor to inequality and fix comment indentation.
  

Comments

Peter Bergner June 16, 2023, 4:25 a.m. UTC | #1
On 6/12/23 6:18 AM, P Jeevitha wrote:
> Bitwise xor performed on bool
> is similar to checking inequality. So changed to inequality
> operator (!=) instead of bitwise xor (^).
[snip'
> -	  if (swapped ^ !BYTES_BIG_ENDIAN
[snip]
> +	  if (swapped != !BYTES_BIG_ENDIAN

I know Andreas mentioned using "swapped != !BYTES_BIG_ENDIAN" in
the bugzilla, but that's the same as "swapped == BYTES_BIG_ENDIAN",
and it doesn't contain a double-negative and seems a little clearer.

It's up to Segher though...and if we go with this, then the ChangeLog
entry needs to be updated slightly since we're no longer testing for
inequality.

Peter
  
jeevitha Oct. 8, 2023, 6:09 p.m. UTC | #2
Ping!

please review.

Thanks & Regards
Jeevitha

On 16/06/23 9:55 am, Peter Bergner wrote:
> On 6/12/23 6:18 AM, P Jeevitha wrote:
>> Bitwise xor performed on bool
>> is similar to checking inequality. So changed to inequality
>> operator (!=) instead of bitwise xor (^).
> [snip'
>> -	  if (swapped ^ !BYTES_BIG_ENDIAN
> [snip]
>> +	  if (swapped != !BYTES_BIG_ENDIAN
> 
> I know Andreas mentioned using "swapped != !BYTES_BIG_ENDIAN" in
> the bugzilla, but that's the same as "swapped == BYTES_BIG_ENDIAN",
> and it doesn't contain a double-negative and seems a little clearer.
> 
> It's up to Segher though...and if we go with this, then the ChangeLog
> entry needs to be updated slightly since we're no longer testing for
> inequality.
> 
> Peter
>
  

Patch

diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
index ea68ca6faef..ea7efda8dcd 100644
--- a/gcc/config/rs6000/rs6000.cc
+++ b/gcc/config/rs6000/rs6000.cc
@@ -23396,10 +23396,10 @@  altivec_expand_vec_perm_const (rtx target, rtx op0, rtx op1,
 		      && GET_MODE (XEXP (op0, 0)) != V8HImode)))
 	    continue;
 
-          /* For little-endian, the two input operands must be swapped
-             (or swapped back) to ensure proper right-to-left numbering
-             from 0 to 2N-1.  */
-	  if (swapped ^ !BYTES_BIG_ENDIAN
+	  /* For little-endian, the two input operands must be swapped
+	     (or swapped back) to ensure proper right-to-left numbering
+	     from 0 to 2N-1.  */
+	  if (swapped != !BYTES_BIG_ENDIAN
 	      && icode != CODE_FOR_vsx_xxpermdi_v16qi)
 	    std::swap (op0, op1);
 	  if (imode != V16QImode)