diff mbox

[rtlanal.c] Convert conditional compilation on WORD_REGISTER_OPERATIONS

Message ID 581B280D.4080401@foss.arm.com
State New
Headers show

Commit Message

Kyrill Tkachov Nov. 3, 2016, 12:05 p.m. UTC
On 02/11/16 11:36, Eric Botcazou wrote:
>> I think you're right. I suppose the new condition should be:

>>

>> #ifdef LOAD_EXTEND_OP

>>    	  /* If this is a typical RISC machine, we only have to worry

>>    	     about the way loads are extended.  */

>> 	  if (!WORD_REGISTER_OPERATIONS

>>

>> 	      || ((LOAD_EXTEND_OP (inner_mode) == SIGN_EXTEND

>>

>> 		     ? val_signbit_known_set_p (inner_mode, nonzero)

>>

>> 		     : LOAD_EXTEND_OP (inner_mode) != ZERO_EXTEND)

>> 		   ||

>> 		   || !MEM_P (SUBREG_REG (x))))

>>

>> #endif

> Agreed.

>

>> Would you prefer me to make this change or just revert the patch?

> Go ahead and make the change, but please do a bit of comment massaging in the

> process, for example:

>

> #ifdef LOAD_EXTEND_OP

>            /* On many CISC machines, accessing an object in a wider mode

> 	     causes the high-order bits to become undefined.  So they are

> 	     not known to be zero.  */

>            if (!WORD_REGISTER_OPERATIONS

> 	     /* If this is a typical RISC machine, we only have to worry

>         		about the way loads are extended.  */

>                || ((LOAD_EXTEND_OP (inner_mode) == SIGN_EXTEND

>                       ? val_signbit_known_set_p (inner_mode, nonzero)

>                       : LOAD_EXTEND_OP (inner_mode) != ZERO_EXTEND)

>                     || !MEM_P (SUBREG_REG (x))))

> #endif

> 	    {

> 	      if (GET_MODE_PRECISION (GET_MODE (x))

> 		  > GET_MODE_PRECISION (inner_mode))

> 		nonzero |= (GET_MODE_MASK (GET_MODE (x))

> 			    & ~GET_MODE_MASK (inner_mode));

> 	    }

>


Thanks, here is the patch doing this.
Committing to trunk after bootstrap and testing on x86_64.

Sorry for the trouble,
Kyrill

Comments

Kyrill Tkachov Nov. 3, 2016, 12:07 p.m. UTC | #1
On 03/11/16 12:05, Kyrill Tkachov wrote:
>

> On 02/11/16 11:36, Eric Botcazou wrote:

>>> I think you're right. I suppose the new condition should be:

>>>

>>> #ifdef LOAD_EXTEND_OP

>>>          /* If this is a typical RISC machine, we only have to worry

>>>             about the way loads are extended.  */

>>>       if (!WORD_REGISTER_OPERATIONS

>>>

>>>           || ((LOAD_EXTEND_OP (inner_mode) == SIGN_EXTEND

>>>

>>>              ? val_signbit_known_set_p (inner_mode, nonzero)

>>>

>>>              : LOAD_EXTEND_OP (inner_mode) != ZERO_EXTEND)

>>>            ||

>>>            || !MEM_P (SUBREG_REG (x))))

>>>

>>> #endif

>> Agreed.

>>

>>> Would you prefer me to make this change or just revert the patch?

>> Go ahead and make the change, but please do a bit of comment massaging in the

>> process, for example:

>>

>> #ifdef LOAD_EXTEND_OP

>>            /* On many CISC machines, accessing an object in a wider mode

>>          causes the high-order bits to become undefined.  So they are

>>          not known to be zero.  */

>>            if (!WORD_REGISTER_OPERATIONS

>>          /* If this is a typical RISC machine, we only have to worry

>>                 about the way loads are extended.  */

>>                || ((LOAD_EXTEND_OP (inner_mode) == SIGN_EXTEND

>>                       ? val_signbit_known_set_p (inner_mode, nonzero)

>>                       : LOAD_EXTEND_OP (inner_mode) != ZERO_EXTEND)

>>                     || !MEM_P (SUBREG_REG (x))))

>> #endif

>>         {

>>           if (GET_MODE_PRECISION (GET_MODE (x))

>>           > GET_MODE_PRECISION (inner_mode))

>>         nonzero |= (GET_MODE_MASK (GET_MODE (x))

>>                 & ~GET_MODE_MASK (inner_mode));

>>         }

>>

>

> Thanks, here is the patch doing this.

> Committing to trunk after bootstrap and testing on x86_64.

>

> Sorry for the trouble,

> Kyrill


With the following ChangeLog entry:

2016-11-03  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     * rtlanal.c (nonzero_bits1): Fix WORD_REGISTER_OPERATIONS condition.
     Move comments into more natural position.
Eric Botcazou Nov. 3, 2016, 12:16 p.m. UTC | #2
> Thanks, here is the patch doing this.

> Committing to trunk after bootstrap and testing on x86_64.


Thanks for the quick turn around!

-- 
Eric Botcazou
diff mbox

Patch

diff --git a/gcc/rtlanal.c b/gcc/rtlanal.c
index 9107af0fda76fe2233bc5cf1e439b9971d6691f0..b655315f380f221abcf9176635441688d1756b1e 100644
--- a/gcc/rtlanal.c
+++ b/gcc/rtlanal.c
@@ -4568,18 +4568,18 @@  nonzero_bits1 (const_rtx x, machine_mode mode, const_rtx known_x,
 					  known_x, known_mode, known_ret);
 
 #ifdef LOAD_EXTEND_OP
-	  /* If this is a typical RISC machine, we only have to worry
-	     about the way loads are extended.  */
-	  if (WORD_REGISTER_OPERATIONS
-	      && ((LOAD_EXTEND_OP (inner_mode) == SIGN_EXTEND
+          /* On many CISC machines, accessing an object in a wider mode
+	     causes the high-order bits to become undefined.  So they are
+	     not known to be zero.  */
+	  if (!WORD_REGISTER_OPERATIONS
+	      /* If this is a typical RISC machine, we only have to worry
+		 about the way loads are extended.  */
+	      || ((LOAD_EXTEND_OP (inner_mode) == SIGN_EXTEND
 		     ? val_signbit_known_set_p (inner_mode, nonzero)
 		     : LOAD_EXTEND_OP (inner_mode) != ZERO_EXTEND)
 		   || !MEM_P (SUBREG_REG (x))))
 #endif
 	    {
-	      /* On many CISC machines, accessing an object in a wider mode
-		 causes the high-order bits to become undefined.  So they are
-		 not known to be zero.  */
 	      if (GET_MODE_PRECISION (GET_MODE (x))
 		  > GET_MODE_PRECISION (inner_mode))
 		nonzero |= (GET_MODE_MASK (GET_MODE (x))