From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 55386 invoked by alias); 23 Jun 2017 10:51:51 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 54699 invoked by uid 89); 23 Jun 2017 10:51:50 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-vk0-f46.google.com Received: from mail-vk0-f46.google.com (HELO mail-vk0-f46.google.com) (209.85.213.46) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 23 Jun 2017 10:51:48 +0000 Received: by mail-vk0-f46.google.com with SMTP id y70so11777959vky.3 for ; Fri, 23 Jun 2017 03:51:48 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=ddwkvOXhtg/DCB7KcbRxAaYGT9UIXYXHeEKNdm+xuR8=; b=qb4LcnO//Mafa0rynEe1EAxXTbmgZDTh6DPGj6hEBUr1tXyA8z4GUVJ7Ogdf1Jgsr4 a9OVwkrIjrk4ehjnBESj+6imPBX2IIb45tx78rk0bm6DBD8IKZPxwIJeLIu0VXX4Vrs7 YXX1JQr06BSYr0jpmHJPlX8MfsK5CTOd5ihOYSiPWQXaNrXZ5O39nQpl1RPuJH3vQBvX 0aBWUI+YlezZR57hFxPwSOu0u+qOb82lxVx0gUMGnPOIo7UHozzU8jORSVF07cm9a0Os z50NlVfZ8VT3RQf7Rk9Am9sbdesJjGhQPcUeQ6oUzejZpRW/MHVnrfXHcP/lfyi2P3aK XpcA== X-Gm-Message-State: AKS2vOxU1WNiBUWn3jFPfg3yaWHGeBAhW447HBYQrPIfu1hMUMF0dYb5 vtO/jWP7Ay40r4IjqaPIgkz5Qgl3Iw== X-Received: by 10.31.21.205 with SMTP id 196mr2357934vkv.126.1498215106435; Fri, 23 Jun 2017 03:51:46 -0700 (PDT) MIME-Version: 1.0 Received: by 10.103.49.142 with HTTP; Fri, 23 Jun 2017 03:51:45 -0700 (PDT) In-Reply-To: References: From: "Bin.Cheng" Date: Fri, 23 Jun 2017 10:51:00 -0000 Message-ID: Subject: Re: [PATCH GCC][09/13]Simply cost model merges partitions with the same references To: Richard Biener Cc: "gcc-patches@gcc.gnu.org" Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2017-06/txt/msg01767.txt.bz2 On Fri, Jun 23, 2017 at 11:48 AM, Richard Biener wrote: > On Fri, Jun 23, 2017 at 12:19 PM, Bin.Cheng wrote: >> On Mon, Jun 19, 2017 at 4:20 PM, Richard Biener >> wrote: >>> On Mon, Jun 19, 2017 at 3:40 PM, Bin.Cheng wrote: >>>> On Wed, Jun 14, 2017 at 2:54 PM, Richard Biener >>>> wrote: >>>>> On Mon, Jun 12, 2017 at 7:03 PM, Bin Cheng wrote: >>>>>> Hi, >>>>>> Current primitive cost model merges partitions with data references sharing the same >>>>>> base address. I believe it's designed to maximize data reuse in distribution, but >>>>>> that should be done by dedicated data reusing algorithm. At this stage of merging, >>>>>> we should be conservative and only merge partitions with the same references. >>>>>> Bootstrap and test on x86_64 and AArch64. Is it OK? >>>>> >>>>> Well, I'd say "conservative" is merging more, not less. For example >>>>> splitting a[i+1] from a[i] >>>>> would be bad(?), so I'd see to allow unequal DR_INIT as "equal" for >>>>> merging. Maybe >>>>> DR_INIT within a cacheline or so. >>>>> >>>>> How many extra distributions in say SPEC do you get from this change alone? >>>> Hi, >>>> I collected data for spec2006 only with/without this patch. I am a >>>> bit surprised that it doesn't change the number of distributed loops. >>>>> >>>>> It shows also that having partition->reads_and_writes would be nice >>>>> ... the code duplication >>>> Yeah, I merged read/write data references in previous patch, now this >>>> duplication is gone. Update patch attached. Is it OK? >>> >>> + gcc_assert (i < datarefs_vec.length ()); >>> + dr1 = datarefs_vec[i]; >>> >>> these asserts are superfluous -- vec::operator[] does them as well. >>> >>> Ok if you remove them. >> Done. >> I realized I made mistakes when measuring the impact of this patch. >> This patch only apparently causes failure of >> gcc.dg/tree-ssa/ldist-6.c, so here is the updated patch. I also >> collected the number of distributed loops in spec2k6 as below: >> trunk: 5882 >> only this patch: 7130 >> whole patch series: 5237 >> So the conclusion is, this patch does aggressive distribution like >> ldist-6.c, which means worse data-locality. The following patch does >> more fusion which mitigates impact of this patch and results in >> conservative distribution overall. > > What changed in the patch? Did you attach the correct one? No code changed in this one. I just added test case change which can't be resolved by following patches. ldist-6.c slipped away because of a bug in patch: [11/13]Annotate partition by its parallelism execution type > > I'm not sure ldist-6.c is a "valid" testcase but I didn't try to see > where it was reduced from. > >> But as we lack of data locality >> cost model, ldist-6.c remains failed even after applying whole patch >> series. Hmm, a cache-sensitive cost model is need for several passes >> now, distribution, prefetch and (possible) interchange. >> Richard, do you have second comment based on the new data? > > I expected the "only this patch" result somewhat, as said, I'd have > allowed "related" references to fuse by not requiring equal > DR_INIT for example. > > I suggest to go forward with it in its current form. We can tweak the > cost model later. Yeah. > > Thanks, > Richard. > >> Thanks, >> bin >> 2017-06-20 Bin Cheng >> >> * tree-loop-distribution.c (ref_base_address): Delete. >> (similar_memory_accesses): Rename ... >> (share_memory_accesses): ... to this. Check if partitions access >> the same memory reference. >> (distribute_loop): Call share_memory_accesses. >> >> gcc/testsuite/ChangeLog >> 2017-06-20 Bin Cheng >> >> * gcc.dg/tree-ssa/ldist-6.c: XFAIL.