From patchwork Wed Nov 30 16:47:44 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kyrill Tkachov X-Patchwork-Id: 701079 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3tTRDp1r9wz9t37 for ; Thu, 1 Dec 2016 03:48:07 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="O5oRo3Xu"; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:content-type; q=dns; s=default; b=BWHa5yZ59qhjx1LWHUoEovwykXz8cJZW6D6HRQuyi6Q xCgrL7idQlm9ibUS7ZKYF4+L1RdBlDBduXRo+7F8Ni+DTtNGyG3xnNkPbd/UMCvB NkaPZU2QcCLMbIsEZthRJNJuDV7gsV34EHxNG/XH6LFrQRv+9KOo+rvRpqlVY818 = DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:content-type; s=default; bh=+S9DsytOvRWHGF91NDT9Pde82Z8=; b=O5oRo3XuhbyOgY+4p WMYRQxXpyy+BMLj2fZlHr/U0ICV1YTqSy43c9NyKSF/EyjFhUexTeOE+WkAJQfmC 0GLbtq91MjHz99XzaH53a8oILG+jtQoYgf+2zVO+sbIK97AyCTeQRKwViZF4zZNf GevhZHFYX4N29E1TnIirxVTsZs= Received: (qmail 65433 invoked by alias); 30 Nov 2016 16:47:59 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 65420 invoked by uid 89); 30 Nov 2016 16:47:58 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.8 required=5.0 tests=BAYES_00, KAM_LAZY_DOMAIN_SECURITY, RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=restrict, 20161130 X-HELO: foss.arm.com Received: from foss.arm.com (HELO foss.arm.com) (217.140.101.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 30 Nov 2016 16:47:48 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 992FAAD7; Wed, 30 Nov 2016 08:47:46 -0800 (PST) Received: from [10.2.207.77] (e100706-lin.cambridge.arm.com [10.2.207.77]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 0470A3F445; Wed, 30 Nov 2016 08:47:45 -0800 (PST) Message-ID: <583F02B0.3030406@foss.arm.com> Date: Wed, 30 Nov 2016 16:47:44 +0000 From: Kyrill Tkachov User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: GCC Patches CC: Ramana Radhakrishnan , Richard Earnshaw Subject: [PATCH][ARM] PR target/71436: Restrict *load_multiple pattern till after LRA 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 PR target/71436 * config/arm/arm.md (*load_multiple): Add reload_completed to matching condition. 2016-11-30 Kyrylo Tkachov PR target/71436 * gcc.c-torture/compile/pr71436.c: New test. commit 996d28e2353badd1b29ef000f94d40c7dab9010f Author: Kyrylo Tkachov 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 "" ;; 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); +}