From patchwork Wed Jun 28 13:29:48 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Sandiford X-Patchwork-Id: 781703 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 3wyNvL06Zmz9s4q for ; Wed, 28 Jun 2017 23:30:05 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="Lm3jzZSR"; 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:from :to:cc:subject:references:date:in-reply-to:message-id :mime-version:content-type; q=dns; s=default; b=v5KMOHoEnRez/00E HdbuLVMcgfJwRrZYD7hpKU4Nbddm3kpjOlbCzBPKHMjyElcvq1pDD2YRL4uv6KIJ SSb/B3QB0Z7Wh2xMxiU91gPDD0Ak4McNLuIb4bb20ZpeplKXXJnnpDrIIHkXkgBO itydlCCpsy1BOIpFiVmC3Li5Ipc= 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:references:date:in-reply-to:message-id :mime-version:content-type; s=default; bh=esHtENRlrakJjul1XI+A/m YBh10=; b=Lm3jzZSRy7BgPB3CFPVZqs+pP/ugIolxvi9Ww2D4/s/tie7sAkt9lF 6azy2a3uPXPc753XEs8ZPCs3VBHtTL7puId6L83Q/ZoX68RvY78Yrkq9euZ3biQM WHZTDmyyJiU6pYvaB+d1JosE7PTNFq7yEhJgd89sVkQvqzB/zFf5E= Received: (qmail 120824 invoked by alias); 28 Jun 2017 13:29:57 -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 120811 invoked by uid 89); 28 Jun 2017 13:29:56 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-10.6 required=5.0 tests=BAYES_00, FREEMAIL_FROM, GIT_PATCH_2, GIT_PATCH_3, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-wm0-f41.google.com Received: from mail-wm0-f41.google.com (HELO mail-wm0-f41.google.com) (74.125.82.41) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 28 Jun 2017 13:29:54 +0000 Received: by mail-wm0-f41.google.com with SMTP id b184so58884016wme.1 for ; Wed, 28 Jun 2017 06:29:53 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:mail-followup-to:cc:subject:references :date:in-reply-to:message-id:user-agent:mime-version; bh=KSpRJJXYLW4OhJmZBZ06GQ8RS8v4HRbsyU+ikNKExd8=; b=SRGkurLb5NrVrc6nqM4vS15jtpLYDF70st1eSu6WpdjFh1irVoHbk2FoKd9zgYDnth Vgr/Wgba5XMBZ9i7eqG95zX+6oK28zbwcIEHwo1vNlxD65EC9d1kk2+ld4StgAFqno/8 nvpO88DYZ2a7FK+LyoV58WhNUU382fv7Wq26qtco2MjhwFQzSS+dP7r1TsFUJB5AAHW3 qHGIHTz56uMG7mKTpwEO1yATx8sIuZSJ1WZNatZYbzxbHEMtaXZpXjXJ/2+lhbqe7L1A 1QqdCC5lOhbDRGnHc0ckVHT2BdNkgq4HsH0SZkuPwGRFLQ5UkgHY7z1hKUzJBcKcBl8U +89Q== X-Gm-Message-State: AKS2vOxDrBrM4cvyJ6txR0vQ9WzrKPPF0kNYAb8tg/xmrZykJA4jh790 OP5SBQiUOw+EElZouUc= X-Received: by 10.28.212.145 with SMTP id l139mr4265863wmg.110.1498656591937; Wed, 28 Jun 2017 06:29:51 -0700 (PDT) Received: from localhost (92.40.249.75.threembb.co.uk. [92.40.249.75]) by smtp.googlemail.com with ESMTPSA id a3sm2092030wra.17.2017.06.28.06.29.49 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 28 Jun 2017 06:29:50 -0700 (PDT) From: Richard Sandiford To: Richard Biener Mail-Followup-To: Richard Biener , GCC Patches , rdsandiford@googlemail.com Cc: GCC Patches Subject: [v2] PR81136: ICE from inconsistent DR_MISALIGNMENTs References: <87k244z2c5.fsf@linaro.org> <8760fmgb90.fsf@linaro.org> <874lv36lww.fsf@linaro.org> <87zicv55p0.fsf@linaro.org> Date: Wed, 28 Jun 2017 14:29:48 +0100 In-Reply-To: (Richard Biener's message of "Tue, 27 Jun 2017 10:41:29 +0200") Message-ID: <87vang6y0j.fsf_-_@googlemail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) MIME-Version: 1.0 Richard Biener writes: > On Mon, Jun 26, 2017 at 1:50 PM, Richard Sandiford > wrote: >> Richard Biener writes: >>> On Mon, Jun 26, 2017 at 1:14 PM, Richard Sandiford >>> wrote: >>>> I don't think the problem is the lack of a cap. In the test case we >>>> see that: >>>> >>>> 1. B is known at compile time to be X * vecsize + Y when considered in >>>> isolation, because the base alignment derived from its DR_REF >= vecsize. >>>> So DR_MISALIGNMENT (B) == Y. >>>> >>>> 2. A's misalignment wrt vecsize is not known at compile time when >>>> considered in isolation, because no useful base alignment can be >>>> derived from its DR_REF. (The DR_REF is to a plain int rather than >>>> to a structure with a high alignment.) So DR_MISALIGNMENT (A) == -1. >>>> >>>> 3. A and B when considered as a pair trivially have the same misalignment >>>> wrt vecsize, for the reasons above. >>>> >>>> Each of these results is individually correct. The problem is that the >>>> assert is conflating two things: it's saying that if we know two datarefs >>>> have the same misaligment, we must either be able to calculate a >>>> compile-time misalignment for both datarefs in isolation, or we must >>>> fail to calculate a compile-time misalignment for both datarefs in >>>> isolation. That isn't true: it's valid to have situations in which the >>>> compile-time misalignment is known for one dataref in isolation but not >>>> for the other. >>> >>> True. So the assert should then become >>> >>> gcc_assert (! known_alignment_for_access_p (dr) >>> || DR_MISALIGNMENT (dr) / dr_size == >>> DR_MISALIGNMENT (dr_peel) / dr_peel_size); >>> >>> ? >> >> I think it would need to be: >> >> gcc_assert (!known_alignment_for_access_p (dr) >> || !known_alignment_for_access_p (dr_peel) >> || (DR_MISALIGNMENT (dr) / dr_size >> == DR_MISALIGNMENT (dr_peel) / dr_peel_size)); > > I think for !known_alignment_for_access_p (dr_peel) the assert doesn't make > any sense (DR_MISALIGNMENT is -1), so yes, you are right. > >> But yeah, that would work too. The idea with the assert in the patch was >> that for unconditional references we probably *do* want to try to compute >> the same compile-time misalignment, but for optimisation reasons rather >> than correctness. Maybe that's more properly a gcc_checking_assert >> though, since nothing goes wrong if it fails. So perhaps: > > We shouldn't have asserts for optimization reasons, even with checking > IMHO. OK. >> gcc_checking_assert (DR_IS_CONDITIONAL_IN_STMT (dr) >> || DR_IS_CONDITIONAL_IN_STMT (dr_peel) >> || (known_alignment_for_access_p (dr) >> == known_alignment_for_access_p (dr_peel))); >> >> as a follow-on assert. >> >> Should I split it into two patches, one to change the gcc_assert and >> another to add the optimisation? > > Yes please. Here's the patch to relax the assert. I'll post the rest in a new thread. Tested as before. OK to install? Thanks, Richard 2017-06-28 Richard Sandiford gcc/ PR tree-optimization/81136 * tree-vect-data-refs.c (vect_update_misalignment_for_peel): Only assert that two references with the same misalignment have the same compile-time misalignment if those compile-time misalignments are known. gcc/testsuite/ PR tree-optimization/81136 * gcc.dg/vect/pr81136.c: New test. Index: gcc/tree-vect-data-refs.c =================================================================== --- gcc/tree-vect-data-refs.c 2017-06-26 19:41:19.549571836 +0100 +++ gcc/tree-vect-data-refs.c 2017-06-28 14:25:58.811888377 +0100 @@ -906,8 +906,10 @@ vect_update_misalignment_for_peel (struc { if (current_dr != dr) continue; - gcc_assert (DR_MISALIGNMENT (dr) / dr_size == - DR_MISALIGNMENT (dr_peel) / dr_peel_size); + gcc_assert (!known_alignment_for_access_p (dr) + || !known_alignment_for_access_p (dr_peel) + || (DR_MISALIGNMENT (dr) / dr_size + == DR_MISALIGNMENT (dr_peel) / dr_peel_size)); SET_DR_MISALIGNMENT (dr, 0); return; } Index: gcc/testsuite/gcc.dg/vect/pr81136.c =================================================================== --- /dev/null 2017-06-28 07:28:02.991792729 +0100 +++ gcc/testsuite/gcc.dg/vect/pr81136.c 2017-06-28 14:25:58.810888422 +0100 @@ -0,0 +1,16 @@ +/* { dg-do compile } */ + +struct __attribute__((aligned (32))) +{ + char misaligner; + int foo[100]; + int bar[100]; +} *a; + +void +fn1 (int n) +{ + int *b = a->foo; + for (int i = 0; i < n; i++) + a->bar[i] = b[i]; +}