[Patch, MIPS] Patch to fix MIPS optimization bug in combine

[Patch, MIPS] Patch to fix MIPS optimization bug in combine

[Steve Ellcey]

A bug was reported against the GCC MIPS64 compiler that involves a bad combine
and this patch fixes the bug.

When using '-fexpensive-optimizations -march=mips64r2 -mabi=64' GCC is
combining these instructions:

(insn 13 12 14 2 (set (reg:DI 206 [ *last_3(D)+-4 ])
        (zero_extend:DI (subreg/s/u:SI (reg:DI 196 [ *last_3(D)+-4 ]) 4))) x.c:21 212 {*zero_extendsidi2_dext}
     (nil))

(insn 15 14 16 2 (set (reg:DI 208)
        (and:DI (reg:DI 207)
            (const_int 4294967295 [0xffffffff]))) x.c:21 182 {*anddi3}
     (expr_list:REG_DEAD (reg:DI 207)
        (nil)))

(jump_insn 16 15 17 2 (set (pc)
        (if_then_else (ne (reg:DI 206 [ *last_3(D)+-4 ])
                (reg:DI 208))
            (label_ref:DI 35)
            (pc))) x.c:21 473 {*branch_equalitydi}
     (expr_list:REG_DEAD (reg:DI 208)
        (expr_list:REG_DEAD (reg:DI 206 [ *last_3(D)+-4 ])
            (int_list:REG_BR_PROB 8010 (nil))))
 -> 35)

Into:

(jump_insn 16 15 17 2 (set (pc)
        (if_then_else (ne (subreg:SI (reg:DI 207) 4)
                (subreg:SI (reg:DI 196 [ *last_3(D)+-4 ]) 4))
            (label_ref:DI 35)
            (pc))) x.c:21 472 {*branch_equalitysi}
     (expr_list:REG_DEAD (reg:DI 207)
        (int_list:REG_BR_PROB 8010 (nil)))
 -> 35)


The problem is that the jump_insn on MIPS64 generates a beq/bne instruction
that compares the entire 64 bit registers and there is no option to only
look at the bottom 32 bits.  When we got rid of insn 15 we lost the AND that
cleared the upper 32 bits of one of the registers and the program fails.

My solution was to not allow subregs in the conditional jump instruction.
Here is the patch and a test case and I ran the GCC testsuite with no
regressions.

OK to checkin?

Steve Ellcey
sellcey@imgtec.com


2015-10-21  Steve Ellcey  <sellcey@imgtec.com>

	* mips.c (mips_legitimate_combined_insn): New function.
	(TARGET_LEGITIMATE_COMBINED_INSN): New macro.


diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index 0b4a5fa..4338628 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -19606,6 +19606,26 @@ mips_ira_change_pseudo_allocno_class (int regno, reg_class_t allocno_class)
     return GR_REGS;
   return allocno_class;
 }
+
+/* Implement TARGET_LEGITIMATE_COMBINED_INSN */
+
+static bool
+mips_legitimate_combined_insn (rtx_insn* insn)
+{
+  /* If we do a conditional jump involving register compares do not allow
+     subregs because beq/bne will always compare the entire register.
+     This should only be an issue with N32/N64 ABI's that do a 32 bit
+     compare and jump.  */
+
+  if (any_condjump_p (insn))
+    {
+      rtx cond = XEXP (SET_SRC (pc_set (insn)), 0);
+      if (GET_RTX_CLASS (GET_CODE (cond)) == RTX_COMPARE
+	  || GET_RTX_CLASS (GET_CODE (cond)) == RTX_COMM_COMPARE)
+	return !SUBREG_P (XEXP (cond, 0)) && !SUBREG_P (XEXP (cond, 1));
+    }
+  return true;
+}
 \f
 /* Initialize the GCC target structure.  */
 #undef TARGET_ASM_ALIGNED_HI_OP
@@ -19868,6 +19888,9 @@ mips_ira_change_pseudo_allocno_class (int regno, reg_class_t allocno_class)
 #undef TARGET_HARD_REGNO_SCRATCH_OK
 #define TARGET_HARD_REGNO_SCRATCH_OK mips_hard_regno_scratch_ok
 
