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.tkac...@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.tkac...@arm.com>
PR rtl-optimization/71594
* gcc.dg/torture/pr71594.c: New test.
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;)
+ ;
+ }
+}
