public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH][ARM] PR target/71436: Restrict *load_multiple pattern till after LRA
@ 2016-11-30 16:47 Kyrill Tkachov
  2016-12-08 11:55 ` Kyrill Tkachov
  2016-12-15 10:00 ` Richard Earnshaw (lists)
  0 siblings, 2 replies; 11+ messages in thread
From: Kyrill Tkachov @ 2016-11-30 16:47 UTC (permalink / raw)
  To: GCC Patches; +Cc: Ramana Radhakrishnan, Richard Earnshaw

[-- Attachment #1: Type: text/plain, Size: 3016 bytes --]

Hi all,

In this awkward ICE we have a *load_multiple pattern that is being transformed in reload from:
(insn 55 67 151 3 (parallel [
             (set (reg:SI 0 r0)
                 (mem/u/c:SI (reg/f:SI 147) [2 c+0 S4 A32]))
             (set (reg:SI 158 [ c+4 ])
                 (mem/u/c:SI (plus:SI (reg/f:SI 147)
                         (const_int 4 [0x4])) [2 c+4 S4 A32]))
         ]) arm-crash.c:25 393 {*load_multiple}
      (expr_list:REG_UNUSED (reg:SI 0 r0)
         (nil)))


into the invalid:
(insn 55 67 70 3 (parallel [
             (set (reg:SI 0 r0)
                 (mem/u/c:SI (reg/f:SI 5 r5 [147]) [2 c+0 S4 A32]))
             (set (mem/c:SI (plus:SI (reg/f:SI 102 sfp)
                         (const_int -4 [0xfffffffffffffffc])) [4 %sfp+-12 S4 A32])
                 (mem/u/c:SI (plus:SI (reg/f:SI 5 r5 [147])
                         (const_int 4 [0x4])) [2 c+4 S4 A32]))
         ]) arm-crash.c:25 393 {*load_multiple}
      (nil))

The operands of *load_multiple are not validated through constraints like LRA is used to, but rather through
a match_parallel predicate which ends up calling ldm_stm_operation_p to validate the multiple sets.
But this means that LRA cannot reason about the constraints properly.
This two-regiseter load should not have used *load_multiple anyway, it should have used *ldm2_ from ldmstm.md
and indeed it did until the loop2_invariant pass which copied the ldm2_ pattern:
(insn 27 23 28 4 (parallel [
             (set (reg:SI 0 r0)
                 (mem/u/c:SI (reg/f:SI 147) [2 c+0 S4 A32]))
             (set (reg:SI 1 r1)
                 (mem/u/c:SI (plus:SI (reg/f:SI 147)
                         (const_int 4 [0x4])) [2 c+4 S4 A32]))
         ]) "ldm.c":25 385 {*ldm2_}
      (nil))

into:
(insn 55 19 67 3 (parallel [
             (set (reg:SI 0 r0)
                 (mem/u/c:SI (reg/f:SI 147) [2 c+0 S4 A32]))
             (set (reg:SI 158)
                 (mem/u/c:SI (plus:SI (reg/f:SI 147)
                         (const_int 4 [0x4])) [2 c+4 S4 A32]))
         ]) "ldm.c":25 404 {*load_multiple}
      (expr_list:REG_UNUSED (reg:SI 0 r0)
         (nil)))

Note that it now got recognised as load_multiple because the second register is not a hard register but the pseudo 158.
In any case, the solution suggested in the PR (and I agree with it) is to restrict *load_multiple to after reload.
The similar pattern *load_multiple_with_writeback also has a similar condition and the comment above *load_multiple says that
it's used to generate epilogues, which is done after reload anyway. For pre-reload load-multiples the patterns in ldmstm.md
should do just fine.

Bootstrapped and tested on arm-none-linux-gnueabihf.

Ok for trunk?

Thanks,
Kyrill

2016-11-30  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     PR target/71436
     * config/arm/arm.md (*load_multiple): Add reload_completed to
     matching condition.

2016-11-30  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     PR target/71436
     * gcc.c-torture/compile/pr71436.c: New test.

[-- Attachment #2: arm-ldm.patch --]
[-- Type: text/x-patch, Size: 1781 bytes --]

commit 996d28e2353badd1b29ef000f94d40c7dab9010f
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date:   Tue Nov 29 15:07:30 2016 +0000

    [ARM] Restrict *load_multiple pattern till after LRA

diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index 74c44f3..22d2a84 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -11807,12 +11807,15 @@ (define_insn "<crc_variant>"
 
 ;; Patterns in ldmstm.md don't cover more than 4 registers. This pattern covers
 ;; large lists without explicit writeback generated for APCS_FRAME epilogue.
+;; The operands are validated through the load_multiple_operation
+;; match_parallel predicate rather than through constraints so enable it only
+;; after reload.
 (define_insn "*load_multiple"
   [(match_parallel 0 "load_multiple_operation"
     [(set (match_operand:SI 2 "s_register_operand" "=rk")
           (mem:SI (match_operand:SI 1 "s_register_operand" "rk")))
         ])]
-  "TARGET_32BIT"
+  "TARGET_32BIT && reload_completed"
   "*
   {
     arm_output_multireg_pop (operands, /*return_pc=*/false,
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr71436.c b/gcc/testsuite/gcc.c-torture/compile/pr71436.c
new file mode 100644
index 0000000..ab08d5d
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr71436.c
@@ -0,0 +1,35 @@
+/* PR target/71436.  */
+
+#pragma pack(1)
+struct S0
+{
+  volatile int f0;
+  short f2;
+};
+
+void foo (struct S0 *);
+int a, d;
+static struct S0 b[5];
+static struct S0 c;
+void fn1 ();
+void
+main ()
+{
+  {
+    struct S0 e;
+    for (; d; fn1 ())
+      {
+        {
+          a = 3;
+          for (; a >= 0; a -= 1)
+            {
+              {
+                e = c;
+              }
+              b[a] = e;
+            }
+        }
+      }
+  }
+  foo (b);
+}

^ permalink raw reply	[flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-03-23  9:05 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-30 16:47 [PATCH][ARM] PR target/71436: Restrict *load_multiple pattern till after LRA Kyrill Tkachov
2016-12-08 11:55 ` Kyrill Tkachov
2016-12-15  9:55   ` Kyrill Tkachov
2016-12-15 10:00 ` Richard Earnshaw (lists)
2016-12-15 10:06   ` Richard Earnshaw (lists)
2016-12-19 15:04     ` Jakub Jelinek
2017-01-18  9:53       ` Kyrill Tkachov
2017-02-07 14:50         ` Kyrill Tkachov
2017-03-14 14:07           ` Kyrill Tkachov
2017-03-23  9:05           ` Ramana Radhakrishnan
2016-12-15 16:40   ` Kyrill Tkachov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).