+#undef TARGET_LEGITIMATE_COMBINED_INSN
+#define TARGET_LEGITIMATE_COMBINED_INSN mips_legitimate_combined_insn
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 \f
 #include "gt-mips.h"





2015-10-21  Steve Ellcey  <sellcey@imgtec.com>

	* gcc.dg/combine-subregs.c: New test.


diff --git a/gcc/testsuite/gcc.dg/combine-subregs.c b/gcc/testsuite/gcc.dg/combine-subregs.c
index e69de29..c480f88 100644
--- a/gcc/testsuite/gcc.dg/combine-subregs.c
+++ b/gcc/testsuite/gcc.dg/combine-subregs.c
@@ -0,0 +1,36 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -fexpensive-optimizations" } */
+
+#include <inttypes.h>
+#include <stdlib.h>
+
+void __attribute__ ((noinline))
+foo (uint64_t state, uint32_t last)
+{
+  if (state == last) abort ();
+}
+
+/* This function may do a bad comparision by trying to
+   use SUBREGS during the compare on machines where comparing
+   two registers always compares the entire register regardless
+   of mode.  */
+
+int __attribute__ ((noinline))
+compare (uint64_t state, uint32_t *last, uint8_t buf)
+{
+    if (*last == ((state | buf) & 0xFFFFFFFF)) {
+	foo (state, *last);
+        return 0;
+    }
+    return 1;
+}
+
+int
+main(int argc, char **argv) {
+    uint64_t state = 0xF00000100U;
+    uint32_t last  = 0x101U;
+    int ret        = compare(state, &last, 0x01);
+    if (ret != 0)
+	abort ();
+    exit (0);
+}

Re: [Patch, MIPS] Patch to fix MIPS optimization bug in combine

[pinskia]

> On Oct 22, 2015, at 12:44 AM, Steve Ellcey <sellcey@imgtec.com> wrote:
> 
> 
> A bug was reported against the GCC MIPS64 compiler that involves a bad combine
> and this patch fixes the bug.
> 
> When using '-fexpensive-optimizations -march=mips64r2 -mabi=64' GCC is
> combining these instructions:
> 
> (insn 13 12 14 2 (set (reg:DI 206 [ *last_3(D)+-4 ])
>        (zero_extend:DI (subreg/s/u:SI (reg:DI 196 [ *last_3(D)+-4 ]) 4))) x.c:21 212 {*zero_extendsidi2_dext}
>     (nil))
> 
> (insn 15 14 16 2 (set (reg:DI 208)
>        (and:DI (reg:DI 207)
>            (const_int 4294967295 [0xffffffff]))) x.c:21 182 {*anddi3}
>     (expr_list:REG_DEAD (reg:DI 207)
>        (nil)))
> 
> (jump_insn 16 15 17 2 (set (pc)
>        (if_then_else (ne (reg:DI 206 [ *last_3(D)+-4 ])
>                (reg:DI 208))
>            (label_ref:DI 35)
>            (pc))) x.c:21 473 {*branch_equalitydi}
>     (expr_list:REG_DEAD (reg:DI 208)
>        (expr_list:REG_DEAD (reg:DI 206 [ *last_3(D)+-4 ])
>            (int_list:REG_BR_PROB 8010 (nil))))
> -> 35)
> 
> Into:
> 
> (jump_insn 16 15 17 2 (set (pc)
>        (if_then_else (ne (subreg:SI (reg:DI 207) 4)
>                (subreg:SI (reg:DI 196 [ *last_3(D)+-4 ]) 4))
>            (label_ref:DI 35)
>            (pc))) x.c:21 472 {*branch_equalitysi}
>     (expr_list:REG_DEAD (reg:DI 207)
>        (int_list:REG_BR_PROB 8010 (nil)))
> -> 35)
> 
> 
> The problem is that the jump_insn on MIPS64 generates a beq/bne instruction
> that compares the entire 64 bit registers and there is no option to only
> look at the bottom 32 bits.  When we got rid of insn 15 we lost the AND that
> cleared the upper 32 bits of one of the registers and the program fails.
> 
> My solution was to not allow subregs in the conditional jump instruction.
> Here is the patch and a test case and I ran the GCC testsuite with no
> regressions.
> 
> OK to checkin?

