From patchwork Fri Nov 27 13:01:01 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Greenhalgh X-Patchwork-Id: 57368 Delivered-To: patch@linaro.org Received: by 10.112.155.196 with SMTP id vy4csp1130023lbb; Fri, 27 Nov 2015 05:01:33 -0800 (PST) X-Received: by 10.66.234.226 with SMTP id uh2mr49450411pac.6.1448629293189; Fri, 27 Nov 2015 05:01:33 -0800 (PST) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id g7si16337072pat.185.2015.11.27.05.01.32 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 27 Nov 2015 05:01:33 -0800 (PST) Received-SPF: pass (google.com: domain of gcc-patches-return-415642-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; spf=pass (google.com: domain of gcc-patches-return-415642-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=gcc-patches-return-415642-patch=linaro.org@gcc.gnu.org; dkim=pass header.i=@gcc.gnu.org DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-type; q=dns; s=default; b=v7WBxlZvSs9xsCot zBSvPoqSO0Fwqr2WdyyCT2ceEe10Kb99004xcFHKvTSewjgOUbdcGq5pxkMTNVLS puUbURgeq4H794nqn6pCqM47xrDRBwzhSmwS/T2Qdkeg36npB/QO6RZPq2UL+Wsc ECi/TrEyzGJ8u31hioltUcxT9Sk= 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:from :to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-type; s=default; bh=FWpU8eV8K2+oH3m1TGOSwc Ndzgw=; b=uvOyo02HHWqWeQXbPHWNluu08D9JakhbRGVshX8I9yZ0hVrUx/H0sC 7TJ9L76tKHi/TMPQIB/WyVgw1V8jOnrLe/Nk7D4Dl2XG8RH9EUb+2jJz4fntciYK /z985vXap9DetSinRwA70yno0BFLpg1j00SGJSextoQfh/XJdRKJo= Received: (qmail 83930 invoked by alias); 27 Nov 2015 13:01:22 -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 81426 invoked by uid 89); 27 Nov 2015 13:01:20 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL, BAYES_00, SPF_PASS autolearn=ham version=3.3.2 X-HELO: eu-smtp-delivery-143.mimecast.com Received: from eu-smtp-delivery-143.mimecast.com (HELO eu-smtp-delivery-143.mimecast.com) (207.82.80.143) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 27 Nov 2015 13:01:13 +0000 Received: from cam-owa2.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.140]) by eu-smtp-1.mimecast.com with ESMTP id uk-mta-20-Qp3hulsbR2atoeMjrtOqsg-1; Fri, 27 Nov 2015 13:01:06 +0000 Received: from e107456-lin.cambridge.arm.com ([10.1.2.79]) by cam-owa2.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Fri, 27 Nov 2015 13:01:05 +0000 From: James Greenhalgh To: gcc-patches@gcc.gnu.org Cc: richard.guenther@gmail.com, rth@redhat.com, marcus.shawcroft@arm.com, richard.earnshaw@arm.com Subject: [Patch AArch64] Reinstate CANNOT_CHANGE_MODE_CLASS to fix pr67609 Date: Fri, 27 Nov 2015 13:01:01 +0000 Message-Id: <1448629261-29938-1-git-send-email-james.greenhalgh@arm.com> In-Reply-To: <5656E50A.3040903@redhat.com> References: <5656E50A.3040903@redhat.com> MIME-Version: 1.0 X-MC-Unique: Qp3hulsbR2atoeMjrtOqsg-1 X-IsSubscribed: yes Hi, This patch follow Richard Henderson's advice to tighten up CANNOT_CHANGE_MODE_CLASS for AArch64 to avoid a simplification bug in the middle-end. There is nothing AArch64-specific about the testcase which triggers this, so I'll put it in the testcase for other targets. If you see a regression, the explanation in the PR is much more thorough and correct than I can reproduce here, so I'd recommend starting there. In short, target maintainers need to: > forbid BITS_PER_WORD (64-bit) subregs of hard registers > > BITS_PER_WORD. See the verbiage I added to the i386 backend for this. We removed the CANNOT_CHANGE_MODE_CLASS macro back in January 2015. Before then, we used it to workaround bugs in big-endian vector support ( https://gcc.gnu.org/ml/gcc-patches/2014-12/msg01216.html ). Ideally, we'd not need to bring this macro back, but if we can't fix the middle-end bug this exposes, we need the workaround. For AArch64, doing this runs in to some trouble with two of our instruction patterns - we end up with: (truncate:DI (reg:TF)) Which fails if it ever make it through to the simplify routines with something nasty like: (and:DI (truncate:DI (reg:TF 32 v0 [ a ])) (const_int 2305843009213693951 [0x1fffffffffffffff])) The simplify routines want to turn this around to look like: (truncate:DI (and:TF (reg:TF 32 v0 [ a ]) (const_int 2305843009213693951 [0x1fffffffffffffff]))) Which then wants to further simplify the expression by first building the constant in TF mode, and trunc_int_for_mode barfs: 0x7a38a5 trunc_int_for_mode(long, machine_mode) .../gcc/explow.c:53 We can fix that by changing the patterns to use a zero_extract, which seems more in line with what they actually express (extracting the two 64-bit halves of a 128-bit value). Bootstrapped on aarch64-none-linux-gnu, and tested on aarch64-none-elf and aarch64_be-none-elf without seeing any correctness regressions. OK? If so, we ought to get this backported to the release branches, the gcc-5 backport applies clean (testing ongoing but looks OK so far) if the release managers and AArch64 maintainers agree this is something that should be backported this late in the 5.3 release cycle. Thanks, James --- 2015-11-27 James Greenhalgh * config/aarch64/aarch64-protos.h (aarch64_cannot_change_mode_class): Bring back. * config/aarch64/aarch64.c (aarch64_cannot_change_mode_class): Likewise. * config/aarch64/aarch64.h (CANNOT_CHANGE_MODE_CLASS): Likewise. * config/aarch64/aarch64.md (aarch64_movdi_low): Use zero_extract rather than truncate. (aarch64_movdi_high): Likewise. 2015-11-27 James Greenhalgh * gcc.dg/torture/pr67609.c: New. diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h index e0a050c..59d3da4 100644 --- a/gcc/config/aarch64/aarch64-protos.h +++ b/gcc/config/aarch64/aarch64-protos.h @@ -269,6 +269,9 @@ int aarch64_get_condition_code (rtx); bool aarch64_bitmask_imm (HOST_WIDE_INT val, machine_mode); int aarch64_branch_cost (bool, bool); enum aarch64_symbol_type aarch64_classify_symbolic_expression (rtx); +bool aarch64_cannot_change_mode_class (machine_mode, + machine_mode, + enum reg_class); bool aarch64_const_vec_all_same_int_p (rtx, HOST_WIDE_INT); bool aarch64_constant_address_p (rtx); bool aarch64_expand_movmem (rtx *); diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index 3fe2f0f..fadb716 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -12408,6 +12408,24 @@ aarch64_vectorize_vec_perm_const_ok (machine_mode vmode, return ret; } +/* Implement target hook CANNOT_CHANGE_MODE_CLASS. */ +bool +aarch64_cannot_change_mode_class (machine_mode from, + machine_mode to, + enum reg_class rclass) +{ + /* We cannot allow word_mode subregs of full vector modes. + Otherwise the middle-end will assume it's ok to store to + (subreg:DI (reg:TI 100) 0) in order to modify only the low 64 bits + of the 128-bit register. However, after reload the subreg will + be dropped leaving a plain DImode store. See PR67609 for a more + detailed dicussion. In all other cases, we want to be premissive + and return false. */ + return (reg_classes_intersect_p (FP_REGS, rclass) + && GET_MODE_SIZE (to) == UNITS_PER_WORD + && GET_MODE_SIZE (from) > UNITS_PER_WORD); +} + rtx aarch64_reverse_mask (enum machine_mode mode) { diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h index 68c006f..66b768d 100644 --- a/gcc/config/aarch64/aarch64.h +++ b/gcc/config/aarch64/aarch64.h @@ -831,6 +831,9 @@ do { \ extern void __aarch64_sync_cache_range (void *, void *); \ __aarch64_sync_cache_range (beg, end) +#define CANNOT_CHANGE_MODE_CLASS(FROM, TO, CLASS) \ + aarch64_cannot_change_mode_class (FROM, TO, CLASS) + #define SHIFT_COUNT_TRUNCATED !TARGET_SIMD /* Choose appropriate mode for caller saves, so we do the minimum diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md index 64a40ae..c3367df 100644 --- a/gcc/config/aarch64/aarch64.md +++ b/gcc/config/aarch64/aarch64.md @@ -4667,7 +4667,8 @@ (define_insn "aarch64_movdi_low" [(set (match_operand:DI 0 "register_operand" "=r") - (truncate:DI (match_operand:TX 1 "register_operand" "w")))] + (zero_extract:DI (match_operand:TX 1 "register_operand" "w") + (const_int 64) (const_int 0)))] "TARGET_FLOAT && (reload_completed || reload_in_progress)" "fmov\\t%x0, %d1" [(set_attr "type" "f_mrc") @@ -4676,9 +4677,8 @@ (define_insn "aarch64_movdi_high" [(set (match_operand:DI 0 "register_operand" "=r") - (truncate:DI - (lshiftrt:TX (match_operand:TX 1 "register_operand" "w") - (const_int 64))))] + (zero_extract:DI (match_operand:TX 1 "register_operand" "w") + (const_int 64) (const_int 64)))] "TARGET_FLOAT && (reload_completed || reload_in_progress)" "fmov\\t%x0, %1.d[1]" [(set_attr "type" "f_mrc") diff --git a/gcc/testsuite/gcc.dg/torture/pr67609.c b/gcc/testsuite/gcc.dg/torture/pr67609.c new file mode 100644 index 0000000..817857d --- /dev/null +++ b/gcc/testsuite/gcc.dg/torture/pr67609.c @@ -0,0 +1,33 @@ +/* { dg-do run } */ + +typedef union +{ + double v[2]; + double s __attribute__ ((vector_size (16))); +} data; + +data reg; + +void __attribute__ ((noinline)) +set_lower (double b) +{ + data stack_var; + double __attribute__ ((vector_size (16))) one = { 1.0, 1.0 }; + stack_var.s = reg.s; + stack_var.s += one; + stack_var.v[0] += b; + reg.s = stack_var.s; +} + +int +main (int argc, char ** argv) +{ + reg.v[0] = 1.0; + reg.v[1] = 1.0; + /* reg should contain { 1.0, 1.0 }. */ + set_lower (2.0); + /* reg should contain { 4.0, 2.0 }. */ + if ((int) reg.v[0] != 4 || (int) reg.v[1] != 2) + __builtin_abort (); + return 0; +}