Hi!

As the testcase shows, the compiler hangs and eats all memory when compiling
it.  This is because in r11-7274-gdecd8fb0128870d0d768ba53dae626913d6d9c54
I have changed the ix86_avoid_lea_for_addr splitting from a splitter
into a peephole2 (because during splitting passes we don't have guaranteed
df, while during peephole2 we do).
The problem is we have another peephole2 that works in the opposite way,
when seeing split lea (in particular ASHIFT followed by PLUS) it attempts
to turn it back into a lea.
In the past, they were fighting against each other, but as they were in
different passes, simply the last one won.  So, split after reload
split the lea into shift left and plus, peephole2 reverted that (but, note
not perfectly, the peephole2 doesn't understand that something can be placed
into lea disp; to be fixed for GCC12) and then another split pass split the
lea appart again.
But my changes and the way peephole2 works means that we endlessly iterate
over those two, the first peephole2 splits the lea, the second one reverts
it, the first peephole2 splits the new lea back into new 2 insns and so
forth forever.
So, we need to break the cycle somehow.  Best would be to call
ix86_avoid_lea_for_addr in the second peephole2 and if it says it would be
split, undo, but unfortunately calling that function is very non-trivial,
because it needs to walk insns forwards and backwards from the given insn
and uses df in those walks.  Furthermore, one of the registers in the new
lea is set by the newly added insn.  So I'd be afraid we'd need to
temporarily register the new insns with df and replace in the IL the old
insns with new insns (and undo their df), then call the function and then
undo everything we did.
So, this patch instead breaks the cycle by remembering INSN_UID of the last
ASHIFT ix86_split_lea_for_addr emitted and punting on the ASHIFT + PLUS
peephole2 when it is that uid.  We need to reset it somewhere, so I've
added clearing of the var to ix86_expand_prologue which is guaranteed to be
a few passes before peephole2.

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

Or would you e.g. prefer if I don't use a global var but stick it into
cfun->machine->last_lea_split_shift_uid ?  That way it would be cleared
automatically.

2021-03-16  Jakub Jelinek  <ja...@redhat.com>

        PR target/99600
        * config/i386/i386-protos.h (ix86_last_lea_split_shift_uid): Declare.
        * config/i386/i386-expand.c (ix86_last_lea_split_shift_uid): New
        variable.
        (ix86_split_lea_for_addr): Set it to INSN_UID of the emitted shift
        insn.
        * config/i386/i386.md (ashift + add peephole2 into lea): Punt if
        INSN_UID of ashift is ix86_last_lea_split_shift_uid.
        * config/i386/i386.c (ix86_expand_prologue): Clear
        ix86_last_lea_split_shift_uid.

        * gcc.target/i386/pr99600.c: New test.

--- gcc/config/i386/i386-protos.h.jj    2021-01-04 10:25:45.436159056 +0100
+++ gcc/config/i386/i386-protos.h       2021-03-15 20:06:36.267515383 +0100
@@ -109,6 +109,7 @@ extern bool ix86_binary_operator_ok (enu
 extern bool ix86_avoid_lea_for_add (rtx_insn *, rtx[]);
 extern bool ix86_use_lea_for_mov (rtx_insn *, rtx[]);
 extern bool ix86_avoid_lea_for_addr (rtx_insn *, rtx[]);
+extern int ix86_last_lea_split_shift_uid;
 extern void ix86_split_lea_for_addr (rtx_insn *, rtx[], machine_mode);
 extern bool ix86_lea_for_add_ok (rtx_insn *, rtx[]);
 extern bool ix86_vec_interleave_v2df_operator_ok (rtx operands[3], bool high);
--- gcc/config/i386/i386-expand.c.jj    2021-03-15 12:34:26.549901726 +0100
+++ gcc/config/i386/i386-expand.c       2021-03-15 20:08:52.992016204 +0100
@@ -1291,6 +1291,10 @@ find_nearest_reg_def (rtx_insn *insn, in
   return false;
 }
 
+/* INSN_UID of the last ASHIFT insn emitted by ix86_split_lea_for_addr
+   or zero if none has been emitted yet.  */
+int ix86_last_lea_split_shift_uid;
+
 /* Split lea instructions into a sequence of instructions
    which are executed on ALU to avoid AGU stalls.
    It is assumed that it is allowed to clobber flags register
@@ -1352,6 +1356,9 @@ ix86_split_lea_for_addr (rtx_insn *insn,
          ix86_emit_binop (ASHIFT, mode, target,
                           GEN_INT (exact_log2 (parts.scale)));
 
+         rtx_insn *ashift_insn = get_last_insn ();
+         ix86_last_lea_split_shift_uid = INSN_UID (ashift_insn);
+
          if (parts.base)
            ix86_emit_binop (PLUS, mode, target, parts.base);
 
--- gcc/config/i386/i386.md.jj  2021-03-05 21:51:33.675350463 +0100
+++ gcc/config/i386/i386.md     2021-03-15 20:16:03.796292449 +0100
@@ -20680,6 +20680,11 @@ (define_peephole2
                         (match_operand 4 "x86_64_general_operand")))
                   (clobber (reg:CC FLAGS_REG))])]
   "IN_RANGE (INTVAL (operands[2]), 1, 3)
+   /* Avoid the ix86_avoid_lea_for_addr peephole2 from splitting
+      lea and this peephole2 to undo that into another lea that
+      would be split again.  Break that cycle by ignoring the last
+      shift that has been emitted by ix86_avoid_lea_for_addr.  */
+   && INSN_UID (insn) != ix86_last_lea_split_shift_uid
    /* Validate MODE for lea.  */
    && ((!TARGET_PARTIAL_REG_STALL
        && (GET_MODE (operands[0]) == QImode
--- gcc/config/i386/i386.c.jj   2021-03-04 16:00:44.062251517 +0100
+++ gcc/config/i386/i386.c      2021-03-15 20:09:40.382496567 +0100
@@ -8155,6 +8155,7 @@ ix86_expand_prologue (void)
   bool save_stub_call_needed;
   rtx static_chain = NULL_RTX;
 
+  ix86_last_lea_split_shift_uid = 0;
   if (ix86_function_naked (current_function_decl))
     {
       if (flag_stack_usage_info)
--- gcc/testsuite/gcc.target/i386/pr99600.c.jj  2021-03-15 20:26:43.868274088 
+0100
+++ gcc/testsuite/gcc.target/i386/pr99600.c     2021-03-15 20:20:08.802605967 
+0100
@@ -0,0 +1,16 @@
+/* PR target/99600 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -march=atom" } */
+
+char a, b;
+long c;
+
+long
+foo (void)
+{
+  if (a)
+    c = b == 1 ? 1 << 3 : 1 << 2;
+  else
+    c = 0;
+  return 0;
+}

        Jakub

Reply via email to