[PATCH 0/2] Set REG_EQUAL when emitting arm_emit_movpair

[PATCH 0/2] Set REG_EQUAL when emitting arm_emit_movpair

[Kugan]

When we split constants with the arm_emit_movpair, we are not setting
the REG_EQUAL note. This patch attempts to do that.

Fist patch allow setting REG_EQUAL for ZERO_EXTRACT and handle that in
cse (where the src for the ZERO_EXTRACT needs to be calculated)
Second patch sets REG_EQUAL when emitting arm_emit_movpair.

Bootstrapped and regression tested on arm-none-linux (Chromebook) and
x86-64-linux-gnu with no new regression.

Is this OK for trunk,

Thanks,
Kugan

[PATCH 1/2] Allow REG_EQUAL for ZERO_EXTRACT

[Kugan]

[-- Attachment #1: Type: text/plain, Size: 345 bytes --]

This patch allows setting REG_EQUAL for ZERO_EXTRACT and handle that in
cse (where the src for the ZERO_EXTRACT needs to be calculated)

Thanks,
Kugan


2015-06-26  Kugan Vivekanandarajah  <kuganv@linaro.org>

	* cse.c (cse_insn): Calculate src_eqv for ZERO_EXTRACT.
	* emit-rtl.c (set_for_reg_notes): Allow ZERO_EXTRACT to set
	REG_EQUAL note.

[-- Attachment #2: 0001-Allow-adding-REG_EQUAL-for-ZERO_EXTRACT.patch --]
[-- Type: text/x-diff, Size: 3259 bytes --]

From 75e746e559ffd21b25542b3db627e3b318118569 Mon Sep 17 00:00:00 2001
From: Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
Date: Fri, 26 Jun 2015 17:12:07 +1000
Subject: [PATCH 1/2] Allow adding REG_EQUAL for ZERO_EXTRACT

---
 gcc/ChangeLog  |  6 ++++++
 gcc/cse.c      | 41 ++++++++++++++++++++++++++++++++++++++++-
 gcc/emit-rtl.c |  3 ++-
 3 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 080aa39..d4a73d6 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@
+2015-06-26  Kugan Vivekanandarajah  <kuganv@linaro.org>
+
+	* cse.c (cse_insn): Calculate src_eqv for ZERO_EXTRACT.
+	* emit-rtl.c (set_for_reg_notes): Allow ZERO_EXTRACT to set
+	REG_EQUAL note.
+
 2015-06-25  H.J. Lu  <hongjiu.lu@intel.com>
 
 	* gentarget-def.c (def_target_insn): Cast return of strtol to
diff --git a/gcc/cse.c b/gcc/cse.c
index 100c9c8..8add651 100644
--- a/gcc/cse.c
+++ b/gcc/cse.c
@@ -4531,8 +4531,47 @@ cse_insn (rtx_insn *insn)
   if (n_sets == 1 && REG_NOTES (insn) != 0
       && (tem = find_reg_note (insn, REG_EQUAL, NULL_RTX)) != 0
       && (! rtx_equal_p (XEXP (tem, 0), SET_SRC (sets[0].rtl))
+	  || GET_CODE (SET_DEST (sets[0].rtl)) == ZERO_EXTRACT
 	  || GET_CODE (SET_DEST (sets[0].rtl)) == STRICT_LOW_PART))
-    src_eqv = copy_rtx (XEXP (tem, 0));
+    {
+      src_eqv = copy_rtx (XEXP (tem, 0));
+
+      /* If DEST is of the form ZERO_EXTACT, as in:
+	 (set (zero_extract:SI (reg:SI 119)
+		  (const_int 16 [0x10])
+		  (const_int 16 [0x10]))
+	      (const_int 51154 [0xc7d2]))
+	 REG_EQUAL note will specify the value of register (reg:SI 119) at this
+	 point.  Note that this is different from SRC_EQV. We can however
+	 calculate SRC_EQV with the position and width of ZERO_EXTRACT.  */
+      if (GET_CODE (SET_DEST (sets[0].rtl)) == ZERO_EXTRACT)
+	{
+	  if (CONST_INT_P (src_eqv)
+	      && CONST_INT_P (XEXP (SET_DEST (sets[0].rtl), 1))
+	      && CONST_INT_P (XEXP (SET_DEST (sets[0].rtl), 2)))
+	    {
+	      rtx dest_reg = XEXP (SET_DEST (sets[0].rtl), 0);
+	      rtx width = XEXP (SET_DEST (sets[0].rtl), 1);
+	      rtx pos = XEXP (SET_DEST (sets[0].rtl), 2);
+	      HOST_WIDE_INT val = INTVAL (src_eqv);
+	      HOST_WIDE_INT mask;
+	      unsigned int shift;
+	      if (BITS_BIG_ENDIAN)
+		shift = GET_MODE_PRECISION (GET_MODE (dest_reg))
+		  - INTVAL (pos) - INTVAL (width);
+	      else
+		shift = INTVAL (pos);
+	      if (INTVAL (width) == HOST_BITS_PER_WIDE_INT)
+		mask = ~(HOST_WIDE_INT) 0;
+	      else
+		mask = ((HOST_WIDE_INT) 1 << INTVAL (width)) - 1;
+	      val = (val >> shift) & mask;
+	      src_eqv = GEN_INT (val);
+	    }
+	  else
+	    src_eqv = 0;
+	}
+    }
 
   /* Set sets[i].src_elt to the class each source belongs to.
      Detect assignments from or to volatile things
diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index e7f7eab..cb891b1 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -5228,7 +5228,8 @@ set_for_reg_notes (rtx insn)
   reg = SET_DEST (pat);
 
   /* Notes apply to the contents of a STRICT_LOW_PART.  */
-  if (GET_CODE (reg) == STRICT_LOW_PART)
+  if (GET_CODE (reg) == STRICT_LOW_PART
+      || GET_CODE (reg) == ZERO_EXTRACT)
     reg = XEXP (reg, 0);
 
   /* Check that we have a register.  */
-- 
1.9.1

Re: [PATCH 1/2] Allow REG_EQUAL for ZERO_EXTRACT

[Maxim Kuvyrkov]

> On Jun 28, 2015, at 2:28 PM, Kugan <kugan.vivekanandarajah@linaro.org> wrote:
> 
> This patch allows setting REG_EQUAL for ZERO_EXTRACT and handle that in
> cse (where the src for the ZERO_EXTRACT needs to be calculated)
> 
> Thanks,
> Kugan

> From 75e746e559ffd21b25542b3db627e3b318118569 Mon Sep 17 00:00:00 2001
> From: Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
> Date: Fri, 26 Jun 2015 17:12:07 +1000
> Subject: [PATCH 1/2] Allow adding REG_EQUAL for ZERO_EXTRACT
> 
> ---
>  gcc/ChangeLog  |  6 ++++++
>  gcc/cse.c      | 41 ++++++++++++++++++++++++++++++++++++++++-
>  gcc/emit-rtl.c |  3 ++-
>  3 files changed, 48 insertions(+), 2 deletions(-)
> 
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index 080aa39..d4a73d6 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,9 @@
> +2015-06-26  Kugan Vivekanandarajah  <kuganv@linaro.org>
> +
> +	* cse.c (cse_insn): Calculate src_eqv for ZERO_EXTRACT.
> +	* emit-rtl.c (set_for_reg_notes): Allow ZERO_EXTRACT to set
> +	REG_EQUAL note.
> +
>  2015-06-25  H.J. Lu  <hongjiu.lu@intel.com>
>  
>  	* gentarget-def.c (def_target_insn): Cast return of strtol to
> diff --git a/gcc/cse.c b/gcc/cse.c
> index 100c9c8..8add651 100644
> --- a/gcc/cse.c
> +++ b/gcc/cse.c
> @@ -4531,8 +4531,47 @@ cse_insn (rtx_insn *insn)
>    if (n_sets == 1 && REG_NOTES (insn) != 0
>        && (tem = find_reg_note (insn, REG_EQUAL, NULL_RTX)) != 0
>        && (! rtx_equal_p (XEXP (tem, 0), SET_SRC (sets[0].rtl))
> +	  || GET_CODE (SET_DEST (sets[0].rtl)) == ZERO_EXTRACT
>  	  || GET_CODE (SET_DEST (sets[0].rtl)) == STRICT_LOW_PART))
> -    src_eqv = copy_rtx (XEXP (tem, 0));
> +    {
> +      src_eqv = copy_rtx (XEXP (tem, 0));
> +
> +      /* If DEST is of the form ZERO_EXTACT, as in:
> +	 (set (zero_extract:SI (reg:SI 119)
> +		  (const_int 16 [0x10])
> +		  (const_int 16 [0x10]))
> +	      (const_int 51154 [0xc7d2]))
> +	 REG_EQUAL note will specify the value of register (reg:SI 119) at this
> +	 point.  Note that this is different from SRC_EQV. We can however
> +	 calculate SRC_EQV with the position and width of ZERO_EXTRACT.  */
> +      if (GET_CODE (SET_DEST (sets[0].rtl)) == ZERO_EXTRACT)

Consider changing

if (something
    && (!rtx_equal_p)
        || ZERO_EXTRACT
        || STRICT_LOW_PART)

to 

if (something
    && !rtx_equal_p)
  {
     if (ZERO_EXTRACT)
       {
       }
     else if (STRICT_LOW_PART)
       {
       }
  }

Otherwise looks good to me, but you still need another approval.


> +	{
> +	  if (CONST_INT_P (src_eqv)
> +	      && CONST_INT_P (XEXP (SET_DEST (sets[0].rtl), 1))
> +	      && CONST_INT_P (XEXP (SET_DEST (sets[0].rtl), 2)))
> +	    {
> +	      rtx dest_reg = XEXP (SET_DEST (sets[0].rtl), 0);
> +	      rtx width = XEXP (SET_DEST (sets[0].rtl), 1);
> +	      rtx pos = XEXP (SET_DEST (sets[0].rtl), 2);
> +	      HOST_WIDE_INT val = INTVAL (src_eqv);
> +	      HOST_WIDE_INT mask;
> +	      unsigned int shift;
> +	      if (BITS_BIG_ENDIAN)
> +		shift = GET_MODE_PRECISION (GET_MODE (dest_reg))
> +		  - INTVAL (pos) - INTVAL (width);
> +	      else
> +		shift = INTVAL (pos);
> +	      if (INTVAL (width) == HOST_BITS_PER_WIDE_INT)
> +		mask = ~(HOST_WIDE_INT) 0;
> +	      else
> +		mask = ((HOST_WIDE_INT) 1 << INTVAL (width)) - 1;
> +	      val = (val >> shift) & mask;
> +	      src_eqv = GEN_INT (val);
> +	    }
> +	  else
> +	    src_eqv = 0;
> +	}
> +    }
>  
>    /* Set sets[i].src_elt to the class each source belongs to.
>       Detect assignments from or to volatile things
> diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
> index e7f7eab..cb891b1 100644
> --- a/gcc/emit-rtl.c
> +++ b/gcc/emit-rtl.c
> @@ -5228,7 +5228,8 @@ set_for_reg_notes (rtx insn)
>    reg = SET_DEST (pat);
>  
>    /* Notes apply to the contents of a STRICT_LOW_PART.  */
> -  if (GET_CODE (reg) == STRICT_LOW_PART)
> +  if (GET_CODE (reg) == STRICT_LOW_PART
> +      || GET_CODE (reg) == ZERO_EXTRACT)
>      reg = XEXP (reg, 0);
>  
>    /* Check that we have a register.  */
> -- 
> 1.9.1
> 
> 


--
Maxim Kuvyrkov
www.linaro.org

Re: [PATCH 1/2] Allow REG_EQUAL for ZERO_EXTRACT

[Kugan]

[-- Attachment #1: Type: text/plain, Size: 4333 bytes --]


On 29/06/15 21:56, Maxim Kuvyrkov wrote:
>> On Jun 28, 2015, at 2:28 PM, Kugan <kugan.vivekanandarajah@linaro.org> wrote:
>>
>> This patch allows setting REG_EQUAL for ZERO_EXTRACT and handle that in
>> cse (where the src for the ZERO_EXTRACT needs to be calculated)
>>
>> Thanks,
>> Kugan
> 
>> From 75e746e559ffd21b25542b3db627e3b318118569 Mon Sep 17 00:00:00 2001
>> From: Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
>> Date: Fri, 26 Jun 2015 17:12:07 +1000
>> Subject: [PATCH 1/2] Allow adding REG_EQUAL for ZERO_EXTRACT
>>
>> ---
>>  gcc/ChangeLog  |  6 ++++++
>>  gcc/cse.c      | 41 ++++++++++++++++++++++++++++++++++++++++-
>>  gcc/emit-rtl.c |  3 ++-
>>  3 files changed, 48 insertions(+), 2 deletions(-)
>>
>> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
>> index 080aa39..d4a73d6 100644
>> --- a/gcc/ChangeLog
>> +++ b/gcc/ChangeLog
>> @@ -1,3 +1,9 @@
>> +2015-06-26  Kugan Vivekanandarajah  <kuganv@linaro.org>
>> +
>> +	* cse.c (cse_insn): Calculate src_eqv for ZERO_EXTRACT.
>> +	* emit-rtl.c (set_for_reg_notes): Allow ZERO_EXTRACT to set
>> +	REG_EQUAL note.
>> +
>>  2015-06-25  H.J. Lu  <hongjiu.lu@intel.com>
>>  
>>  	* gentarget-def.c (def_target_insn): Cast return of strtol to
>> diff --git a/gcc/cse.c b/gcc/cse.c
>> index 100c9c8..8add651 100644
>> --- a/gcc/cse.c
>> +++ b/gcc/cse.c
>> @@ -4531,8 +4531,47 @@ cse_insn (rtx_insn *insn)
>>    if (n_sets == 1 && REG_NOTES (insn) != 0
>>        && (tem = find_reg_note (insn, REG_EQUAL, NULL_RTX)) != 0
>>        && (! rtx_equal_p (XEXP (tem, 0), SET_SRC (sets[0].rtl))
>> +	  || GET_CODE (SET_DEST (sets[0].rtl)) == ZERO_EXTRACT
>>  	  || GET_CODE (SET_DEST (sets[0].rtl)) == STRICT_LOW_PART))
>> -    src_eqv = copy_rtx (XEXP (tem, 0));
>> +    {
>> +      src_eqv = copy_rtx (XEXP (tem, 0));
>> +
>> +      /* If DEST is of the form ZERO_EXTACT, as in:
>> +	 (set (zero_extract:SI (reg:SI 119)
>> +		  (const_int 16 [0x10])
>> +		  (const_int 16 [0x10]))
>> +	      (const_int 51154 [0xc7d2]))
>> +	 REG_EQUAL note will specify the value of register (reg:SI 119) at this
>> +	 point.  Note that this is different from SRC_EQV. We can however
>> +	 calculate SRC_EQV with the position and width of ZERO_EXTRACT.  */
>> +      if (GET_CODE (SET_DEST (sets[0].rtl)) == ZERO_EXTRACT)
> 
> Consider changing
> 
> if (something
>     && (!rtx_equal_p)
>         || ZERO_EXTRACT
>         || STRICT_LOW_PART)
> 
> to 
> 
> if (something
>     && !rtx_equal_p)
>   {
>      if (ZERO_EXTRACT)
>        {
>        }
>      else if (STRICT_LOW_PART)
>        {
>        }
>   }
> 
> Otherwise looks good to me, but you still need another approval.

Thanks Maxim for the review. How about the attached patch?

Thanks,
Kugan
> 
> 
>> +	{
>> +	  if (CONST_INT_P (src_eqv)
>> +	      && CONST_INT_P (XEXP (SET_DEST (sets[0].rtl), 1))
>> +	      && CONST_INT_P (XEXP (SET_DEST (sets[0].rtl), 2)))
>> +	    {
>> +	      rtx dest_reg = XEXP (SET_DEST (sets[0].rtl), 0);
>> +	      rtx width = XEXP (SET_DEST (sets[0].rtl), 1);
>> +	      rtx pos = XEXP (SET_DEST (sets[0].rtl), 2);
>> +	      HOST_WIDE_INT val = INTVAL (src_eqv);
>> +	      HOST_WIDE_INT mask;
>> +	      unsigned int shift;
>> +	      if (BITS_BIG_ENDIAN)
>> +		shift = GET_MODE_PRECISION (GET_MODE (dest_reg))
>> +		  - INTVAL (pos) - INTVAL (width);
>> +	      else
>> +		shift = INTVAL (pos);
>> +	      if (INTVAL (width) == HOST_BITS_PER_WIDE_INT)
>> +		mask = ~(HOST_WIDE_INT) 0;
>> +	      else
>> +		mask = ((HOST_WIDE_INT) 1 << INTVAL (width)) - 1;
>> +	      val = (val >> shift) & mask;
>> +	      src_eqv = GEN_INT (val);
>> +	    }
>> +	  else
>> +	    src_eqv = 0;
>> +	}
>> +    }
>>  
>>    /* Set sets[i].src_elt to the class each source belongs to.
>>       Detect assignments from or to volatile things
>> diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
>> index e7f7eab..cb891b1 100644
>> --- a/gcc/emit-rtl.c
>> +++ b/gcc/emit-rtl.c
>> @@ -5228,7 +5228,8 @@ set_for_reg_notes (rtx insn)
>>    reg = SET_DEST (pat);
>>  
>>    /* Notes apply to the contents of a STRICT_LOW_PART.  */
>> -  if (GET_CODE (reg) == STRICT_LOW_PART)
>> +  if (GET_CODE (reg) == STRICT_LOW_PART
>> +      || GET_CODE (reg) == ZERO_EXTRACT)
>>      reg = XEXP (reg, 0);
>>  
>>    /* Check that we have a register.  */
>> -- 
>> 1.9.1
>>
>>
> 
> 
> --
> Maxim Kuvyrkov
> www.linaro.org
> 
> 
> 
> 

[-- Attachment #2: 0001-Allow-adding-REG_EQUAL-for-ZERO_EXTRACT.patch --]
[-- Type: text/x-diff, Size: 3953 bytes --]

From 3754bcd11fb732e03a39d6625a9eb14934b36643 Mon Sep 17 00:00:00 2001
From: Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
Date: Fri, 26 Jun 2015 17:12:07 +1000
Subject: [PATCH 1/2] Allow adding REG_EQUAL for ZERO_EXTRACT

---
 gcc/ChangeLog  |  6 ++++++
 gcc/cse.c      | 49 ++++++++++++++++++++++++++++++++++++++++++-------
 gcc/emit-rtl.c |  3 ++-
 3 files changed, 50 insertions(+), 8 deletions(-)

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 080aa39..d4a73d6 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@
+2015-06-26  Kugan Vivekanandarajah  <kuganv@linaro.org>
+
+	* cse.c (cse_insn): Calculate src_eqv for ZERO_EXTRACT.
+	* emit-rtl.c (set_for_reg_notes): Allow ZERO_EXTRACT to set
+	REG_EQUAL note.
+
 2015-06-25  H.J. Lu  <hongjiu.lu@intel.com>
 
 	* gentarget-def.c (def_target_insn): Cast return of strtol to
diff --git a/gcc/cse.c b/gcc/cse.c
index 100c9c8..ea9e989 100644
--- a/gcc/cse.c
+++ b/gcc/cse.c
@@ -4525,14 +4525,49 @@ cse_insn (rtx_insn *insn)
   canonicalize_insn (insn, &sets, n_sets);
 
   /* If this insn has a REG_EQUAL note, store the equivalent value in SRC_EQV,
-     if different, or if the DEST is a STRICT_LOW_PART.  The latter condition
-     is necessary because SRC_EQV is handled specially for this case, and if
-     it isn't set, then there will be no equivalence for the destination.  */
+     if different, or if the DEST is a STRICT_LOW_PART/ZERO_EXTRACT.  The
+     latter condition is necessary because SRC_EQV is handled specially for
+     this case, and if it isn't set, then there will be no equivalence
+     for the destination.  */
   if (n_sets == 1 && REG_NOTES (insn) != 0
-      && (tem = find_reg_note (insn, REG_EQUAL, NULL_RTX)) != 0
-      && (! rtx_equal_p (XEXP (tem, 0), SET_SRC (sets[0].rtl))
-	  || GET_CODE (SET_DEST (sets[0].rtl)) == STRICT_LOW_PART))
-    src_eqv = copy_rtx (XEXP (tem, 0));
+      && (tem = find_reg_note (insn, REG_EQUAL, NULL_RTX)) != 0)
+    {
+      if ((! rtx_equal_p (XEXP (tem, 0), SET_SRC (sets[0].rtl)))
+	  || GET_CODE (SET_DEST (sets[0].rtl)) == STRICT_LOW_PART)
+      src_eqv = copy_rtx (XEXP (tem, 0));
+
+      /* If DEST is of the form ZERO_EXTACT, as in:
+	 (set (zero_extract:SI (reg:SI 119)
+		  (const_int 16 [0x10])
+		  (const_int 16 [0x10]))
+	      (const_int 51154 [0xc7d2]))
+	 REG_EQUAL note will specify the value of register (reg:SI 119) at this
+	 point.  Note that this is different from SRC_EQV. We can however
+	 calculate SRC_EQV with the position and width of ZERO_EXTRACT.  */
+      else if (GET_CODE (SET_DEST (sets[0].rtl)) == ZERO_EXTRACT
+	       &&CONST_INT_P (src_eqv)
+	       && CONST_INT_P (XEXP (SET_DEST (sets[0].rtl), 1))
+	       && CONST_INT_P (XEXP (SET_DEST (sets[0].rtl), 2)))
+	{
+	  rtx dest_reg = XEXP (SET_DEST (sets[0].rtl), 0);
+	  rtx width = XEXP (SET_DEST (sets[0].rtl), 1);
+	  rtx pos = XEXP (SET_DEST (sets[0].rtl), 2);
+	  HOST_WIDE_INT val = INTVAL (src_eqv);
+	  HOST_WIDE_INT mask;
+	  unsigned int shift;
+	  if (BITS_BIG_ENDIAN)
+	    shift = GET_MODE_PRECISION (GET_MODE (dest_reg))
+	      - INTVAL (pos) - INTVAL (width);
+	  else
+	    shift = INTVAL (pos);
+	  if (INTVAL (width) == HOST_BITS_PER_WIDE_INT)
+	    mask = ~(HOST_WIDE_INT) 0;
+	  else
+	    mask = ((HOST_WIDE_INT) 1 << INTVAL (width)) - 1;
+	  val = (val >> shift) & mask;
+	  src_eqv = GEN_INT (val);
+	}
+    }
 
   /* Set sets[i].src_elt to the class each source belongs to.
      Detect assignments from or to volatile things
diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index e7f7eab..cb891b1 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -5228,7 +5228,8 @@ set_for_reg_notes (rtx insn)
   reg = SET_DEST (pat);
 
   /* Notes apply to the contents of a STRICT_LOW_PART.  */
-  if (GET_CODE (reg) == STRICT_LOW_PART)
+  if (GET_CODE (reg) == STRICT_LOW_PART
+      || GET_CODE (reg) == ZERO_EXTRACT)
     reg = XEXP (reg, 0);
 
   /* Check that we have a register.  */
-- 
1.9.1

Re: [PATCH 1/2] Allow REG_EQUAL for ZERO_EXTRACT

[Maxim Kuvyrkov]

> On Jun 30, 2015, at 6:54 AM, Kugan <kugan.vivekanandarajah@linaro.org> wrote:
> 
> 
> On 29/06/15 21:56, Maxim Kuvyrkov wrote:
>>> On Jun 28, 2015, at 2:28 PM, Kugan <kugan.vivekanandarajah@linaro.org> wrote:
>>> 
>>> This patch allows setting REG_EQUAL for ZERO_EXTRACT and handle that in
>>> cse (where the src for the ZERO_EXTRACT needs to be calculated)
>>> 
>>> Thanks,
>>> Kugan
>> 
>>> From 75e746e559ffd21b25542b3db627e3b318118569 Mon Sep 17 00:00:00 2001
>>> From: Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
>>> Date: Fri, 26 Jun 2015 17:12:07 +1000
>>> Subject: [PATCH 1/2] Allow adding REG_EQUAL for ZERO_EXTRACT
>>> 
>>> ---
>>> gcc/ChangeLog  |  6 ++++++
>>> gcc/cse.c      | 41 ++++++++++++++++++++++++++++++++++++++++-
>>> gcc/emit-rtl.c |  3 ++-
>>> 3 files changed, 48 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
>>> index 080aa39..d4a73d6 100644
>>> --- a/gcc/ChangeLog
>>> +++ b/gcc/ChangeLog
>>> @@ -1,3 +1,9 @@
>>> +2015-06-26  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>> +
>>> +	* cse.c (cse_insn): Calculate src_eqv for ZERO_EXTRACT.
>>> +	* emit-rtl.c (set_for_reg_notes): Allow ZERO_EXTRACT to set
>>> +	REG_EQUAL note.
>>> +
>>> 2015-06-25  H.J. Lu  <hongjiu.lu@intel.com>
>>> 
>>> 	* gentarget-def.c (def_target_insn): Cast return of strtol to
>>> diff --git a/gcc/cse.c b/gcc/cse.c
>>> index 100c9c8..8add651 100644
>>> --- a/gcc/cse.c
>>> +++ b/gcc/cse.c
>>> @@ -4531,8 +4531,47 @@ cse_insn (rtx_insn *insn)
>>>   if (n_sets == 1 && REG_NOTES (insn) != 0
>>>       && (tem = find_reg_note (insn, REG_EQUAL, NULL_RTX)) != 0
>>>       && (! rtx_equal_p (XEXP (tem, 0), SET_SRC (sets[0].rtl))
>>> +	  || GET_CODE (SET_DEST (sets[0].rtl)) == ZERO_EXTRACT
>>> 	  || GET_CODE (SET_DEST (sets[0].rtl)) == STRICT_LOW_PART))
>>> -    src_eqv = copy_rtx (XEXP (tem, 0));
>>> +    {
>>> +      src_eqv = copy_rtx (XEXP (tem, 0));
>>> +
>>> +      /* If DEST is of the form ZERO_EXTACT, as in:
>>> +	 (set (zero_extract:SI (reg:SI 119)
>>> +		  (const_int 16 [0x10])
>>> +		  (const_int 16 [0x10]))
>>> +	      (const_int 51154 [0xc7d2]))
>>> +	 REG_EQUAL note will specify the value of register (reg:SI 119) at this
>>> +	 point.  Note that this is different from SRC_EQV. We can however
>>> +	 calculate SRC_EQV with the position and width of ZERO_EXTRACT.  */
>>> +      if (GET_CODE (SET_DEST (sets[0].rtl)) == ZERO_EXTRACT)
>> 
>> Consider changing
>> 
>> if (something
>>    && (!rtx_equal_p)
>>        || ZERO_EXTRACT
>>        || STRICT_LOW_PART)
>> 
>> to 
>> 
>> if (something
>>    && !rtx_equal_p)
>>  {
>>     if (ZERO_EXTRACT)
>>       {
>>       }
>>     else if (STRICT_LOW_PART)
>>       {
>>       }
>>  }
>> 
>> Otherwise looks good to me, but you still need another approval.
> 
> Thanks Maxim for the review. How about the attached patch?

Looks good, with a couple of indentation nit-picks below.  No need to repost the patch on their account.  Wait for another a maintainer's review.

> --- a/gcc/cse.c
> +++ b/gcc/cse.c
> @@ -4525,14 +4525,49 @@ cse_insn (rtx_insn *insn)
>    canonicalize_insn (insn, &sets, n_sets);
>  
>    /* If this insn has a REG_EQUAL note, store the equivalent value in SRC_EQV,
> -     if different, or if the DEST is a STRICT_LOW_PART.  The latter condition
> -     is necessary because SRC_EQV is handled specially for this case, and if
> -     it isn't set, then there will be no equivalence for the destination.  */
> +     if different, or if the DEST is a STRICT_LOW_PART/ZERO_EXTRACT.  The
> +     latter condition is necessary because SRC_EQV is handled specially for
> +     this case, and if it isn't set, then there will be no equivalence
> +     for the destination.  */
>    if (n_sets == 1 && REG_NOTES (insn) != 0
> -      && (tem = find_reg_note (insn, REG_EQUAL, NULL_RTX)) != 0
> -      && (! rtx_equal_p (XEXP (tem, 0), SET_SRC (sets[0].rtl))
> -	  || GET_CODE (SET_DEST (sets[0].rtl)) == STRICT_LOW_PART))
> -    src_eqv = copy_rtx (XEXP (tem, 0));
> +      && (tem = find_reg_note (insn, REG_EQUAL, NULL_RTX)) != 0)
> +    {
> +      if ((! rtx_equal_p (XEXP (tem, 0), SET_SRC (sets[0].rtl)))
> +	  || GET_CODE (SET_DEST (sets[0].rtl)) == STRICT_LOW_PART)
> +      src_eqv = copy_rtx (XEXP (tem, 0));

Please double check indentation here.

> +
> +      /* If DEST is of the form ZERO_EXTACT, as in:
> +	 (set (zero_extract:SI (reg:SI 119)
> +		  (const_int 16 [0x10])
> +		  (const_int 16 [0x10]))
> +	      (const_int 51154 [0xc7d2]))
> +	 REG_EQUAL note will specify the value of register (reg:SI 119) at this
> +	 point.  Note that this is different from SRC_EQV. We can however
> +	 calculate SRC_EQV with the position and width of ZERO_EXTRACT.  */
> +      else if (GET_CODE (SET_DEST (sets[0].rtl)) == ZERO_EXTRACT
> +	       &&CONST_INT_P (src_eqv)

Add a space between && and CONST_INT_P.

> +	       && CONST_INT_P (XEXP (SET_DEST (sets[0].rtl), 1))
> +	       && CONST_INT_P (XEXP (SET_DEST (sets[0].rtl), 2)))
> +	{
> +	  rtx dest_reg = XEXP (SET_DEST (sets[0].rtl), 0);
> +	  rtx width = XEXP (SET_DEST (sets[0].rtl), 1);
> +	  rtx pos = XEXP (SET_DEST (sets[0].rtl), 2);
> +	  HOST_WIDE_INT val = INTVAL (src_eqv);
> +	  HOST_WIDE_INT mask;
> +	  unsigned int shift;
> +	  if (BITS_BIG_ENDIAN)
> +	    shift = GET_MODE_PRECISION (GET_MODE (dest_reg))
> +	      - INTVAL (pos) - INTVAL (width);

The usual practice is to brace the calculation that spans multiple lines, so that second and subsequent lines are aligned to the right of "=", e.g.,
shift = (GET_MODE_PRECISION (GET_MODE (dest_reg))
         - INTVAL (pos) - INTVAL (width));

> +	  else
> +	    shift = INTVAL (pos);
> +	  if (INTVAL (width) == HOST_BITS_PER_WIDE_INT)
> +	    mask = ~(HOST_WIDE_INT) 0;
> +	  else
> +	    mask = ((HOST_WIDE_INT) 1 << INTVAL (width)) - 1;
> +	  val = (val >> shift) & mask;
> +	  src_eqv = GEN_INT (val);
> +	}
> +    }
> 


--
Maxim Kuvyrkov
www.linaro.org

Re: [PATCH 1/2] Allow REG_EQUAL for ZERO_EXTRACT

[Kugan]

[-- Attachment #1: Type: text/plain, Size: 3143 bytes --]



On 30/06/15 17:16, Maxim Kuvyrkov wrote:
>> On Jun 30, 2015, at 6:54 AM, Kugan <kugan.vivekanandarajah@linaro.org> wrote:
>>
>>
>> On 29/06/15 21:56, Maxim Kuvyrkov wrote:
>>>> On Jun 28, 2015, at 2:28 PM, Kugan <kugan.vivekanandarajah@linaro.org> wrote:
>>>>
>>>> This patch allows setting REG_EQUAL for ZERO_EXTRACT and handle that in
>>>> cse (where the src for the ZERO_EXTRACT needs to be calculated)
>>>>
>>>> Thanks,
>>>> Kugan
>>>
>>>> From 75e746e559ffd21b25542b3db627e3b318118569 Mon Sep 17 00:00:00 2001
>>>> From: Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
>>>> Date: Fri, 26 Jun 2015 17:12:07 +1000
>>>> Subject: [PATCH 1/2] Allow adding REG_EQUAL for ZERO_EXTRACT
>>>>
>>>> ---
>>>> gcc/ChangeLog  |  6 ++++++
>>>> gcc/cse.c      | 41 ++++++++++++++++++++++++++++++++++++++++-
>>>> gcc/emit-rtl.c |  3 ++-
>>>> 3 files changed, 48 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
>>>> index 080aa39..d4a73d6 100644
>>>> --- a/gcc/ChangeLog
>>>> +++ b/gcc/ChangeLog
>>>> @@ -1,3 +1,9 @@
>>>> +2015-06-26  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>>> +
>>>> +	* cse.c (cse_insn): Calculate src_eqv for ZERO_EXTRACT.
>>>> +	* emit-rtl.c (set_for_reg_notes): Allow ZERO_EXTRACT to set
>>>> +	REG_EQUAL note.
>>>> +
>>>> 2015-06-25  H.J. Lu  <hongjiu.lu@intel.com>
>>>>
>>>> 	* gentarget-def.c (def_target_insn): Cast return of strtol to
>>>> diff --git a/gcc/cse.c b/gcc/cse.c
>>>> index 100c9c8..8add651 100644
>>>> --- a/gcc/cse.c
>>>> +++ b/gcc/cse.c
>>>> @@ -4531,8 +4531,47 @@ cse_insn (rtx_insn *insn)
>>>>   if (n_sets == 1 && REG_NOTES (insn) != 0
>>>>       && (tem = find_reg_note (insn, REG_EQUAL, NULL_RTX)) != 0
>>>>       && (! rtx_equal_p (XEXP (tem, 0), SET_SRC (sets[0].rtl))
>>>> +	  || GET_CODE (SET_DEST (sets[0].rtl)) == ZERO_EXTRACT
>>>> 	  || GET_CODE (SET_DEST (sets[0].rtl)) == STRICT_LOW_PART))
>>>> -    src_eqv = copy_rtx (XEXP (tem, 0));
>>>> +    {
>>>> +      src_eqv = copy_rtx (XEXP (tem, 0));
>>>> +
>>>> +      /* If DEST is of the form ZERO_EXTACT, as in:
>>>> +	 (set (zero_extract:SI (reg:SI 119)
>>>> +		  (const_int 16 [0x10])
>>>> +		  (const_int 16 [0x10]))
>>>> +	      (const_int 51154 [0xc7d2]))
>>>> +	 REG_EQUAL note will specify the value of register (reg:SI 119) at this
>>>> +	 point.  Note that this is different from SRC_EQV. We can however
>>>> +	 calculate SRC_EQV with the position and width of ZERO_EXTRACT.  */
>>>> +      if (GET_CODE (SET_DEST (sets[0].rtl)) == ZERO_EXTRACT)
>>>
>>> Consider changing
>>>
>>> if (something
>>>    && (!rtx_equal_p)
>>>        || ZERO_EXTRACT
>>>        || STRICT_LOW_PART)
>>>
>>> to 
>>>
>>> if (something
>>>    && !rtx_equal_p)
>>>  {
>>>     if (ZERO_EXTRACT)
>>>       {
>>>       }
>>>     else if (STRICT_LOW_PART)
>>>       {
>>>       }
>>>  }
>>>
>>> Otherwise looks good to me, but you still need another approval.
>>
>> Thanks Maxim for the review. How about the attached patch?
> 
> Looks good, with a couple of indentation nit-picks below.  No need to repost the patch on their account.  Wait for another a maintainer's review.

Thanks. Here is the update patch.

Kugan

[-- Attachment #2: 0001-Allow-adding-REG_EQUAL-for-ZERO_EXTRACT.patch --]
[-- Type: text/x-diff, Size: 3956 bytes --]

From 0b86c1ed630ef70e17412808d6baca93259de2ee Mon Sep 17 00:00:00 2001
From: Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
Date: Fri, 26 Jun 2015 17:12:07 +1000
Subject: [PATCH 1/3] Allow adding REG_EQUAL for ZERO_EXTRACT

---
 gcc/ChangeLog  |  7 +++++++
 gcc/cse.c      | 49 ++++++++++++++++++++++++++++++++++++++++++-------
 gcc/emit-rtl.c |  3 ++-
 3 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index fc23abd..5796f16 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,10 @@
+
+2015-06-26  Kugan Vivekanandarajah  <kuganv@linaro.org>
+
+	* cse.c (cse_insn): Calculate src_eqv for ZERO_EXTRACT.
+	* emit-rtl.c (set_for_reg_notes): Allow ZERO_EXTRACT to set
+	REG_EQUAL note.
+
 2015-06-30  Eric Botcazou  <ebotcazou@adacore.com>
 
 	* lto-streamer-out.c (class DFS): Adjust hash_scc method.
diff --git a/gcc/cse.c b/gcc/cse.c
index e01240c..09dc7c2 100644
--- a/gcc/cse.c
+++ b/gcc/cse.c
@@ -4525,14 +4525,49 @@ cse_insn (rtx_insn *insn)
   canonicalize_insn (insn, &sets, n_sets);
 
   /* If this insn has a REG_EQUAL note, store the equivalent value in SRC_EQV,
-     if different, or if the DEST is a STRICT_LOW_PART.  The latter condition
-     is necessary because SRC_EQV is handled specially for this case, and if
-     it isn't set, then there will be no equivalence for the destination.  */
+     if different, or if the DEST is a STRICT_LOW_PART/ZERO_EXTRACT.  The
+     latter condition is necessary because SRC_EQV is handled specially for
+     this case, and if it isn't set, then there will be no equivalence
+     for the destination.  */
   if (n_sets == 1 && REG_NOTES (insn) != 0
-      && (tem = find_reg_note (insn, REG_EQUAL, NULL_RTX)) != 0
-      && (! rtx_equal_p (XEXP (tem, 0), SET_SRC (sets[0].rtl))
-	  || GET_CODE (SET_DEST (sets[0].rtl)) == STRICT_LOW_PART))
-    src_eqv = copy_rtx (XEXP (tem, 0));
+      && (tem = find_reg_note (insn, REG_EQUAL, NULL_RTX)) != 0)
+    {
+      if ((! rtx_equal_p (XEXP (tem, 0), SET_SRC (sets[0].rtl)))
+	  || GET_CODE (SET_DEST (sets[0].rtl)) == STRICT_LOW_PART)
+	src_eqv = copy_rtx (XEXP (tem, 0));
+
+      /* If DEST is of the form ZERO_EXTACT, as in:
+	 (set (zero_extract:SI (reg:SI 119)
+		  (const_int 16 [0x10])
+		  (const_int 16 [0x10]))
+	      (const_int 51154 [0xc7d2]))
+	 REG_EQUAL note will specify the value of register (reg:SI 119) at this
+	 point.  Note that this is different from SRC_EQV. We can however
+	 calculate SRC_EQV with the position and width of ZERO_EXTRACT.  */
+      else if (GET_CODE (SET_DEST (sets[0].rtl)) == ZERO_EXTRACT
+	       && CONST_INT_P (src_eqv)
+	       && CONST_INT_P (XEXP (SET_DEST (sets[0].rtl), 1))
+	       && CONST_INT_P (XEXP (SET_DEST (sets[0].rtl), 2)))
+	{
+	  rtx dest_reg = XEXP (SET_DEST (sets[0].rtl), 0);
+	  rtx width = XEXP (SET_DEST (sets[0].rtl), 1);
+	  rtx pos = XEXP (SET_DEST (sets[0].rtl), 2);
+	  HOST_WIDE_INT val = INTVAL (src_eqv);
+	  HOST_WIDE_INT mask;
+	  unsigned int shift;
+	  if (BITS_BIG_ENDIAN)
+	    shift = GET_MODE_PRECISION (GET_MODE (dest_reg))
+	      - INTVAL (pos) - INTVAL (width);
+	  else
+	    shift = INTVAL (pos);
+	  if (INTVAL (width) == HOST_BITS_PER_WIDE_INT)
+	    mask = ~(HOST_WIDE_INT) 0;
+	  else
+	    mask = ((HOST_WIDE_INT) 1 << INTVAL (width)) - 1;
+	  val = (val >> shift) & mask;
+	  src_eqv = GEN_INT (val);
+	}
+    }
 
   /* Set sets[i].src_elt to the class each source belongs to.
      Detect assignments from or to volatile things
diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index 80c0adb..cdb70ac 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -5228,7 +5228,8 @@ set_for_reg_notes (rtx insn)
   reg = SET_DEST (pat);
 
   /* Notes apply to the contents of a STRICT_LOW_PART.  */
-  if (GET_CODE (reg) == STRICT_LOW_PART)
+  if (GET_CODE (reg) == STRICT_LOW_PART
+      || GET_CODE (reg) == ZERO_EXTRACT)
     reg = XEXP (reg, 0);
 
   /* Check that we have a register.  */
-- 
1.9.1

Re: [PATCH 1/2] Allow REG_EQUAL for ZERO_EXTRACT

[Jeff Law]

On 07/05/2015 05:16 PM, Kugan wrote:
>
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index fc23abd..5796f16 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,10 @@
> +
> +2015-06-26  Kugan Vivekanandarajah<kuganv@linaro.org>
> +
> +	* cse.c (cse_insn): Calculate src_eqv for ZERO_EXTRACT.
> +	* emit-rtl.c (set_for_reg_notes): Allow ZERO_EXTRACT to set
> +	REG_EQUAL note.
OK.
jeff

Re: [PATCH 1/2] Allow REG_EQUAL for ZERO_EXTRACT

[Kugan]

[-- Attachment #1: Type: text/plain, Size: 940 bytes --]

I have made a mistake while addressing the review comments for this
patch. Unfortunately, It was not detected in my earlier testing. My
sincere graphology for the mistake.

I have basically missed the STRICT_LOW_PART check for the first if-check
thus the second part (which is the ZERO_EXTRACT part) will never get
executed. Attached patch fixes this along with some minor changes.

Bootstrapped and regression tested on arm-none-linux (Chromebook) and
x86-64-linux-gnu with no new regression along with the ARM ennoblement
patch.

Also did a complete arm qemu regression testing with Chriophe's scripts
with no new regression.
(http://people.linaro.org/~christophe.lyon/cross-validation/gcc-test-patches/225987-reg4/report-build-info.html)

Is this OK for trunk,


Thanks,
Kugan

gcc/ChangeLog:

2015-07-20  Kugan Vivekanandarajah  <kuganv@linaro.org>

	* cse.c (cse_insn): Fix missing check for STRICT_LOW_PART and minor
	clean up.

[-- Attachment #2: p.txt --]
[-- Type: text/plain, Size: 1548 bytes --]

diff --git a/gcc/cse.c b/gcc/cse.c
index 1c14d83..96adf18 100644
--- a/gcc/cse.c
+++ b/gcc/cse.c
@@ -4529,10 +4529,10 @@ cse_insn (rtx_insn *insn)
      this case, and if it isn't set, then there will be no equivalence
      for the destination.  */
   if (n_sets == 1 && REG_NOTES (insn) != 0
-      && (tem = find_reg_note (insn, REG_EQUAL, NULL_RTX)) != 0)
+      && (tem = find_reg_note (insn, REG_EQUAL, NULL_RTX)) != 0
+      && (! rtx_equal_p (XEXP (tem, 0), SET_SRC (sets[0].rtl))))
     {
-      if ((! rtx_equal_p (XEXP (tem, 0), SET_SRC (sets[0].rtl)))
-	  || GET_CODE (SET_DEST (sets[0].rtl)) == STRICT_LOW_PART)
+      if (GET_CODE (SET_DEST (sets[0].rtl)) == STRICT_LOW_PART)
 	src_eqv = copy_rtx (XEXP (tem, 0));
 
       /* If DEST is of the form ZERO_EXTACT, as in:
@@ -4544,14 +4544,14 @@ cse_insn (rtx_insn *insn)
 	 point.  Note that this is different from SRC_EQV. We can however
 	 calculate SRC_EQV with the position and width of ZERO_EXTRACT.  */
       else if (GET_CODE (SET_DEST (sets[0].rtl)) == ZERO_EXTRACT
-	       && CONST_INT_P (src_eqv)
+	       && CONST_INT_P (XEXP (tem, 0))
 	       && CONST_INT_P (XEXP (SET_DEST (sets[0].rtl), 1))
 	       && CONST_INT_P (XEXP (SET_DEST (sets[0].rtl), 2)))
 	{
 	  rtx dest_reg = XEXP (SET_DEST (sets[0].rtl), 0);
 	  rtx width = XEXP (SET_DEST (sets[0].rtl), 1);
 	  rtx pos = XEXP (SET_DEST (sets[0].rtl), 2);
-	  HOST_WIDE_INT val = INTVAL (src_eqv);
+	  HOST_WIDE_INT val = INTVAL (XEXP (tem, 0));
 	  HOST_WIDE_INT mask;
 	  unsigned int shift;
 	  if (BITS_BIG_ENDIAN)

Re: [PATCH 1/2] Allow REG_EQUAL for ZERO_EXTRACT

[Jeff Law]

On 07/19/2015 09:17 PM, Kugan wrote:
> I have made a mistake while addressing the review comments for this
> patch. Unfortunately, It was not detected in my earlier testing. My
> sincere graphology for the mistake.
>
> I have basically missed the STRICT_LOW_PART check for the first if-check
> thus the second part (which is the ZERO_EXTRACT part) will never get
> executed. Attached patch fixes this along with some minor changes.
>
> Bootstrapped and regression tested on arm-none-linux (Chromebook) and
> x86-64-linux-gnu with no new regression along with the ARM ennoblement
> patch.
>
> Also did a complete arm qemu regression testing with Chriophe's scripts
> with no new regression.
> (http://people.linaro.org/~christophe.lyon/cross-validation/gcc-test-patches/225987-reg4/report-build-info.html)
>
> Is this OK for trunk,
>
>
> Thanks,
> Kugan
>
> gcc/ChangeLog:
>
> 2015-07-20  Kugan Vivekanandarajah  <kuganv@linaro.org>
>
> 	* cse.c (cse_insn): Fix missing check for STRICT_LOW_PART and minor
> 	clean up.
OK.
jeff

Re: [PATCH 1/2] Allow REG_EQUAL for ZERO_EXTRACT

[Andreas Schwab]

Kugan <kugan.vivekanandarajah@linaro.org> writes:

> 	* cse.c (cse_insn): Fix missing check for STRICT_LOW_PART and minor
> 	clean up.

This breaks 

gcc.target/m68k/tls-ie-xgot.c scan-assembler jsr __m68k_read_tp
gcc.target/m68k/tls-ie.c scan-assembler jsr __m68k_read_tp
gcc.target/m68k/tls-le-xtls.c scan-assembler jsr __m68k_read_tp
gcc.target/m68k/tls-le.c scan-assembler jsr __m68k_read_tp

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH 1/2] Allow REG_EQUAL for ZERO_EXTRACT

[Kugan]

On 27/07/15 05:38, Andreas Schwab wrote:
> Kugan <kugan.vivekanandarajah@linaro.org> writes:
> 
>> 	* cse.c (cse_insn): Fix missing check for STRICT_LOW_PART and minor
>> 	clean up.
> 
> This breaks 
> 
> gcc.target/m68k/tls-ie-xgot.c scan-assembler jsr __m68k_read_tp
> gcc.target/m68k/tls-ie.c scan-assembler jsr __m68k_read_tp
> gcc.target/m68k/tls-le-xtls.c scan-assembler jsr __m68k_read_tp
> gcc.target/m68k/tls-le.c scan-assembler jsr __m68k_read_tp

I am Looking into it now.

Thanks,
Kugan

Re: [PATCH 1/2] Allow REG_EQUAL for ZERO_EXTRACT

[Kugan]

[-- Attachment #1: Type: text/plain, Size: 1256 bytes --]



On 27/07/15 05:38, Andreas Schwab wrote:
> Kugan <kugan.vivekanandarajah@linaro.org> writes:
> 
>> 	* cse.c (cse_insn): Fix missing check for STRICT_LOW_PART and minor
>> 	clean up.
> 
> This breaks 
> 
> gcc.target/m68k/tls-ie-xgot.c scan-assembler jsr __m68k_read_tp
> gcc.target/m68k/tls-ie.c scan-assembler jsr __m68k_read_tp
> gcc.target/m68k/tls-le-xtls.c scan-assembler jsr __m68k_read_tp
> gcc.target/m68k/tls-le.c scan-assembler jsr __m68k_read_tp
> 
> Andreas.
> 

Sorry for the breakage. My patch to add ZERO_EXTRACT unfortunately
restricts the behaviour in one other case. That is, even when REG_EQUAL
note and src are same, we were setting src_eqv to src when it is
STRICT_LOW_PART. Not sure why but restored the old behaviour.

I could reproduce this issue by inspecting the generated asm and made
sure that it is fixed. However I could not run regression for m68k
(Sorry I donÂ’t have access to the set-up).
I bootstrapped and regression tested on x86_64-linux-gnu and
arm-none-linux-gnu with no new regressions.

Thanks,
Kugan


gcc/ChangeLog:

2015-07-27  Kugan Vivekanandarajah  <kuganv@linaro.org>

	* cse.c (cse_insn): Restoring old behaviour for src_eqv
	 when dest and value in the REG_EQUAL are same and dest
	 is STRICT_LOW_PART.

[-- Attachment #2: p.txt --]
[-- Type: text/plain, Size: 947 bytes --]

diff --git a/gcc/cse.c b/gcc/cse.c
index 96adf18..17c0954 100644
--- a/gcc/cse.c
+++ b/gcc/cse.c
@@ -4529,12 +4529,13 @@ cse_insn (rtx_insn *insn)
      this case, and if it isn't set, then there will be no equivalence
      for the destination.  */
   if (n_sets == 1 && REG_NOTES (insn) != 0
-      && (tem = find_reg_note (insn, REG_EQUAL, NULL_RTX)) != 0
-      && (! rtx_equal_p (XEXP (tem, 0), SET_SRC (sets[0].rtl))))
+      && (tem = find_reg_note (insn, REG_EQUAL, NULL_RTX)) != 0)
     {
-      if (GET_CODE (SET_DEST (sets[0].rtl)) == STRICT_LOW_PART)
-	src_eqv = copy_rtx (XEXP (tem, 0));
 
+      if (GET_CODE (SET_DEST (sets[0].rtl)) != ZERO_EXTRACT
+	  && (! rtx_equal_p (XEXP (tem, 0), SET_SRC (sets[0].rtl))
+	      || GET_CODE (SET_DEST (sets[0].rtl)) == STRICT_LOW_PART))
+	src_eqv = copy_rtx (XEXP (tem, 0));
       /* If DEST is of the form ZERO_EXTACT, as in:
 	 (set (zero_extract:SI (reg:SI 119)
 		  (const_int 16 [0x10])

Re: [PATCH 1/2] Allow REG_EQUAL for ZERO_EXTRACT

[Andreas Schwab]

Kugan <kugan.vivekanandarajah@linaro.org> writes:

> 	* cse.c (cse_insn): Restoring old behaviour for src_eqv
> 	 when dest and value in the REG_EQUAL are same and dest
> 	 is STRICT_LOW_PART.

This fixes the regression and doesn't introduce any new one.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH 1/2] Allow REG_EQUAL for ZERO_EXTRACT

[Jeff Law]

On 07/28/2015 06:00 AM, Kugan wrote:
>
>
> On 27/07/15 05:38, Andreas Schwab wrote:
>> Kugan <kugan.vivekanandarajah@linaro.org> writes:
>>
>>> 	* cse.c (cse_insn): Fix missing check for STRICT_LOW_PART and minor
>>> 	clean up.
>>
>> This breaks
>>
>> gcc.target/m68k/tls-ie-xgot.c scan-assembler jsr __m68k_read_tp
>> gcc.target/m68k/tls-ie.c scan-assembler jsr __m68k_read_tp
>> gcc.target/m68k/tls-le-xtls.c scan-assembler jsr __m68k_read_tp
>> gcc.target/m68k/tls-le.c scan-assembler jsr __m68k_read_tp
>>
>> Andreas.
>>
>
> Sorry for the breakage. My patch to add ZERO_EXTRACT unfortunately
> restricts the behaviour in one other case. That is, even when REG_EQUAL
> note and src are same, we were setting src_eqv to src when it is
> STRICT_LOW_PART. Not sure why but restored the old behaviour.
>
> I could reproduce this issue by inspecting the generated asm and made
> sure that it is fixed. However I could not run regression for m68k
> (Sorry I don’t have access to the set-up).
> I bootstrapped and regression tested on x86_64-linux-gnu and
> arm-none-linux-gnu with no new regressions.
>
> Thanks,
> Kugan
>
>
> gcc/ChangeLog:
>
> 2015-07-27  Kugan Vivekanandarajah  <kuganv@linaro.org>
>
> 	* cse.c (cse_insn): Restoring old behaviour for src_eqv
> 	 when dest and value in the REG_EQUAL are same and dest
> 	 is STRICT_LOW_PART.
OK.

I verified there were no regressions on m68k.exp with a cross compiler 
and that the tests referenced by Andreas indeed passes with the patch 
installed.

Thanks,
jeff

[PATCH 2/2] Set REG_EQUAL

[Kugan]

[-- Attachment #1: Type: text/plain, Size: 344 bytes --]

This patch sets REG_EQUAL when emitting arm_emit_movpair.

Thanks,
Kugan

gcc/testsuite/ChangeLog:

2015-06-26  Kugan Vivekanandarajah  <kuganv@linaro.org>

	* gcc.target/arm/reg_equal_test.c: New test.

gcc.

2015-06-26  Kugan Vivekanandarajah  <kuganv@linaro.org>

	* config/arm/arm.c (arm_emit_movpair): Add REG_EQUAL notes to
	instruction.

[-- Attachment #2: 0002-Add-REG_EQUAL-note-for-arm_emit_movpair.patch --]
[-- Type: text/x-diff, Size: 2852 bytes --]

From e90feaca4d7dfc893cb2a0142e1888655c9ffa1f Mon Sep 17 00:00:00 2001
From: Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
Date: Fri, 26 Jun 2015 17:22:22 +1000
Subject: [PATCH 2/2] Add REG_EQUAL note for arm_emit_movpair

---
 gcc/config/arm/arm.c                          | 14 +++++++++++---
 gcc/testsuite/ChangeLog                       |  4 ++++
 gcc/testsuite/gcc.target/arm/reg_equal_test.c | 24 ++++++++++++++++++++++++
 3 files changed, 39 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/arm/reg_equal_test.c

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 83f3269..8a47c72 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -17884,19 +17884,27 @@ output_mov_long_double_arm_from_arm (rtx *operands)
 void
 arm_emit_movpair (rtx dest, rtx src)
  {
+  rtx insn;
+
   /* If the src is an immediate, simplify it.  */
   if (CONST_INT_P (src))
     {
       HOST_WIDE_INT val = INTVAL (src);
       emit_set_insn (dest, GEN_INT (val & 0x0000ffff));
       if ((val >> 16) & 0x0000ffff)
-        emit_set_insn (gen_rtx_ZERO_EXTRACT (SImode, dest, GEN_INT (16),
-                                             GEN_INT (16)),
-                       GEN_INT ((val >> 16) & 0x0000ffff));
+	{
+	  emit_set_insn (gen_rtx_ZERO_EXTRACT (SImode, dest, GEN_INT (16),
+					       GEN_INT (16)),
+			 GEN_INT ((val >> 16) & 0x0000ffff));
+	  insn = get_last_insn ();
+	  set_unique_reg_note (insn, REG_EQUAL, copy_rtx (src));
+	}
       return;
     }
    emit_set_insn (dest, gen_rtx_HIGH (SImode, src));
    emit_set_insn (dest, gen_rtx_LO_SUM (SImode, dest, src));
+   insn = get_last_insn ();
+   set_unique_reg_note (insn, REG_EQUAL, copy_rtx (src));
  }
 
 /* Output a move between double words.  It must be REG<-MEM
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index b6486ac..8edb484 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2015-06-26  Kugan Vivekanandarajah  <kuganv@linaro.org>
+
+	* gcc.target/arm/reg_equal_test.c: New test.
+
 2015-06-25  Richard Biener  <rguenther@suse.de>
 
 	* gcc.dg/tree-ssa/pr52631.c: Disable forwprop.
diff --git a/gcc/testsuite/gcc.target/arm/reg_equal_test.c b/gcc/testsuite/gcc.target/arm/reg_equal_test.c
new file mode 100644
index 0000000..58fa9dd
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/reg_equal_test.c
@@ -0,0 +1,24 @@
+/* { dg-do compile } */
+/* { dg-options "-O1 -fdump-rtl-expand" } */
+
+extern void abort (void);
+unsigned int a = 1;
+
+int
+main (void)
+{
+  unsigned int b, c, d;
+
+  if (sizeof (int) != 4 || (int) 0xc7d24b5e > 0)
+    return 0;
+
+  c = 0xc7d24b5e;
+  d = a | -2;
+  b = (d == 0) ? c : (c % d);
+  if (b != c)
+    abort ();
+
+  return 0;
+}
+
+/* { dg-final { scan-rtl-dump "expr_list:REG_EQUAL \\(const_int -942519458" "expand" } } */
-- 
1.9.1

Re: [PATCH 2/2] Set REG_EQUAL

[Maxim Kuvyrkov]

> On Jun 28, 2015, at 2:30 PM, Kugan <kugan.vivekanandarajah@linaro.org> wrote:
> 
> This patch sets REG_EQUAL when emitting arm_emit_movpair.
> 
> Thanks,
> Kugan
> 
> gcc/testsuite/ChangeLog:
> 
> 2015-06-26  Kugan Vivekanandarajah  <kuganv@linaro.org>
> 
> 	* gcc.target/arm/reg_equal_test.c: New test.
> 
> gcc.
> 
> 2015-06-26  Kugan Vivekanandarajah  <kuganv@linaro.org>
> 
> 	* config/arm/arm.c (arm_emit_movpair): Add REG_EQUAL notes to
> 	instruction.
> <0002-Add-REG_EQUAL-note-for-arm_emit_movpair.patch>

LGTM, but you need ARM maintainer's approval.

--
Maxim Kuvyrkov
www.linaro.org

Re: [PATCH 2/2] Set REG_EQUAL

[Kugan]

Ping?


On 28/06/15 21:30, Kugan wrote:
> This patch sets REG_EQUAL when emitting arm_emit_movpair.
> 
> Thanks,
> Kugan
> 
> gcc/testsuite/ChangeLog:
> 
> 2015-06-26  Kugan Vivekanandarajah  <kuganv@linaro.org>
> 
> 	* gcc.target/arm/reg_equal_test.c: New test.
> 
> gcc.
> 
> 2015-06-26  Kugan Vivekanandarajah  <kuganv@linaro.org>
> 
> 	* config/arm/arm.c (arm_emit_movpair): Add REG_EQUAL notes to
> 	instruction.
> 

Re: [PATCH 2/2] Set REG_EQUAL

[Kyrill Tkachov]

Hi Kugan,

On 28/06/15 12:30, Kugan wrote:
> This patch sets REG_EQUAL when emitting arm_emit_movpair.
>
> Thanks,
> Kugan
>
> gcc/testsuite/ChangeLog:
>
> 2015-06-26  Kugan Vivekanandarajah  <kuganv@linaro.org>
>
> 	* gcc.target/arm/reg_equal_test.c: New test.
>
> gcc.
>
> 2015-06-26  Kugan Vivekanandarajah  <kuganv@linaro.org>
>
> 	* config/arm/arm.c (arm_emit_movpair): Add REG_EQUAL notes to
> 	instruction.

This is ok for trunk.
Sorry for the delay.
Can you please re-test this on arm and commit if all is clean?

Thanks,
Kyrill