No this is the wrong approach. The problem is in combine. I had submitted a patch to fix a while back but I never got around to the comments to make it consistent with the rest of combine. 

Let me dig up my patch in a few minutes. 

Thanks,
Andrew

> 
> Steve Ellcey
> sellcey@imgtec.com
> 
> 
> 2015-10-21  Steve Ellcey  <sellcey@imgtec.com>
> 
>    * mips.c (mips_legitimate_combined_insn): New function.
>    (TARGET_LEGITIMATE_COMBINED_INSN): New macro.
> 
> 
> diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
> index 0b4a5fa..4338628 100644
> --- a/gcc/config/mips/mips.c
> +++ b/gcc/config/mips/mips.c
> @@ -19606,6 +19606,26 @@ mips_ira_change_pseudo_allocno_class (int regno, reg_class_t allocno_class)
>     return GR_REGS;
>   return allocno_class;
> }
> +
> +/* Implement TARGET_LEGITIMATE_COMBINED_INSN */
> +
> +static bool
> +mips_legitimate_combined_insn (rtx_insn* insn)
> +{
> +  /* If we do a conditional jump involving register compares do not allow
> +     subregs because beq/bne will always compare the entire register.
> +     This should only be an issue with N32/N64 ABI's that do a 32 bit
> +     compare and jump.  */
> +
> +  if (any_condjump_p (insn))
> +    {
> +      rtx cond = XEXP (SET_SRC (pc_set (insn)), 0);
> +      if (GET_RTX_CLASS (GET_CODE (cond)) == RTX_COMPARE
> +      || GET_RTX_CLASS (GET_CODE (cond)) == RTX_COMM_COMPARE)
> +    return !SUBREG_P (XEXP (cond, 0)) && !SUBREG_P (XEXP (cond, 1));
> +    }
> +  return true;
> +}
> \f
> /* Initialize the GCC target structure.  */
> #undef TARGET_ASM_ALIGNED_HI_OP
> @@ -19868,6 +19888,9 @@ mips_ira_change_pseudo_allocno_class (int regno, reg_class_t allocno_class)
> #undef TARGET_HARD_REGNO_SCRATCH_OK
> #define TARGET_HARD_REGNO_SCRATCH_OK mips_hard_regno_scratch_ok
> 
> +#undef TARGET_LEGITIMATE_COMBINED_INSN
> +#define TARGET_LEGITIMATE_COMBINED_INSN mips_legitimate_combined_insn
> +
> struct gcc_target targetm = TARGET_INITIALIZER;
> \f
> #include "gt-mips.h"
> 
> 
> 
> 
> 
> 2015-10-21  Steve Ellcey  <sellcey@imgtec.com>
> 
>    * gcc.dg/combine-subregs.c: New test.
> 
> 
> diff --git a/gcc/testsuite/gcc.dg/combine-subregs.c b/gcc/testsuite/gcc.dg/combine-subregs.c
> index e69de29..c480f88 100644
> --- a/gcc/testsuite/gcc.dg/combine-subregs.c
> +++ b/gcc/testsuite/gcc.dg/combine-subregs.c
> @@ -0,0 +1,36 @@
> +/* { dg-do run } */
> +/* { dg-options "-O2 -fexpensive-optimizations" } */
> +
> +#include <inttypes.h>
> +#include <stdlib.h>
> +
> +void __attribute__ ((noinline))
> +foo (uint64_t state, uint32_t last)
> +{
> +  if (state == last) abort ();
> +}
> +
> +/* This function may do a bad comparision by trying to
> +   use SUBREGS during the compare on machines where comparing
> +   two registers always compares the entire register regardless
> +   of mode.  */
> +
> +int __attribute__ ((noinline))
> +compare (uint64_t state, uint32_t *last, uint8_t buf)
> +{
> +    if (*last == ((state | buf) & 0xFFFFFFFF)) {
> +    foo (state, *last);
> +        return 0;
> +    }
> +    return 1;
> +}
> +
> +int
> +main(int argc, char **argv) {
> +    uint64_t state = 0xF00000100U;
> +    uint32_t last  = 0x101U;
> +    int ret        = compare(state, &last, 0x01);
> +    if (ret != 0)
> +    abort ();
> +    exit (0);
> +}

