diff mbox

[RTL,ifcvt] PR rtl-optimization/71594: ICE in noce_emit_cmove due to mismatched source modes

Message ID 577BBB0E.4010509@foss.arm.com
State New
Headers show

Commit Message

Kyrill Tkachov July 5, 2016, 1:50 p.m. UTC 
On 04/07/16 12:19, Bernd Schmidt wrote:
> On 07/04/2016 01:18 PM, Kyrill Tkachov wrote:
>> That does seem like it could cause trouble but I couldn't think of how
>> that sequence could appear or what its
>> semantics would be. Would assigning to the SImode reg 0 in your example
>> not touch the upper bits of the DImode value?
>
> No, multi-word subreg accesses are per-word.
>
>> In any case, bb_ok_for_noce_convert_multiple_sets doesn't keep track of
>> dependencies between the instructions
>> so I think the best place to handle this case would be in
>> noce_convert_multiple_sets where instead of the assert
>> in this patch we'd just end_sequence () and return FALSE.
>> Would that be preferable?
>
> That should at least work, and I'd be ok with that.
>

Ok, here's the updated patch with the assert replaced by failing the conversion.
Bootstrapped and tested on x86_64. Also tested on aarch64.

Is this ok?

Thanks,
Kyrill

2016-07-05  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     PR rtl-optimization/71594
     * ifcvt.c (noce_convert_multiple_sets): Wrap new_val or old_val
     into subregs of appropriate mode before trying to emit a conditional
     move.

2016-07-05  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     PR rtl-optimization/71594
     * gcc.dg/torture/pr71594.c: New test.

Comments

Bernd Schmidt July 5, 2016, 3:42 p.m. UTC | #1 
On 07/05/2016 03:50 PM, Kyrill Tkachov wrote:
> Ok, here's the updated patch with the assert replaced by failing the
> conversion.
> Bootstrapped and tested on x86_64. Also tested on aarch64.
>
> Is this ok?

Sure. Thanks!


Bernd
diff mbox

Patch

diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
index fd2951673fb6bd6d9e5d52cdb88765434a603fb6..f7f120e04b11dc4f25be969e0c183a36e067a61c 100644
--- a/gcc/ifcvt.c
+++ b/gcc/ifcvt.c
@@ -3228,6 +3228,41 @@  noce_convert_multiple_sets (struct noce_if_info *if_info)
       if (if_info->then_else_reversed)
 	std::swap (old_val, new_val);
 
+
+      /* We allow simple lowpart register subreg SET sources in
+	 bb_ok_for_noce_convert_multiple_sets.  Be careful when processing
+	 sequences like:
+	 (set (reg:SI r1) (reg:SI r2))
+	 (set (reg:HI r3) (subreg:HI (r1)))
+	 For the second insn new_val or old_val (r1 in this example) will be
+	 taken from the temporaries and have the wider mode which will not
+	 match with the mode of the other source of the conditional move, so
+	 we'll end up trying to emit r4:HI = cond ? (r1:SI) : (r3:HI).
+	 Wrap the two cmove operands into subregs if appropriate to prevent
+	 that.  */
+      if (GET_MODE (new_val) != GET_MODE (temp))
+	{
+	  machine_mode src_mode = GET_MODE (new_val);
+	  machine_mode dst_mode = GET_MODE (temp);
+	  if (GET_MODE_SIZE (src_mode) <= GET_MODE_SIZE (dst_mode))
+	    {
+	      end_sequence ();
+	      return FALSE;
+	    }
+	  new_val = lowpart_subreg (dst_mode, new_val, src_mode);
+	}
+      if (GET_MODE (old_val) != GET_MODE (temp))
+	{
+	  machine_mode src_mode = GET_MODE (old_val);
+	  machine_mode dst_mode = GET_MODE (temp);
+	  if (GET_MODE_SIZE (src_mode) <= GET_MODE_SIZE (dst_mode))
+	    {
+	      end_sequence ();
+	      return FALSE;
+	    }
+	  old_val = lowpart_subreg (dst_mode, old_val, src_mode);
+	}
+
       /* Actually emit the conditional move.  */
       rtx temp_dest = noce_emit_cmove (if_info, temp, cond_code,
 				       x, y, new_val, old_val);
diff --git a/gcc/testsuite/gcc.dg/torture/pr71594.c b/gcc/testsuite/gcc.dg/torture/pr71594.c
new file mode 100644
index 0000000000000000000000000000000000000000..468a9f6891c92ff76520af0eee242f08b01ae0cf
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr71594.c
@@ -0,0 +1,15 @@ 
+/* { dg-do compile } */
+/* { dg-options "--param max-rtl-if-conversion-insns=2" } */
+
+unsigned short a;
+int b, c;
+int *d;
+void fn1() {
+  *d = 24;
+  for (; *d <= 65;) {
+    unsigned short *e = &a;
+    b = (a &= 0 <= 0) < (c ?: (*e %= *d));
+    for (; *d <= 83;)
+      ;
+  }
+}