From patchwork Tue Oct 1 14:19:11 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Tobias Burnus X-Patchwork-Id: 1169970 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-509985-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="YfS8+icr"; dkim-atps=neutral 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 46jLxj06cHz9sDB for ; Wed, 2 Oct 2019 00:19:35 +1000 (AEST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :subject:to:cc:message-id:date:mime-version:content-type; q=dns; s=default; b=NSoNJAZeLHQ8z2LSyhL+IsIyHvrRng1yCS6ofBOUhbbcl2nRrQ pbu4MkM9+wmA6/3GdmQnnpkUki70BCsRfAvziahhnLuT/PNeHRoEceBtOdhIzv2U 69CEi7vtk5T+MS5+8Y65gIpZWH4AlkzAGvcopmsBVpEUfSn5LQjbyj0pQ= 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:from :subject:to:cc:message-id:date:mime-version:content-type; s= default; bh=uShrp06QWOONdtPMivTpn2Js0zk=; b=YfS8+icru/IsG71B3WzN zVZf1Ua9u1JUeK3bVEmHEhlpYhUscU0o9Q+ahO2nJ06RXR0gw6ULbWdXEXx39oqt XLrWWsVoaZ8SZY12AmySep3CAmObxdL0XpCt3bEbe5a4aYms19GdylyjpldrTKUL g3WK0dCqDSNqsvIK59V3vdE= Received: (qmail 45489 invoked by alias); 1 Oct 2019 14:19:23 -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 45471 invoked by uid 89); 1 Oct 2019 14:19:23 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-21.1 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, SPF_PASS autolearn=ham version=3.3.1 spammy=ups, 10007, H*Ad:U*burnus X-HELO: esa3.mentor.iphmx.com Received: from esa3.mentor.iphmx.com (HELO esa3.mentor.iphmx.com) (68.232.137.180) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 01 Oct 2019 14:19:21 +0000 IronPort-SDR: zKzsf81Pp76OPOYXRlRWf+th2tt02/A8H0lCxHDGJVqcqsjD1dQ7sscMlsKSl1v/S+nR3jDN+f 39yUxsoC7TVCJ6oF2VHoOalQ06YCAk6iInTaHAKCfswN9VHx7IpK1aC7cXfnUKXbIOcDzPKwwd V0WsqvXaAL/RihJhNJb0Bj419WRSlIRGEaQp7FT7gQ2iBe+JHMAdUrgTgSpYPuP5Pvlr4fbj02 uM58di9Jbx7mSxV/cmu/7XCdOucrConT+E4hyLJ9f3vrq+N6yUgp1l1q39bDvVcHvlcPemFntJ eJ4= Received: from orw-gwy-02-in.mentorg.com ([192.94.38.167]) by esa3.mentor.iphmx.com with ESMTP; 01 Oct 2019 06:19:19 -0800 IronPort-SDR: rHRiMdYGxL1fGaAg2TbaGr+bHvwL3P8KFTl5aCghNH5Xk6qumQgj3gqJz4BQMFKMQwZ2MI4EA5 ZaK+UVgDHlADA2ZScmIRfCyz2TjZnE6+Zy/aRcsnlpGwuukarnDFF28q4wVIY682ModV+McxX3 OWWhjyUCDU6uBr4iNgfdEubC0TT4S2pK+SdsSXZp0TF2+lhrSXKfYAOyQFtaOQ7K3NQTfRH2qn o1GuxvPQAoEcmRD2DBV+Wzv8IlpaQK5ERBaV9G4sBPEf/eqRkbrmWPAUYpX32BAbjfDp+ukgv1 kIw= From: Tobias Burnus Subject: [Patch][omp-low.c, fortran] Simple fix for optional argument handling with OpenMP's use_device_ptr To: Jakub Jelinek , Kwok Cheung Yeung , Andrew Stubbs , gcc-patches CC: Tobias Burnus , fortran Message-ID: Date: Tue, 1 Oct 2019 16:19:11 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.1.0 MIME-Version: 1.0 X-IsSubscribed: yes Hi all, [For those who got it twice, I actually forget to include the mailing lists in the first round. Ups.] this patch fixes the bug that with "optional" the wrong pointer is used with "use_device_ptr"; the bug is already observable without doing actual offloading. Namely, "present(ptr)" checks whether the passed argument is != NULL. While using "ptr" – e.g. as "associated(ptr)" – workes the (once) dereferenced dummy argument, which matches the actual argument. The test case is written such that the pointer passed to "use_device_ptr" is present.* Built and regtested on x86_64-gnu-linux without device; I am currently doing a full bootstrap + regtesting and want to test it also with nvptx offloading. Assuming no issue pops up: OK for the trunk? Regarding the patches: * The first tiny patch is mine * The second patch which added the lang_hook omp_is_optional_argument is the one posted at https://gcc.gnu.org/ml/gcc-patches/2019-07/msg01743.html This one was approved by Jakub and I only did two things: (a) re-diff-ed it (trivial as fuzzy worked) (b) I followed both suggestions of Jakub (PARAM_DECL + adding "( )") [Motivation of this patch is – besides fixing an issue – to get the second patch it, which makes it easier to consolidate some other bits and pieces.] Thanks, Tobias * OpenACC (Sec. 2.17) demands that a variable 'arg' in "clauses has no effect at runtime if PRESENT(arg) is .false." – Hence, one needs to go beyond this patch. That's done in the patch series at https://gcc.gnu.org/ml/gcc-patches/2019-07/threads.html#00960 – the patch lang_hook patch of this email is 2/5 of that series. Patch from: https://gcc.gnu.org/ml/gcc-patches/2019-07/msg01743.html Re: [PATCH 2/5, OpenACC] Support Fortran optional arguments in the firstprivate clause Changes: Rediffed, review changes added (added parentheses, PARM_DECL check) gcc/fortran/ * f95-lang.c (LANG_HOOKS_OMP_IS_OPTIONAL_ARGUMENT): Define to gfc_omp_is_optional_argument. * trans-decl.c (create_function_arglist): Set GFC_DECL_OPTIONAL_ARGUMENT in the generated decl if the parameter is optional. * trans-openmp.c (gfc_omp_is_optional_argument): New. (gfc_omp_privatize_by_reference): Return true if the decl is an optional pass-by-reference argument. * trans.h (gfc_omp_is_optional_argument): New declaration. (lang_decl): Add new optional_arg field. (GFC_DECL_OPTIONAL_ARGUMENT): New macro. gcc/ * langhooks-def.h (LANG_HOOKS_OMP_IS_OPTIONAL_ARGUMENT): Default to false. (LANG_HOOKS_DECLS): Add LANG_HOOKS_OMP_IS_OPTIONAL_ARGUMENT. * langhooks.h (omp_is_optional_argument): New hook. * omp-general.c (omp_is_optional_argument): New. * omp-general.h (omp_is_optional_argument): New declaration. * omp-low.c (lower_omp_target): Create temporary for received value and take the address for new_var if the original variable was a DECL_BY_REFERENCE. Use size of referenced object when a pass-by-reference optional argument used as argument to firstprivate. gcc/fortran/f95-lang.c | 2 ++ gcc/fortran/trans-decl.c | 5 +++++ gcc/fortran/trans-openmp.c | 13 +++++++++++++ gcc/fortran/trans.h | 4 ++++ gcc/langhooks-def.h | 2 ++ gcc/langhooks.h | 3 +++ gcc/omp-general.c | 8 ++++++++ gcc/omp-general.h | 1 + gcc/omp-low.c | 3 ++- 9 files changed, 40 insertions(+), 1 deletion(-) diff --git a/gcc/fortran/f95-lang.c b/gcc/fortran/f95-lang.c index 6b9f490d2bb..2467cd968af 100644 --- a/gcc/fortran/f95-lang.c +++ b/gcc/fortran/f95-lang.c @@ -113,6 +113,7 @@ static const struct attribute_spec gfc_attribute_table[] = #undef LANG_HOOKS_TYPE_FOR_MODE #undef LANG_HOOKS_TYPE_FOR_SIZE #undef LANG_HOOKS_INIT_TS +#undef LANG_HOOKS_OMP_IS_OPTIONAL_ARGUMENT #undef LANG_HOOKS_OMP_PRIVATIZE_BY_REFERENCE #undef LANG_HOOKS_OMP_PREDETERMINED_SHARING #undef LANG_HOOKS_OMP_REPORT_DECL @@ -145,6 +146,7 @@ static const struct attribute_spec gfc_attribute_table[] = #define LANG_HOOKS_TYPE_FOR_MODE gfc_type_for_mode #define LANG_HOOKS_TYPE_FOR_SIZE gfc_type_for_size #define LANG_HOOKS_INIT_TS gfc_init_ts +#define LANG_HOOKS_OMP_IS_OPTIONAL_ARGUMENT gfc_omp_is_optional_argument #define LANG_HOOKS_OMP_PRIVATIZE_BY_REFERENCE gfc_omp_privatize_by_reference #define LANG_HOOKS_OMP_PREDETERMINED_SHARING gfc_omp_predetermined_sharing #define LANG_HOOKS_OMP_REPORT_DECL gfc_omp_report_decl diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c index c2c5d9d1b6a..a113f08e26b 100644 --- a/gcc/fortran/trans-decl.c +++ b/gcc/fortran/trans-decl.c @@ -2687,6 +2687,11 @@ create_function_arglist (gfc_symbol * sym) && (!f->sym->attr.proc_pointer && f->sym->attr.flavor != FL_PROCEDURE)) DECL_BY_REFERENCE (parm) = 1; + if (f->sym->attr.optional) + { + gfc_allocate_lang_decl (parm); + GFC_DECL_OPTIONAL_ARGUMENT (parm) = 1; + } gfc_finish_decl (parm); gfc_finish_decl_attrs (parm, &f->sym->attr); diff --git a/gcc/fortran/trans-openmp.c b/gcc/fortran/trans-openmp.c index b4c77aebf4d..88ecc331166 100644 --- a/gcc/fortran/trans-openmp.c +++ b/gcc/fortran/trans-openmp.c @@ -47,6 +47,15 @@ along with GCC; see the file COPYING3. If not see int ompws_flags; +/* True if OpenMP should treat this DECL as an optional argument. */ + +bool +gfc_omp_is_optional_argument (const_tree decl) +{ + return (TREE_CODE (decl) == PARM_DECL && DECL_LANG_SPECIFIC (decl) + && GFC_DECL_OPTIONAL_ARGUMENT (decl)); +} + /* True if OpenMP should privatize what this DECL points to rather than the DECL itself. */ @@ -59,6 +68,10 @@ gfc_omp_privatize_by_reference (const_tree decl) && (!DECL_ARTIFICIAL (decl) || TREE_CODE (decl) == PARM_DECL)) return true; + if (TREE_CODE (type) == POINTER_TYPE + && gfc_omp_is_optional_argument (decl)) + return true; + if (TREE_CODE (type) == POINTER_TYPE) { /* Array POINTER/ALLOCATABLE have aggregate types, all user variables diff --git a/gcc/fortran/trans.h b/gcc/fortran/trans.h index 6ebb71de152..405e88dd1c4 100644 --- a/gcc/fortran/trans.h +++ b/gcc/fortran/trans.h @@ -786,6 +786,7 @@ struct array_descr_info; bool gfc_get_array_descr_info (const_tree, struct array_descr_info *); /* In trans-openmp.c */ +bool gfc_omp_is_optional_argument (const_tree); bool gfc_omp_privatize_by_reference (const_tree); enum omp_clause_default_kind gfc_omp_predetermined_sharing (tree); tree gfc_omp_report_decl (tree); @@ -999,6 +1000,7 @@ struct GTY(()) lang_decl { tree token, caf_offset; unsigned int scalar_allocatable : 1; unsigned int scalar_pointer : 1; + unsigned int optional_arg : 1; }; @@ -1013,6 +1015,8 @@ struct GTY(()) lang_decl { (DECL_LANG_SPECIFIC (node)->scalar_allocatable) #define GFC_DECL_SCALAR_POINTER(node) \ (DECL_LANG_SPECIFIC (node)->scalar_pointer) +#define GFC_DECL_OPTIONAL_ARGUMENT(node) \ + (DECL_LANG_SPECIFIC (node)->optional_arg) #define GFC_DECL_GET_SCALAR_ALLOCATABLE(node) \ (DECL_LANG_SPECIFIC (node) ? GFC_DECL_SCALAR_ALLOCATABLE (node) : 0) #define GFC_DECL_GET_SCALAR_POINTER(node) \ diff --git a/gcc/langhooks-def.h b/gcc/langhooks-def.h index a059841b3df..55d5fe01495 100644 --- a/gcc/langhooks-def.h +++ b/gcc/langhooks-def.h @@ -236,6 +236,7 @@ extern tree lhd_unit_size_without_reusable_padding (tree); #define LANG_HOOKS_WARN_UNUSED_GLOBAL_DECL lhd_warn_unused_global_decl #define LANG_HOOKS_POST_COMPILATION_PARSING_CLEANUPS NULL #define LANG_HOOKS_DECL_OK_FOR_SIBCALL lhd_decl_ok_for_sibcall +#define LANG_HOOKS_OMP_IS_OPTIONAL_ARGUMENT hook_bool_const_tree_false #define LANG_HOOKS_OMP_PRIVATIZE_BY_REFERENCE hook_bool_const_tree_false #define LANG_HOOKS_OMP_PREDETERMINED_SHARING lhd_omp_predetermined_sharing #define LANG_HOOKS_OMP_REPORT_DECL lhd_pass_through_t @@ -261,6 +262,7 @@ extern tree lhd_unit_size_without_reusable_padding (tree); LANG_HOOKS_WARN_UNUSED_GLOBAL_DECL, \ LANG_HOOKS_POST_COMPILATION_PARSING_CLEANUPS, \ LANG_HOOKS_DECL_OK_FOR_SIBCALL, \ + LANG_HOOKS_OMP_IS_OPTIONAL_ARGUMENT, \ LANG_HOOKS_OMP_PRIVATIZE_BY_REFERENCE, \ LANG_HOOKS_OMP_PREDETERMINED_SHARING, \ LANG_HOOKS_OMP_REPORT_DECL, \ diff --git a/gcc/langhooks.h b/gcc/langhooks.h index a45579b3325..9d2714a5b1d 100644 --- a/gcc/langhooks.h +++ b/gcc/langhooks.h @@ -222,6 +222,9 @@ struct lang_hooks_for_decls /* True if this decl may be called via a sibcall. */ bool (*ok_for_sibcall) (const_tree); + /* True if OpenMP should treat DECL as a Fortran optional argument. */ + bool (*omp_is_optional_argument) (const_tree); + /* True if OpenMP should privatize what this DECL points to rather than the DECL itself. */ bool (*omp_privatize_by_reference) (const_tree); diff --git a/gcc/omp-general.c b/gcc/omp-general.c index 66be94f6ff9..5ef6e251698 100644 --- a/gcc/omp-general.c +++ b/gcc/omp-general.c @@ -48,6 +48,14 @@ omp_find_clause (tree clauses, enum omp_clause_code kind) return NULL_TREE; } +/* Return true if DECL is a Fortran optional argument. */ + +bool +omp_is_optional_argument (tree decl) +{ + return lang_hooks.decls.omp_is_optional_argument (decl); +} + /* Return true if DECL is a reference type. */ bool diff --git a/gcc/omp-general.h b/gcc/omp-general.h index 80d42aff3c8..bbaa7b11707 100644 --- a/gcc/omp-general.h +++ b/gcc/omp-general.h @@ -73,6 +73,7 @@ struct omp_for_data #define OACC_FN_ATTRIB "oacc function" extern tree omp_find_clause (tree clauses, enum omp_clause_code kind); +extern bool omp_is_optional_argument (tree decl); extern bool omp_is_reference (tree decl); extern void omp_adjust_for_condition (location_t loc, enum tree_code *cond_code, tree *n2, tree v, tree step); diff --git a/gcc/omp-low.c b/gcc/omp-low.c index 5db182c6841..a0e5041d3f2 100644 --- a/gcc/omp-low.c +++ b/gcc/omp-low.c @@ -11395,7 +11395,8 @@ lower_omp_target (gimple_stmt_iterator *gsi_p, omp_context *ctx) { gcc_assert (is_gimple_omp_oacc (ctx->stmt)); if (omp_is_reference (new_var) - && TREE_CODE (TREE_TYPE (new_var)) != POINTER_TYPE) + && (TREE_CODE (TREE_TYPE (new_var)) != POINTER_TYPE + || DECL_BY_REFERENCE (var))) { /* Create a local object to hold the instance value. */