Re: [Patch, MIPS] Patch to fix MIPS optimization bug in combine

[Segher Boessenkool]

Hi,

On Wed, Oct 21, 2015 at 09:44:25AM -0700, Steve Ellcey  wrote:
> (jump_insn 16 15 17 2 (set (pc)
>         (if_then_else (ne (subreg:SI (reg:DI 207) 4)
>                 (subreg:SI (reg:DI 196 [ *last_3(D)+-4 ]) 4))
>             (label_ref:DI 35)
>             (pc))) x.c:21 472 {*branch_equalitysi}
>      (expr_list:REG_DEAD (reg:DI 207)
>         (int_list:REG_BR_PROB 8010 (nil)))
>  -> 35)

Why does *branch_equalitysi allow these subregs if it cannot handle them?
Not just combine can generate code like this (in theory at least).


Segher

Re: [Patch, MIPS] Patch to fix MIPS optimization bug in combine

[Andrew Pinski]

On Thu, Oct 22, 2015 at 3:32 AM,  <pinskia@gmail.com> wrote:
>
>
>> On Oct 22, 2015, at 12:44 AM, Steve Ellcey <sellcey@imgtec.com> wrote:
>>
>>
>> A bug was reported against the GCC MIPS64 compiler that involves a bad combine
>> and this patch fixes the bug.
>>
>> When using '-fexpensive-optimizations -march=mips64r2 -mabi=64' GCC is
>> combining these instructions:
>>
>> (insn 13 12 14 2 (set (reg:DI 206 [ *last_3(D)+-4 ])
>>        (zero_extend:DI (subreg/s/u:SI (reg:DI 196 [ *last_3(D)+-4 ]) 4))) x.c:21 212 {*zero_extendsidi2_dext}
>>     (nil))
>>
>> (insn 15 14 16 2 (set (reg:DI 208)
>>        (and:DI (reg:DI 207)
>>            (const_int 4294967295 [0xffffffff]))) x.c:21 182 {*anddi3}
>>     (expr_list:REG_DEAD (reg:DI 207)
>>        (nil)))
>>
>> (jump_insn 16 15 17 2 (set (pc)
>>        (if_then_else (ne (reg:DI 206 [ *last_3(D)+-4 ])
>>                (reg:DI 208))
>>            (label_ref:DI 35)
>>            (pc))) x.c:21 473 {*branch_equalitydi}
>>     (expr_list:REG_DEAD (reg:DI 208)
>>        (expr_list:REG_DEAD (reg:DI 206 [ *last_3(D)+-4 ])
>>            (int_list:REG_BR_PROB 8010 (nil))))
>> -> 35)
>>
>> Into:
>>
>> (jump_insn 16 15 17 2 (set (pc)
>>        (if_then_else (ne (subreg:SI (reg:DI 207) 4)
>>                (subreg:SI (reg:DI 196 [ *last_3(D)+-4 ]) 4))
>>            (label_ref:DI 35)
>>            (pc))) x.c:21 472 {*branch_equalitysi}
>>     (expr_list:REG_DEAD (reg:DI 207)
>>        (int_list:REG_BR_PROB 8010 (nil)))
>> -> 35)
>>
>>
>> The problem is that the jump_insn on MIPS64 generates a beq/bne instruction
>> that compares the entire 64 bit registers and there is no option to only
>> look at the bottom 32 bits.  When we got rid of insn 15 we lost the AND that
>> cleared the upper 32 bits of one of the registers and the program fails.
>>
>> My solution was to not allow subregs in the conditional jump instruction.
>> Here is the patch and a test case and I ran the GCC testsuite with no
>> regressions.
>>
>> OK to checkin?
>
> No this is the wrong approach. The problem is in combine. I had submitted a patch to fix a while back but I never got around to the comments to make it consistent with the rest of combine.
>
> Let me dig up my patch in a few minutes.

