From patchwork Mon Jan 2 22:10:39 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 133921 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]) by ozlabs.org (Postfix) with SMTP id 6D02CB6FA8 for ; Tue, 3 Jan 2012 09:11:01 +1100 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1326147062; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Received:Received:Date:From:To:Cc:Subject:Message-ID:Reply-To: MIME-Version:Content-Type:Content-Disposition:User-Agent: Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:Sender:Delivered-To; bh=UIHchuVvKyhnbK5tQwLg E3xeyVs=; b=c6vRz4b81Gt2jFDfO4L3VKD6SJ9IPu2NrOBJcTe7DqV/rK+Ru97l ha1AZ62aHmVeJ2OixZ2mNG7mEZhyuCVJS8Tp/Tx2c1b98K1ynIKid1TXTKBwoHQv uTAbgV12NW+zr/hVj6xpBu5XRTcCc2JGUcZr7lXJu3EDQ784KXBwUL8= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:Received:Received:Received:Date:From:To:Cc:Subject:Message-ID:Reply-To:MIME-Version:Content-Type:Content-Disposition:User-Agent:X-IsSubscribed:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=Kp2v+NU+BixtvyjmS35fUdA8yFyKjDkz75V+f5Ttr0SJqZhN0LFCOILHIXPHpF kVGdLXxx9/FEvpXPt7QcDmkE811iftL91h+wJO2pbsFAZ5+t6JTr5iE9HsqNr8Sg F4WXIfTWLONAzQ+Qijyjh8Q/7A+b3Oik4PWjVN65QoAjA=; Received: (qmail 5594 invoked by alias); 2 Jan 2012 22:10:58 -0000 Received: (qmail 5584 invoked by uid 22791); 2 Jan 2012 22:10:55 -0000 X-SWARE-Spam-Status: No, hits=-7.3 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, SPF_HELO_PASS, TW_TM X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 02 Jan 2012 22:10:41 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q02MAfld001192 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 2 Jan 2012 17:10:41 -0500 Received: from tyan-ft48-01.lab.bos.redhat.com (tyan-ft48-01.lab.bos.redhat.com [10.16.42.4]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q02MAedE004923 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 2 Jan 2012 17:10:41 -0500 Received: from tyan-ft48-01.lab.bos.redhat.com (tyan-ft48-01.lab.bos.redhat.com [127.0.0.1]) by tyan-ft48-01.lab.bos.redhat.com (8.14.4/8.14.4) with ESMTP id q02MAeV7019649; Mon, 2 Jan 2012 23:10:40 +0100 Received: (from jakub@localhost) by tyan-ft48-01.lab.bos.redhat.com (8.14.4/8.14.4/Submit) id q02MAdk7019647; Mon, 2 Jan 2012 23:10:39 +0100 Date: Mon, 2 Jan 2012 23:10:39 +0100 From: Jakub Jelinek To: Jason Merrill , Richard Henderson Cc: gcc-patches@gcc.gnu.org Subject: [C++ PATCH] Add CLEANUP_POINT_EXPRs around OMP_PARALLEL/TASK/FOR if needed (PR c++/51669) Message-ID: <20120102221039.GS18937@tyan-ft48-01.lab.bos.redhat.com> Reply-To: Jakub Jelinek MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes 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 Hi! The f1 function in the testcase fails newly starting with http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=181332 because there is no CLEANUP_POINT_EXPR around OMP_PARALLEL/OMP_TASK/OMP_FOR whose IF/FINAL/NUM_THREADS/SCHEDULE clause expression might need some cleanups. But as the testcase shows, it isn't hard to write a testcase where even 4.6 or 4.2 ICEs too. It isn't clear where to put the CLEANUP_POINT_EXPR though, OpenMP 3.1 says just (e.g. for parallel): "The if clause expression and the num_threads clause expression are evaluated in the context outside of the parallel construct, and no ordering of those evaluations is specified. It is also unspecified whether, in what order, or how many times any side-effects of the evaluation of the num_threads or if clause expressions occur." Attached are two different patches, the first one puts the CLEANUP_POINT_EXPR around the whole OMP_PARALLEL etc. stmt, the second one wraps the individual clause expressions into CLEANUP_POINT_EXPR. Both patches have been bootstrapped/regtested on x86_64-linux and i686-linux, which one is preferrable? Jakub 2012-01-02 Jakub Jelinek PR c++/51669 * tree.h (find_omp_clause): New prototype. * tree-flow.h (find_omp_clause): Remove prototype. * c-parser.c (c_parser_omp_for_loop): Call add_stmt if c_finish_omp_for returned non-NULL. c-family/ * c-omp.c (c_parser_omp_for_loop): Don't call add_stmt here. cp/ * semantics.c (finish_omp_parallel): If there is IF or NUM_THREADS clause, call add_stmt on maybe_cleanup_point_expr result. (finish_omp_task): If there is IF or FINAL clause, call add_stmt on maybe_cleanup_point_expr result. (finish_omp_for): If there is SCHEDULE clause, call maybe_cleanup_point_expr. Call add_stmt if omp_for is non-NULL. testsuite/ * g++.dg/gomp/pr51669.C: New test. 2012-01-02 Jakub Jelinek PR c++/51669 * semantics.c (finish_omp_clauses): Call fold_build_cleanup_point_expr on OMP_CLAUSE_{IF,FINAL,NUM_THREADS,SCHEDULE_CHUNK}_EXPR. * g++.dg/gomp/pr51669.C: New test. --- gcc/cp/semantics.c.jj 2012-01-02 20:39:59.000000000 +0100 +++ gcc/cp/semantics.c 2012-01-02 20:45:30.420357459 +0100 @@ -4067,6 +4067,8 @@ finish_omp_clauses (tree clauses) t = maybe_convert_cond (t); if (t == error_mark_node) remove = true; + else if (!processing_template_decl) + t = fold_build_cleanup_point_expr (TREE_TYPE (t), t); OMP_CLAUSE_IF_EXPR (c) = t; break; @@ -4075,6 +4077,8 @@ finish_omp_clauses (tree clauses) t = maybe_convert_cond (t); if (t == error_mark_node) remove = true; + else if (!processing_template_decl) + t = fold_build_cleanup_point_expr (TREE_TYPE (t), t); OMP_CLAUSE_FINAL_EXPR (c) = t; break; @@ -4089,7 +4093,12 @@ finish_omp_clauses (tree clauses) remove = true; } else - OMP_CLAUSE_NUM_THREADS_EXPR (c) = mark_rvalue_use (t); + { + t = mark_rvalue_use (t); + if (!processing_template_decl) + t = fold_build_cleanup_point_expr (TREE_TYPE (t), t); + OMP_CLAUSE_NUM_THREADS_EXPR (c) = t; + } break; case OMP_CLAUSE_SCHEDULE: @@ -4105,7 +4114,12 @@ finish_omp_clauses (tree clauses) remove = true; } else - OMP_CLAUSE_SCHEDULE_CHUNK_EXPR (c) = mark_rvalue_use (t); + { + t = mark_rvalue_use (t); + if (!processing_template_decl) + t = fold_build_cleanup_point_expr (TREE_TYPE (t), t); + OMP_CLAUSE_SCHEDULE_CHUNK_EXPR (c) = t; + } break; case OMP_CLAUSE_NOWAIT: --- gcc/testsuite/g++.dg/gomp/pr51669.C.jj 2012-01-02 20:41:57.320572664 +0100 +++ gcc/testsuite/g++.dg/gomp/pr51669.C 2012-01-02 20:41:57.320572664 +0100 @@ -0,0 +1,32 @@ +// PR c++/51669 +// { dg-do compile } +// { dg-options "-fopenmp" } + +template const T & min (const T &, const T &); + +void +f1 () +{ +#pragma omp parallel num_threads (min (4, 5)) + ; +} + +struct A { A (); ~A (); }; +int foo (const A &); + +void +f2 () +{ + int i; +#pragma omp parallel if (foo (A ())) num_threads (foo (A ())) + ; +#pragma omp task if (foo (A ())) final (foo (A ())) + ; +#pragma omp for schedule (static, foo (A ())) + for (i = 0; i < 10; i++) + ; +#pragma omp parallel for schedule (static, foo (A ())) \ + if (foo (A ())) num_threads (foo (A ())) + for (i = 0; i < 10; i++) + ; +} --- gcc/tree.h.jj 2011-12-16 08:37:45.000000000 +0100 +++ gcc/tree.h 2012-01-02 16:59:18.037923291 +0100 @@ -5866,6 +5866,9 @@ extern unsigned HOST_WIDE_INT compute_bu extern unsigned HOST_WIDE_INT highest_pow2_factor (const_tree); extern tree build_personality_function (const char *); +/* In omp-low.c. */ +extern tree find_omp_clause (tree, enum omp_clause_code); + /* In trans-mem.c. */ extern tree build_tm_abort_call (location_t, bool); extern bool is_tm_safe (const_tree); --- gcc/tree-flow.h.jj 2011-11-29 08:58:52.000000000 +0100 +++ gcc/tree-flow.h 2012-01-02 16:57:46.538512564 +0100 @@ -392,7 +392,6 @@ extern struct omp_region *new_omp_region struct omp_region *); extern void free_omp_regions (void); void omp_expand_local (basic_block); -extern tree find_omp_clause (tree, enum omp_clause_code); tree copy_var_decl (tree, tree, tree); /*--------------------------------------------------------------------------- --- gcc/c-parser.c.jj 2011-12-21 08:43:48.000000000 +0100 +++ gcc/c-parser.c 2012-01-02 17:13:17.187988709 +0100 @@ -10082,6 +10082,7 @@ c_parser_omp_for_loop (location_t loc, stmt = c_finish_omp_for (loc, declv, initv, condv, incrv, body, NULL); if (stmt) { + add_stmt (stmt); if (par_clauses != NULL) { tree *c; --- gcc/c-family/c-omp.c.jj 2011-10-12 20:28:08.000000000 +0200 +++ gcc/c-family/c-omp.c 2012-01-02 17:12:54.706121214 +0100 @@ -576,7 +576,7 @@ c_finish_omp_for (location_t locus, tree OMP_FOR_PRE_BODY (t) = pre_body; SET_EXPR_LOCATION (t, locus); - return add_stmt (t); + return t; } } --- gcc/cp/semantics.c.jj 2012-01-01 19:54:46.000000000 +0100 +++ gcc/cp/semantics.c 2012-01-02 17:16:08.640980769 +0100 @@ -4384,7 +4384,7 @@ begin_omp_parallel (void) tree finish_omp_parallel (tree clauses, tree body) { - tree stmt; + tree stmt, ret; body = finish_omp_structured_block (body); @@ -4392,8 +4392,13 @@ finish_omp_parallel (tree clauses, tree TREE_TYPE (stmt) = void_type_node; OMP_PARALLEL_CLAUSES (stmt) = clauses; OMP_PARALLEL_BODY (stmt) = body; + ret = stmt; + if (find_omp_clause (clauses, OMP_CLAUSE_IF) + || find_omp_clause (clauses, OMP_CLAUSE_NUM_THREADS)) + stmt = maybe_cleanup_point_expr (stmt); + add_stmt (stmt); - return add_stmt (stmt); + return ret; } tree @@ -4406,7 +4411,7 @@ begin_omp_task (void) tree finish_omp_task (tree clauses, tree body) { - tree stmt; + tree stmt, ret; body = finish_omp_structured_block (body); @@ -4414,8 +4419,13 @@ finish_omp_task (tree clauses, tree body TREE_TYPE (stmt) = void_type_node; OMP_TASK_CLAUSES (stmt) = clauses; OMP_TASK_BODY (stmt) = body; + ret = stmt; + if (find_omp_clause (clauses, OMP_CLAUSE_IF) + || find_omp_clause (clauses, OMP_CLAUSE_FINAL)) + stmt = maybe_cleanup_point_expr (stmt); + add_stmt (stmt); - return add_stmt (stmt); + return ret; } /* Helper function for finish_omp_for. Convert Ith random access iterator @@ -4876,7 +4886,13 @@ finish_omp_for (location_t locus, tree d TREE_VEC_ELT (OMP_FOR_INCR (omp_for), i) = TREE_VEC_ELT (orig_incr, i); } if (omp_for != NULL) - OMP_FOR_CLAUSES (omp_for) = clauses; + { + tree stmt = omp_for; + OMP_FOR_CLAUSES (omp_for) = clauses; + if (find_omp_clause (clauses, OMP_CLAUSE_SCHEDULE)) + stmt = maybe_cleanup_point_expr (stmt); + add_stmt (stmt); + } return omp_for; } --- gcc/testsuite/g++.dg/gomp/pr51669.C.jj 2012-01-02 17:18:47.268031834 +0100 +++ gcc/testsuite/g++.dg/gomp/pr51669.C 2012-01-02 17:17:05.000000000 +0100 @@ -0,0 +1,32 @@ +// PR c++/51669 +// { dg-do compile } +// { dg-options "-fopenmp" } + +template const T & min (const T &, const T &); + +void +f1 () +{ +#pragma omp parallel num_threads (min (4, 5)) + ; +} + +struct A { A (); ~A (); }; +int foo (const A &); + +void +f2 () +{ + int i; +#pragma omp parallel if (foo (A ())) num_threads (foo (A ())) + ; +#pragma omp task if (foo (A ())) final (foo (A ())) + ; +#pragma omp for schedule (static, foo (A ())) + for (i = 0; i < 10; i++) + ; +#pragma omp parallel for schedule (static, foo (A ())) \ + if (foo (A ())) num_threads (foo (A ())) + for (i = 0; i < 10; i++) + ; +}