From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13825 invoked by alias); 26 Jul 2014 12:08:57 -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 13808 invoked by uid 89); 26 Jul 2014 12:08:56 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 X-HELO: out1-smtp.messagingengine.com Received: from out1-smtp.messagingengine.com (HELO out1-smtp.messagingengine.com) (66.111.4.25) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Sat, 26 Jul 2014 12:08:55 +0000 Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by gateway1.nyi.internal (Postfix) with ESMTP id DB5AB20EC5 for ; Sat, 26 Jul 2014 08:08:43 -0400 (EDT) Received: from frontend1 ([10.202.2.160]) by compute5.internal (MEProxy); Sat, 26 Jul 2014 08:08:43 -0400 Received: from [129.199.97.210] (unknown [129.199.97.210]) by mail.messagingengine.com (Postfix) with ESMTPA id 60DCBC00005; Sat, 26 Jul 2014 08:08:43 -0400 (EDT) Message-ID: <53D39A4A.2000707@grosser.es> Date: Sat, 26 Jul 2014 12:59:00 -0000 From: Tobias Grosser User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.4.0 MIME-Version: 1.0 To: Roman Gareev CC: Mircea Namolaru , gcc-patches@gcc.gnu.org Subject: Re: [GSoC] generation of Gimple code from isl_ast_node_if References: <53D37432.3000107@grosser.es> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2014-07/txt/msg01796.txt.bz2 On 26/07/2014 11:53, Roman Gareev wrote: >> Any reason you don't use isl_id_for_pbb() to create this isl_id? > > Yes, it is redundant. I've used isl_id_for_pbb() in the improved version. > >> Otherwise, the patch looks good to me. You can commit it if the graphite tests still pass and you add an appropriate ChangeLog entry. > > I haven't found out regression during testing with the graphite tests. > >> This patch looks also good. Can you quickly repost with the two test cases as well as the appropriate ChangeLog, before I give the final OK. > > If I'm not mistaken, I sent you only one test case. Should we create > another one? Right, I got confused. I would still add a test case which does not contain a reduction (+=) and where graphite is not duplicating pbbs. If you make res of type float I assume graphite will not detect it as a reduction. On the other side, it may not even introduce if conditions. So you may need a little bit more complicated code e.g: for (i = 0; i < 50; i++) { res += i * 2.1; if (i >= 25) res += i * 3; } This should in the best case yield an isl_ast which matches the input code. Also, please add a comment to the other test case that notes what kind of issue this one is testing (reduction, where the pbbs are possibly duplicated). Cheers, Tobias