From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 120821 invoked by alias); 16 Jun 2017 17:15:53 -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 120774 invoked by uid 89); 16 Jun 2017 17:15:53 -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-f48.google.com Received: from mail-vk0-f48.google.com (HELO mail-vk0-f48.google.com) (209.85.213.48) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 16 Jun 2017 17:15:50 +0000 Received: by mail-vk0-f48.google.com with SMTP id p62so25687415vkp.0 for ; Fri, 16 Jun 2017 10:15:55 -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=jqsPfKZ+DYb0vXAilR+ghSpXEOUby67UbNtboZJ4nOA=; b=OOqIH/G2YCEsRvPNQHnFZM3OB9CxvqpNhGllAOn8eNZjwxtg8DX/WPKALC7lthbTNq ngo36ZyBXIjxbBNs/3mDm0+43Ilks+s8ux+xvUSvbp+M6ymvEu0d0lqU8GYD7KJPake3 lmjnW0XC+SXCMX9Ip2mItS4zbjCRy8pWhDw27Bf28TKa2+b8ysWepNuz4RzO+Ugi/GbB MDchov+85EnMmu438Ns8w2Om5m2Y63TwUQ7yTEBYTCX+0VMhMU5qskeGSp5H6H1kkQf4 fM5nZJLLiaRSohQx2jDr6nF4krlO0RF9mTd7tX9uqN0RiaRx4DX5fEoyGIagFgccWJ7E 2kag== X-Gm-Message-State: AKS2vOybpNP9v8HATuIQ+QnC1Nmh36D1Amuwtw3z4RhkkiZsZ4il/Lp4 rbDJu3MNvK90Te+WIKLEBonzib/6cQ== X-Received: by 10.31.21.205 with SMTP id 196mr1854014vkv.126.1497633353571; Fri, 16 Jun 2017 10:15:53 -0700 (PDT) MIME-Version: 1.0 Received: by 10.103.47.19 with HTTP; Fri, 16 Jun 2017 10:15:53 -0700 (PDT) In-Reply-To: References: From: "Bin.Cheng" Date: Fri, 16 Jun 2017 17:15:00 -0000 Message-ID: Subject: Re: [PATCH GCC][12/13]Workaround reduction statements for distribution 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/msg01223.txt.bz2 On Fri, Jun 16, 2017 at 11:21 AM, Richard Biener wrote: > On Mon, Jun 12, 2017 at 7:03 PM, Bin Cheng wrote: >> Hi, >> For now, loop distribution handles variables used outside of loop as reduction. >> This is inaccurate because all partitions contain statement defining induction >> vars. > > But final induction values are usually not used outside of the loop... This is in actuality for induction variable which is used outside of the loop. > > What is missing is loop distribution trying to change partition order. In fact > we somehow assume we can move a reduction across a detected builtin > (I don't remember if we ever check for validity of that...). Hmm, I am not sure when we can't. If there is any dependence between builtin/reduction partitions, it should be captured by RDG or PG, otherwise the partitions are independent and can be freely ordered as long as reduction partition is scheduled last? > >> Ideally we should factor out scev-propagation as a standalone interface >> which can be called when necessary. Before that, this patch simply workarounds >> reduction issue by checking if the statement belongs to all partitions. If yes, >> the reduction must be computed in the last partition no matter how the loop is >> distributed. >> Bootstrap and test on x86_64 and AArch64. Is it OK? > > stmt_in_all_partitions is not kept up-to-date during partition merging and if > merging makes the reduction partition(s) pass the stmt_in_all_partitions > test your simple workaround doesn't work ... I think it doesn't matter because: A) it's really workaround for induction variables. In general, induction variables are included by all partition. B) After classify partition, we immediately fuses all reduction partitions. More stmt_in_all_partitions means we are fusing non-reduction partition with reduction partition, so the newly generated (stmt_in_all_partitions) are actually not reduction statements. The workaround won't work anyway even the bitmap is maintained. > > As written it's a valid optimization but can you please note it's limitation in > some comment please? Yeah, I will add comment explaining it. Thanks, bin > > Also... > > + bitmap_set_range (stmt_in_all_partitions, 0, rdg->n_vertices); > + rdg_build_partitions (rdg, stmts, &partitions, stmt_in_all_partitions); > > ick. Please instead do > > bitmap_copy (smtt_in_all_partitions, partitions[0]->stmts); > for (i = 1; i < ...) > bitmap_and_into (stmt_in_all_partitons, partitions[i]->stmts); > > Thanks, > Richard. > >> Thanks, >> bin >> 2017-06-07 Bin Cheng >> >> * tree-loop-distribution.c (classify_partition): New parameter and >> better handle reduction statement. >> (rdg_build_partitions): New parameter and record statements belonging >> to all partitions. >> (distribute_loop): Update use of above functions.