From patchwork Fri Oct 30 11:29:31 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Biener X-Patchwork-Id: 538270 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 A9F0C140D24 for ; Fri, 30 Oct 2015 22:29:46 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=QHUhzlv/; 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 :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; q=dns; s=default; b=svCUVdBrfk9A2lRo0P pg091q0v0pHjDEEifK2QETcirb2OzT7LLSAe8J8an4xLNQTYLiBqQN1t+YEpFBx7 nLsDL30BkQbIN6w2E0xg6/GUST4XDw4OICemEvn36quspgsp2njLM2OqERs8kHPW v0Be8JZ1SE3b7uMIpZo9QC6Fg= 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 :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; s=default; bh=xzAOf7qUIlzKd7KZ3yHYFl+H sEw=; b=QHUhzlv/izGnmpMXDqCAtbl7U/kZzPpU7C8ugmYD55SS140LWwrOID/X CP3n9KVHqzpTLDEcDuU0QLEEA1u3CWFRm1DZLJEO7IA70rId5IjUO32zs1HQpBZj yUOn4auCBwtUiSce6/e3hp0ZTLqYE+iHquI/bV24XAv1v9LB1MQ= Received: (qmail 78258 invoked by alias); 30 Oct 2015 11:29:36 -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 78246 invoked by uid 89); 30 Oct 2015 11:29:35 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-yk0-f181.google.com Received: from mail-yk0-f181.google.com (HELO mail-yk0-f181.google.com) (209.85.160.181) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Fri, 30 Oct 2015 11:29:33 +0000 Received: by ykft191 with SMTP id t191so72126560ykf.0 for ; Fri, 30 Oct 2015 04:29:31 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.129.125.194 with SMTP id y185mr5587404ywc.5.1446204571116; Fri, 30 Oct 2015 04:29:31 -0700 (PDT) Received: by 10.37.117.136 with HTTP; Fri, 30 Oct 2015 04:29:31 -0700 (PDT) In-Reply-To: References: Date: Fri, 30 Oct 2015 12:29:31 +0100 Message-ID: Subject: Re: [RFC] [Patch] Relax tree-if-conv.c trap assumptions. From: Richard Biener To: "Kumar, Venkataramanan" Cc: Andrew Pinski , "gcc-patches@gcc.gnu.org" X-IsSubscribed: yes On Fri, Oct 30, 2015 at 11:21 AM, Kumar, Venkataramanan wrote: > Hi Andrew, > >> -----Original Message----- >> From: Andrew Pinski [mailto:pinskia@gmail.com] >> Sent: Friday, October 30, 2015 3:38 PM >> To: Kumar, Venkataramanan >> Cc: Richard Beiner (richard.guenther@gmail.com); gcc-patches@gcc.gnu.org >> Subject: Re: [RFC] [Patch] Relax tree-if-conv.c trap assumptions. >> >> On Fri, Oct 30, 2015 at 6:06 PM, Kumar, Venkataramanan >> wrote: >> > Hi Richard, >> > >> > I am trying to "if covert the store" in the below test case and later >> > help it to get vectorized under -Ofast -ftree-loop-if-convert-stores >> > -fno-common >> > >> > #define LEN 4096 >> > __attribute__((aligned(32))) float array[LEN]; void test() { for (int i = 0; i < >> LEN; i++) { >> > if (array[i] > (float)0.) >> > array[i] =3 ; >> > >> > } >> > } >> > >> > Currently in GCC 5.2 does not vectorize it. >> > https://goo.gl/9nS6Pd >> > >> > However ICC seems to vectorize it >> > https://goo.gl/y1yGHx >> > >> > As discussed with you earlier, to allow "if convert store" I am checking the >> following: >> > >> > (1) We already read the reference "array[i]" unconditionally once . >> > (2) I am now checking if we are conditionally writing to memory which is >> defined as read and write and is bound to the definition we are seeing. >> >> >> I don't think this is thread safe .... >> > > I thought under -ftree-loop-if-convert-stores it is ok to do this transformation. Yes, that's what we have done in the past here. Note that I think we should remove the flag in favor of the --param allow-store-data-races and if-convert safe stores always (stores to thread-local memory). Esp. using masked stores should be always safe. > Regards, > Venkat. > >> Thanks, >> Andrew >> >> > >> > With this change, I get able to if convert and the vectorize the case also. >> > >> > /build/gcc/xgcc -B ./build/gcc/ ifconv.c -Ofast -fopt-info-vec -S >> > -ftree-loop-if-convert-stores -fno-common >> > ifconv.c:2:63: note: loop vectorized >> > >> > Patch >> > ------ >> > diff --git a/gcc/tree-if-conv.c b/gcc/tree-if-conv.c index >> > f201ab5..6475cc0 100644 >> > --- a/gcc/tree-if-conv.c >> > +++ b/gcc/tree-if-conv.c >> > @@ -727,6 +727,34 @@ write_memrefs_written_at_least_once (gimple >> *stmt, >> > return true; >> > } >> > >> > +static bool >> > +write_memrefs_safe_to_access_unconditionally (gimple *stmt, >> > + >> > +vec drs) { { to the next line The function has a bad name it should be write_memrefs_writable () >> > + int i; >> > + data_reference_p a; >> > + bool found = false; >> > + >> > + for (i = 0; drs.iterate (i, &a); i++) >> > + { >> > + if (DR_STMT (a) == stmt >> > + && DR_IS_WRITE (a) >> > + && (DR_WRITTEN_AT_LEAST_ONCE (a) == 0) >> > + && (DR_RW_UNCONDITIONALLY (a) == 1)) >> > + { >> > + tree base = get_base_address (DR_REF (a)); >> > + found = false; >> > + if (DECL_P (base) >> > + && decl_binds_to_current_def_p (base) >> > + && !TREE_READONLY (base)) >> > + { >> > + found = true; So if the vector ever would contain more than one write you'd return true if only one of them is not readonly. IMHO all the routines need refactoring to operate on single DRs which AFAIK is the only case if-conversion handles anyway (can't if-convert calls or aggregate assignments or asms). Ugh, it seems the whole thing is quadratic, doing linear walks to find the DRs for a stmt ... A simple would improve that tremendously ... (well, given the array of DRs is sorted by stmt which is an implementation detail not documented). Computing all the DR flags in separate loops also doesn't make much sense to me and just complicates code. Richard. >> > + } >> > + } >> > + } >> > + return found; >> > +} >> > + >> > /* Return true when the memory references of STMT won't trap in the >> > if-converted code. There are two things that we have to check for: >> > >> > @@ -748,8 +776,20 @@ write_memrefs_written_at_least_once (gimple >> > *stmt, static bool ifcvt_memrefs_wont_trap (gimple *stmt, >> > vec refs) { >> > - return write_memrefs_written_at_least_once (stmt, refs) >> > - && memrefs_read_or_written_unconditionally (stmt, refs); >> > + bool memrefs_written_once, memrefs_read_written_unconditionally; >> > + bool memrefs_safe_to_access; >> > + >> > + memrefs_written_once >> > + = write_memrefs_written_at_least_once (stmt, refs); >> > + >> > + memrefs_read_written_unconditionally >> > + = memrefs_read_or_written_unconditionally (stmt, refs); >> > + >> > + memrefs_safe_to_access >> > + = write_memrefs_safe_to_access_unconditionally (stmt, >> > + refs); >> > + >> > + return ((memrefs_written_once || memrefs_safe_to_access) >> > + && memrefs_read_written_unconditionally); >> > } >> > >> > /* Wrapper around gimple_could_trap_p refined for the needs of the >> > >> > >> > do I need this function write_memrefs_written_at_least_once anymore? >> > Please suggest if there a better way to do this. >> > >> > Bootstrapped and regression tested on x86_64. >> > I am not adding change log and comments now, as I wanted to check >> approach first. >> > >> > Regards, >> > Venkat. >> > >> > Index: gcc/tree-if-conv.c =================================================================== --- gcc/tree-if-conv.c (revision 229572) +++ gcc/tree-if-conv.c (working copy) @@ -612,9 +612,10 @@ memrefs_read_or_written_unconditionally data_reference_p a, b; tree ca = bb_predicate (gimple_bb (stmt)); - for (i = 0; drs.iterate (i, &a); i++) - if (DR_STMT (a) == stmt) - { + for (i = gimple_uid (stmt) - 1; drs.iterate (i, &a); i++) + { + if (DR_STMT (a) != stmt) + break; bool found = false; int x = DR_RW_UNCONDITIONALLY (a); @@ -684,10 +685,13 @@ write_memrefs_written_at_least_once (gim data_reference_p a, b; tree ca = bb_predicate (gimple_bb (stmt)); - for (i = 0; drs.iterate (i, &a); i++) - if (DR_STMT (a) == stmt - && DR_IS_WRITE (a)) - { + for (i = gimple_uid (stmt) - 1; drs.iterate (i, &a); i++) + { + if (DR_STMT (a) != stmt) + break; + if (! DR_IS_WRITE (a)) + continue; + bool found = false; int x = DR_WRITTEN_AT_LEAST_ONCE (a); @@ -721,7 +725,7 @@ write_memrefs_written_at_least_once (gim DR_WRITTEN_AT_LEAST_ONCE (a) = 0; return false; } - } + } return true; } @@ -1291,6 +1297,7 @@ if_convertible_loop_p_1 (struct loop *lo case GIMPLE_CALL: case GIMPLE_DEBUG: case GIMPLE_COND: + gimple_set_uid (gsi_stmt (gsi), 0); break; default: return false; @@ -1304,6 +1311,8 @@ if_convertible_loop_p_1 (struct loop *lo dr->aux = XNEW (struct ifc_dr); DR_WRITTEN_AT_LEAST_ONCE (dr) = -1; DR_RW_UNCONDITIONALLY (dr) = -1; + if (gimple_uid (DR_STMT (dr)) == 0) + gimple_set_uid (DR_STMT (dr), i + 1); } predicate_bbs (loop);