On 11/14/14 12:19, Segher Boessenkool wrote:
If I2 is a PARALLEL of two SETs, split it into two instructions, I1
and I2.  If there already was an I1, rename it to I0.  If there
already was an I0, don't do anything.

This surprisingly simple patch is enough to let combine handle such
PARALLELs properly.
It's clever.



2014-11-14  Segher Boessenkool  <seg...@kernel.crashing.org>

gcc/
        * combine.c (try_combine): If I2 is a PARALLEL of two SETs,
        split it into two insns.
So you're virtually serializing the PARALLEL to make combine happy if I'm reading this correctly.

THe first thing I worry about is preserving the semantics of a PARALLEL. Specifically that all the inputs are evaluated, then all the side effects happen. So I think one of the checks you need is that the destinations of the SETs are not used as source operands in the SETs.

The second thing I worry about handling of match_dup operands. But presumably all the resulting insns must match in one way or another or the whole thing gets reset to its prior state. So I suspect those are OK as well.

Related, I was worried about RTL structure sharing, but in the end I think those are a non-concern for the same basic reasons as match_dups aren't a real worry.



---
  gcc/combine.c | 31 +++++++++++++++++++++++++++++++
  1 file changed, 31 insertions(+)

diff --git a/gcc/combine.c b/gcc/combine.c
index f7797e7..c4d23e3 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -2780,6 +2780,37 @@ try_combine (rtx_insn *i3, rtx_insn *i2, rtx_insn *i1, 
rtx_insn *i0,
          SUBST_LINK (LOG_LINKS (i2), alloc_insn_link (i1, LOG_LINKS (i2)));
        }
      }
+
+  /* If I2 is a PARALLEL of two SETs of REGs (and perhaps some CLOBBERs),
+     make those two SETs separate I1 and I2 insns, and make an I0 that is
+     the original I1.  */
+  if (i0 == 0
Test for NULL.


+      && GET_CODE (PATTERN (i2)) == PARALLEL
+      && XVECLEN (PATTERN (i2), 0) >= 2
+      && GET_CODE (XVECEXP (PATTERN (i2), 0, 0)) == SET
+      && GET_CODE (XVECEXP (PATTERN (i2), 0, 1)) == SET
+      && REG_P (SET_DEST (XVECEXP (PATTERN (i2), 0, 0)))
+      && REG_P (SET_DEST (XVECEXP (PATTERN (i2), 0, 1)))
+      && !reg_used_between_p (SET_DEST (XVECEXP (PATTERN (i2), 0, 0)), i2, i3)
+      && !reg_used_between_p (SET_DEST (XVECEXP (PATTERN (i2), 0, 1)), i2, i3)
+      && (XVECLEN (PATTERN (i2), 0) == 2
+         || GET_CODE (XVECEXP (PATTERN (i2), 0, 2)) == CLOBBER))
As noted above, I think you need to verify the set/clobbered operands do not conflict with any of the source operands. Otherwise you run the risk of changing the semantics when you rip apart the PARALLEL.

Ah, just saw that Bernd made the same observation.  Good.

And I think while convention has CLOBBERs at the end of insns, I don't think that's a hard requirement. So I think you need a stronger check for elements 2 and beyond in the vector.

OK with the direction this is going, but I think another iteration is going to be necessary.

Jeff





Reply via email to