So this is recorded as https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67736 .
And my patch which I submitted 3 years ago to fix this (though not
fixed up for the comments) is here:
https://gcc.gnu.org/ml/gcc-patches/2012-05/msg00401.html .

Basically combine's simplify_comparison uses gen_lowpart instead of
gen_lowpart_or_truncate but Erir's comment is also true so just update
my patch to Eric's comments instead.

Thanks,
Andrew

>
> Thanks,
> Andrew
>
>>
>> Steve Ellcey
>> sellcey@imgtec.com
>>
>>
>> 2015-10-21  Steve Ellcey  <sellcey@imgtec.com>
>>
>>    * mips.c (mips_legitimate_combined_insn): New function.
>>    (TARGET_LEGITIMATE_COMBINED_INSN): New macro.
>>
>>
>> diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
>> index 0b4a5fa..4338628 100644
>> --- a/gcc/config/mips/mips.c
>> +++ b/gcc/config/mips/mips.c
>> @@ -19606,6 +19606,26 @@ mips_ira_change_pseudo_allocno_class (int regno, reg_class_t allocno_class)
>>     return GR_REGS;
>>   return allocno_class;
>> }
>> +
>> +/* Implement TARGET_LEGITIMATE_COMBINED_INSN */
>> +
>> +static bool
>> +mips_legitimate_combined_insn (rtx_insn* insn)
>> +{
>> +  /* If we do a conditional jump involving register compares do not allow
>> +     subregs because beq/bne will always compare the entire register.
>> +     This should only be an issue with N32/N64 ABI's that do a 32 bit
>> +     compare and jump.  */
>> +
>> +  if (any_condjump_p (insn))
>> +    {
>> +      rtx cond = XEXP (SET_SRC (pc_set (insn)), 0);
>> +      if (GET_RTX_CLASS (GET_CODE (cond)) == RTX_COMPARE
>> +      || GET_RTX_CLASS (GET_CODE (cond)) == RTX_COMM_COMPARE)
>> +    return !SUBREG_P (XEXP (cond, 0)) && !SUBREG_P (XEXP (cond, 1));
>> +    }
>> +  return true;
>> +}
>>
>> /* Initialize the GCC target structure.  */
>> #undef TARGET_ASM_ALIGNED_HI_OP
>> @@ -19868,6 +19888,9 @@ mips_ira_change_pseudo_allocno_class (int regno, reg_class_t allocno_class)
>> #undef TARGET_HARD_REGNO_SCRATCH_OK
>> #define TARGET_HARD_REGNO_SCRATCH_OK mips_hard_regno_scratch_ok
>>
>> +#undef TARGET_LEGITIMATE_COMBINED_INSN
>> +#define TARGET_LEGITIMATE_COMBINED_INSN mips_legitimate_combined_insn
>> +
>> struct gcc_target targetm = TARGET_INITIALIZER;
>>
>> #include "gt-mips.h"
>>
>>
>>
>>
>>
>> 2015-10-21  Steve Ellcey  <sellcey@imgtec.com>
>>
>>    * gcc.dg/combine-subregs.c: New test.
>>
>>
>> diff --git a/gcc/testsuite/gcc.dg/combine-subregs.c b/gcc/testsuite/gcc.dg/combine-subregs.c
>> index e69de29..c480f88 100644
>> --- a/gcc/testsuite/gcc.dg/combine-subregs.c
>> +++ b/gcc/testsuite/gcc.dg/combine-subregs.c
>> @@ -0,0 +1,36 @@
>> +/* { dg-do run } */
>> +/* { dg-options "-O2 -fexpensive-optimizations" } */
>> +
>> +#include <inttypes.h>
>> +#include <stdlib.h>
>> +
>> +void __attribute__ ((noinline))
>> +foo (uint64_t state, uint32_t last)
>> +{
>> +  if (state == last) abort ();
>> +}
>> +
>> +/* This function may do a bad comparision by trying to
>> +   use SUBREGS during the compare on machines where comparing
>> +   two registers always compares the entire register regardless
>> +   of mode.  */
>> +
>> +int __attribute__ ((noinline))
>> +compare (uint64_t state, uint32_t *last, uint8_t buf)
>> +{
>> +    if (*last == ((state | buf) & 0xFFFFFFFF)) {
>> +    foo (state, *last);
>> +        return 0;
>> +    }
>> +    return 1;
>> +}
>> +
>> +int
>> +main(int argc, char **argv) {
>> +    uint64_t state = 0xF00000100U;
>> +    uint32_t last  = 0x101U;
>> +    int ret        = compare(state, &last, 0x01);
>> +    if (ret != 0)
>> +    abort ();
>> +    exit (0);
>> +}

