From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12698 invoked by alias); 21 Mar 2013 12:55:25 -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 12683 invoked by uid 89); 21 Mar 2013 12:55:18 -0000 X-Spam-SWARE-Status: No, score=-8.0 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.1 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Thu, 21 Mar 2013 12:55:15 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r2LCtCcK016745 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 21 Mar 2013 08:55:12 -0400 Received: from houston.quesejoda.com (vpn-51-63.rdu2.redhat.com [10.10.51.63]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r2LCtBMZ031117; Thu, 21 Mar 2013 08:55:11 -0400 Message-ID: <514B032F.30708@redhat.com> Date: Thu, 21 Mar 2013 12:55:00 -0000 From: Aldy Hernandez User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130311 Thunderbird/17.0.4 MIME-Version: 1.0 To: "Iyer, Balaji V" CC: "Joseph S. Myers" , gcc-patches Subject: Re: [patch] cilkplus array notation for C (clean, independent patchset, take 1) References: <5149D62F.9070503@redhat.com> <5149E4C7.1090206@redhat.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2013-03/txt/msg00801.txt.bz2 >> All these builtins need to be documented in doc/. > > DONE! > +initialize builtin functions are stored in @file{array-notation-common.c}. In > +the current array notation implementation there are 12 builtin reduction > +operations. Details about these functions and their usage are available in > +the Cilk Plus language specification at @w{@uref{http://www.cilkplus.org}}. > + > +@itemize @bullet > +@item __sec_reduce_add > +@item __sec_reduce_mul > +@item __sec_reduce_max > +@item __sec_reduce_min > +@item __sec_reduce_max_ind First, the common documentation idiom is "built-in" not builtin. Second, these should be documented in extend.texi along with the rest of the builtins listed there. >> As I'd mentioned, you have .exp files named compile.exp and execute.exp which >> seem to be causing ambiguity problems in parallel checks (make check -jN). For >> some reason, with this patch, the rest of dg.exp fails to run after Cilkplus' >> compile/execute.exp runs. Renaming these to something less generic does the >> trick. Do you mind prefixing all the .exp's with "cilkplus_" or something similar? > > FIXED! I added the "cilkplus_AN_c_" prefix to everything, where "an" stands for Array Notation . This way it won't interfere with the future Cilk Plus patches' test case. I think eventually all the compile tests for all of cilkplus should go in one directory and one for the execute tests. There is no need for a separate directory for the array notation, etc, etc. For that matter, we should probably munge everything into one dg style directory. But this is ok for now. > --- a/gcc/testsuite/gcc.dg/cilk-plus/array_notation/compile/array_test2.c > +++ b/gcc/testsuite/gcc.dg/cilk-plus/array_notation/compile/array_test2.c > @@ -26,7 +26,7 @@ int main(int argc, char **argv) > array[ii] = 10; > array2[ii] = 5000000; > } > - array2[0:10:2] = array[0:10:2]; > + array2[0:5:2] = array[0:5:2]; What is this non-related change? >> If these are mere syntax error tests, I suggest you get rid of all the optimization >> variants. Surely there is no need to test the error at -O, then at -O2, etc. -O0 >> should suffice. For that matter, you shouldn't pass any optimization flag and let >> the test itself set the flags with "// dg-options" or whatever in the test itself (if >> for some reason you have a test that has one particular error only reportable at >> some optimization level). > > I did this purely as a safety measure for the commonly used flags that I know of. If it won't cause too much of a problem I would like to keep it. All these errors are in the front-end, so optimization shouldn't matter. It just seems like over kill to run 750 tests for just 15 individual .c files. We can leave this for now. > >> >> And please make sure there are no regressions on the branch when checking >> with "make check -k -jN" (N > 1) and for a plain serial check-- at least while we >> iron out this dejagnu setup. > > It works for make -j8 check on my 8 core machine. I assume "make -j1" still works and has no regressions? Can you repost an updated (and tested) patch? Thanks.