[PATCH] optabs: Don't reuse target for multi-word expansions if it overlaps operand(s) [PR97073]

[PATCH] optabs: Don't reuse target for multi-word expansions if it overlaps operand(s) [PR97073]

[Jakub Jelinek]

Hi!

The following testcase is miscompiled on i686-linux, because
we try to expand a double-word bitwise logic operation with op0
being a (mem:DI u) and target (mem:DI u+4), i.e. partial overlap, and
thus end up with:
        movl    4(%esp), %eax
        andl    u, %eax
        movl    %eax, u+4
! movl u+4, %eax optimized out
        andl    8(%esp), %eax
        movl    %eax, u+8
rather than with the desired:
        movl    4(%esp), %edx
        movl    8(%esp), %eax
        andl    u, %edx
        andl    u+4, %eax
        movl    %eax, u+8
        movl    %edx, u+4
because the store of the first word to target overwrites the second word of
the operand.
expand_binop for this (and several similar places) already check for target
== op0 or target == op1, this patch just adds reg_overlap_mentioned_p calls
next to it.
Pedantically, at least for some of these it might be sufficient to force
a different target if there is overlap but target is not rtx_equal_p to
the operand (e.g. in this bitwise logical case, but e.g. not in the shift
cases where there is reordering), though that would go against the
preexisting target == op? checks and the rationale that REG_EQUAL notes in
that case isn't correct.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk and
release branches?

2020-09-18  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/97073
	* optabs.c (expand_binop, expand_absneg_bit, expand_unop,
	expand_copysign_bit): Check reg_overlap_mentioned_p between target
	and operand(s) and if it returns true, force a pseudo as target.

	* gcc.c-torture/execute/pr97073.c: New test.

--- gcc/optabs.c.jj	2020-07-28 15:39:09.000000000 +0200
+++ gcc/optabs.c	2020-09-17 22:07:53.238396458 +0200
@@ -1395,6 +1395,8 @@ expand_binop (machine_mode mode, optab b
       if (target == 0
 	  || target == op0
 	  || target == op1
+	  || reg_overlap_mentioned_p (target, op0)
+	  || reg_overlap_mentioned_p (target, op1)
 	  || !valid_multiword_target_p (target))
 	target = gen_reg_rtx (int_mode);
 
@@ -1475,6 +1477,8 @@ expand_binop (machine_mode mode, optab b
 	  if (target == 0
 	      || target == op0
 	      || target == op1
+	      || reg_overlap_mentioned_p (target, op0)
+	      || reg_overlap_mentioned_p (target, op1)
 	      || !valid_multiword_target_p (target))
 	    target = gen_reg_rtx (int_mode);
 
@@ -1533,6 +1537,8 @@ expand_binop (machine_mode mode, optab b
 	  || target == op0
 	  || target == op1
 	  || !REG_P (target)
+	  || reg_overlap_mentioned_p (target, op0)
+	  || reg_overlap_mentioned_p (target, op1)
 	  || !valid_multiword_target_p (target))
 	target = gen_reg_rtx (int_mode);
 
@@ -2670,6 +2676,7 @@ expand_absneg_bit (enum rtx_code code, s
 
   if (target == 0
       || target == op0
+      || reg_overlap_mentioned_p (target, op0)
       || (nwords > 1 && !valid_multiword_target_p (target)))
     target = gen_reg_rtx (mode);
 
@@ -2951,7 +2958,10 @@ expand_unop (machine_mode mode, optab un
       int i;
       rtx_insn *insns;
 
-      if (target == 0 || target == op0 || !valid_multiword_target_p (target))
+      if (target == 0
+	  || target == op0
+	  || reg_overlap_mentioned_p (target, op0)
+	  || !valid_multiword_target_p (target))
 	target = gen_reg_rtx (int_mode);
 
       start_sequence ();
@@ -3472,6 +3482,8 @@ expand_copysign_bit (scalar_float_mode m
   if (target == 0
       || target == op0
       || target == op1
+      || reg_overlap_mentioned_p (target, op0)
+      || reg_overlap_mentioned_p (target, op1)
       || (nwords > 1 && !valid_multiword_target_p (target)))
     target = gen_reg_rtx (mode);
 
--- gcc/testsuite/gcc.c-torture/execute/pr97073.c.jj	2020-09-17 22:00:39.778614611 +0200
+++ gcc/testsuite/gcc.c-torture/execute/pr97073.c	2020-09-17 22:06:08.870893863 +0200
@@ -0,0 +1,21 @@
+/* PR middle-end/97073 */
+/* { dg-additional-options "-mno-stv" { target i?86-*-* x86_64-*-* } } */
+
+typedef unsigned long long L;
+union U { L i; struct T { unsigned k; L l; } j; } u;
+
+__attribute__((noinline,noclone)) void
+foo (L x)
+{
+  u.j.l = u.i & x;
+}
+
+int
+main ()
+{
+  u.i = 5;
+  foo (-1ULL);
+  if (u.j.l != 5)
+    __builtin_abort ();
+  return 0;
+}

	Jakub

Re: [PATCH] optabs: Don't reuse target for multi-word expansions if it overlaps operand(s) [PR97073]

[Eric Botcazou]

> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk and
> release branches?
> 
> 2020-09-18  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR middle-end/97073
> 	* optabs.c (expand_binop, expand_absneg_bit, expand_unop,
> 	expand_copysign_bit): Check reg_overlap_mentioned_p between target
> 	and operand(s) and if it returns true, force a pseudo as target.
> 
> 	* gcc.c-torture/execute/pr97073.c: New test.

This looks good to me.

-- 
Eric Botcazou