Re: [Patch, MIPS] Patch to fix MIPS optimization bug in combine

[Andrew Pinski]

On Thu, Oct 22, 2015 at 3:47 AM, Segher Boessenkool
<segher@kernel.crashing.org> wrote:
> Hi,
>
> On Wed, Oct 21, 2015 at 09:44:25AM -0700, Steve Ellcey  wrote:
>> (jump_insn 16 15 17 2 (set (pc)
>>         (if_then_else (ne (subreg:SI (reg:DI 207) 4)
>>                 (subreg:SI (reg:DI 196 [ *last_3(D)+-4 ]) 4))
>>             (label_ref:DI 35)
>>             (pc))) x.c:21 472 {*branch_equalitysi}
>>      (expr_list:REG_DEAD (reg:DI 207)
>>         (int_list:REG_BR_PROB 8010 (nil)))
>>  -> 35)
>
> Why does *branch_equalitysi allow these subregs if it cannot handle them?

It can handle subreg here if the register was signed extended from SI
mode in the first place (I have seen cases where we want to use the
subreg).  In this case combine should not have generated the subreg
but rather truncation for this comparison.  See my other email about
this.

Thanks,
Andrew

> Not just combine can generate code like this (in theory at least).
>
>
> Segher

Re: [Patch, MIPS] Patch to fix MIPS optimization bug in combine

[Steve Ellcey]

Andrew,

Here is the new patch that I am currently testing with your change and
incorporating Eric's comment.  I included both test cases but renamed
yours and put it into gcc.dg/torture.  Does the code in combine.c to
address Eric's comment look OK to you?

Steve Ellcey
steve.ellcey@imgtec.com


2015-10-21  Steve Ellcey  <sellcey@imgtec.com>
	    Andrew Pinski  <apinski@cavium.com>

	* combine.c (simplify_comparison): Use gen_lowpart_or_truncate instead
	of gen_lowpart when we had a truncating and.


diff --git a/gcc/combine.c b/gcc/combine.c
index fd3e19c..7568ebb 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -11529,8 +11529,8 @@ simplify_comparison (enum rtx_code code, rtx *pop0, rtx *pop1)
 		 tmode != GET_MODE (op0); tmode = GET_MODE_WIDER_MODE (tmode))
 	      if ((unsigned HOST_WIDE_INT) c0 == GET_MODE_MASK (tmode))
 		{
-		  op0 = gen_lowpart (tmode, inner_op0);
-		  op1 = gen_lowpart (tmode, inner_op1);
+		  op0 = gen_lowpart_or_truncate (tmode, inner_op0);
+		  op1 = gen_lowpart_or_truncate (tmode, inner_op1);
 		  code = unsigned_condition (code);
 		  changed = 1;
 		  break;
@@ -12048,12 +12048,9 @@ simplify_comparison (enum rtx_code code, rtx *pop0, rtx *pop1)
 				   & GET_MODE_MASK (mode))
 				  + 1)) >= 0
 	      && const_op >> i == 0
-	      && (tmode = mode_for_size (i, MODE_INT, 1)) != BLKmode
-	      && (TRULY_NOOP_TRUNCATION_MODES_P (tmode, GET_MODE (op0))
-		  || (REG_P (XEXP (op0, 0))
-		      && reg_truncated_to_mode (tmode, XEXP (op0, 0)))))
+	      && (tmode = mode_for_size (i, MODE_INT, 1)) != BLKmode)
 	    {
-	      op0 = gen_lowpart (tmode, XEXP (op0, 0));
+	      op0 = gen_lowpart_or_truncate (tmode, XEXP (op0, 0));
 	      continue;
 	    }
 

2015-10-21  Steve Ellcey  <sellcey@imgtec.com>
	    Andrew Pinski  <apinski@cavium.com>

	* gcc.dg/torture/pr67736.c: New test.
	* gcc.dg/combine-subregs.c: New test.

diff --git a/gcc/testsuite/gcc.dg/combine-subregs.c b/gcc/testsuite/gcc.dg/combine-subregs.c
index e69de29..c480f88 100644
--- a/gcc/testsuite/gcc.dg/combine-subregs.c
+++ b/gcc/testsuite/gcc.dg/combine-subregs.c
@@ -0,0 +1,36 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -fexpensive-optimizations" } */
+
+#include <inttypes.h>
+#include <stdlib.h>
+
+void __attribute__ ((noinline))
+foo (uint64_t state, uint32_t last)
+{
+  if (state == last) abort ();
+}
+
+/* This function may do a bad comparision by trying to
+   use SUBREGS during the compare on machines where comparing
+   two registers always compares the entire register regardless
+   of mode.  */
+
+int __attribute__ ((noinline))
+compare (uint64_t state, uint32_t *last, uint8_t buf)
+{
+    if (*last == ((state | buf) & 0xFFFFFFFF)) {
+	foo (state, *last);
+        return 0;
+    }
+    return 1;
+}
+
+int
+main(int argc, char **argv) {
+    uint64_t state = 0xF00000100U;
+    uint32_t last  = 0x101U;
+    int ret        = compare(state, &last, 0x01);
+    if (ret != 0)
+	abort ();
+    exit (0);
+}
diff --git a/gcc/testsuite/gcc.dg/torture/pr67736.c b/gcc/testsuite/gcc.dg/torture/pr67736.c
index e69de29..b45874e 100644
--- a/gcc/testsuite/gcc.dg/torture/pr67736.c
+++ b/gcc/testsuite/gcc.dg/torture/pr67736.c
@@ -0,0 +1,32 @@
+/* { dg-do run } */
+
+#include <stdlib.h>
+
+typedef unsigned long long uint64_t;
+void f(uint64_t *a, uint64_t aa) __attribute__((noinline));
+void f(uint64_t *a, uint64_t aa)
+{
+  uint64_t new_value = aa;
+  uint64_t old_value = *a;
+  int bit_size = 32;
+    uint64_t mask = (uint64_t)(unsigned)(-1);
+    uint64_t tmp = old_value & mask;
+    new_value &= mask;
+    /* On overflow we need to add 1 in the upper bits */
+    if (tmp > new_value)
+        new_value += 1ull<<bit_size;
+    /* Add in the upper bits from the old value */
+    new_value += old_value & ~mask;
+    *a = new_value;
+}
+int main(void)
+{
+  uint64_t value, new_value, old_value;
+  value = 0x100000001;
+  old_value = value;
+  new_value = (value+1)&(uint64_t)(unsigned)(-1);
+  f(&value, new_value);
+  if (value != old_value+1)
+    __builtin_abort ();
+  return 0;
+}

Re: [Patch, MIPS] Patch to fix MIPS optimization bug in combine

[Eric Botcazou]

> Here is the new patch that I am currently testing with your change and
> incorporating Eric's comment.  I included both test cases but renamed
> yours and put it into gcc.dg/torture.  Does the code in combine.c to
> address Eric's comment look OK to you?
> 
> Steve Ellcey
> steve.ellcey@imgtec.com
> 
> 
> 2015-10-21  Steve Ellcey  <sellcey@imgtec.com>
> 	    Andrew Pinski  <apinski@cavium.com>
> 
> 	* combine.c (simplify_comparison): Use gen_lowpart_or_truncate instead
> 	of gen_lowpart when we had a truncating and.

The patch is OK for mainline if testing succeeded and you remove the obsolete 
last part of the comment:

	     Note that in:

	     (ne:DI (and:DI (reg:DI 4) (const_int 0xffffffff)) (const_int 0))
	     -> (ne:DI (reg:SI 4) (const_int 0))

	     unless TRULY_NOOP_TRUNCATION allows it or the register is
	     known to hold a value of the required mode the
	     transformation is invalid.  */

-- 
Eric Botcazou