From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from esa2.mentor.iphmx.com (esa2.mentor.iphmx.com [68.232.141.98]) by sourceware.org (Postfix) with ESMTPS id 321BD385800F for ; Fri, 27 Nov 2020 21:55:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 321BD385800F Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=Kwok_Yeung@mentor.com IronPort-SDR: Fr758F7cK2GokgI/QiQfbKMXBmQ4cbab0Y1sk4ovx4c77zYx4x3OmXXO0sTW3TXo8+KSsXPeGf SeARGK0UwoF2Es1eJnz2PN0138U8q4JosEFxqWBxpRW5l1Apk+HCNyFMYm70Dp1fXNkF3xMDRU 9mUac/INxVbkuEMybgiK8uDMzWSrKDQEnJNTvsEELaqfDmekQS2FuX5Vc7MXnaqDTFKE+l3DCb ENGutzNw+/Z0KsMcPl/Et7TLHLwTFhUPi21SqhFuAMxG9FFjOwo8uAOT9dQNp+eA0TtE05RENz 1vc= X-IronPort-AV: E=Sophos;i="5.78,375,1599552000"; d="scan'208";a="55568583" Received: from orw-gwy-02-in.mentorg.com ([192.94.38.167]) by esa2.mentor.iphmx.com with ESMTP; 27 Nov 2020 13:55:46 -0800 IronPort-SDR: SB002fyO+H9HaBRN12XfXYYGnS0cDlV+qmC40/36FfhvkkHlS8CsmCSS4YGFKRTroOVWbX7Sqo 6COHqpwdtS1bSX0XQh7Rf6NkIdNfQEa3pXYRn9lTFJkMjhemILUMXQ7feRTNZOID8TR/5EZ10j gQ66ya6rGdqn6w8DG5qPhipveHfIOwaZggkZB6ySELv3Hc56Mf+lu2MBkx56NlPkZEIMPD6FNF z5oAa6heSNrD+AhmwMgPMdEextPh9VcPneGVv6Xra76ZX6j+OEJHpeaZNN/K9LKzs1t8YRnMjp 2cg= Subject: PING Re: [PATCH] openmp: Implicit 'declare target' for C++ static initializers From: Kwok Cheung Yeung To: Jakub Jelinek CC: GCC Patches References: <20201029100311.GC3788@tucnak> Message-ID: <5d69b8a4-050b-2b35-fed9-3407b1e70e8d@codesourcery.com> Date: Fri, 27 Nov 2020 21:55:10 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.5.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 8bit X-Originating-IP: [137.202.0.90] X-ClientProxiedBy: svr-ies-mbx-02.mgc.mentorg.com (139.181.222.2) To SVR-IES-MBX-04.mgc.mentorg.com (139.181.222.4) X-Spam-Status: No, score=-5.1 required=5.0 tests=BAYES_00, BODY_8BITS, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=no autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 27 Nov 2020 21:55:49 -0000 Hello This patch still needs review. Thanks Kwok On 19/11/2020 6:07 pm, Kwok Cheung Yeung wrote: > On 29/10/2020 10:03 am, Jakub Jelinek wrote: >> I'm actually not sure how this can work correctly. >> Let's say we have >> int foo () { return 1; } >> int bar () { return 2; } >> int baz () { return 3; } >> int qux () { return 4; } >> int a = foo (); >> int b = bar (); >> int c = baz (); >> int *d = &c; >> int e = qux (); >> int f = e + 1; >> int *g = &f; >> #pragma omp declare target to (b, d, g) >> So, for the implicit declare target discovery, a is not declare target to, >> nor is foo, and everything else is; b, d, g explicitly, c because it is >> referenced in initializer of b, f because it is mentioned in initializer of >> g and e because it is mentioned in initializer of f. >> Haven't checked if the new function you've added is called before or after >> analyze_function calls omp_discover_implicit_declare_target, but I don't >> really see how it can work when it is not inside of that function, so that >> discovery of new static vars that are implicitly declare target to doesn't >> result in marking of its dynamic initializers too.  Perhaps we need a >> langhook for that.  But if it is a separate function, either it is called >> before the other discovery and will ignore static initializers for vars >> that will only be marked as implicit declare target to later, or it is done >> afterwards, but then it would really need to duplicate everything what the >> other function does, otherwise it woiuldn't discover everything. >> > > I have added a new langhook GET_DECL_INIT that by default returns the > DECL_INITIAL of a variable declaration, but for C++ can also return the dynamic > initializer if present. omp_discover_implicit_declare_target and > omp_discover_declare_target_var_r have been changed to use the new langhook > instead of using DECL_INITIAL. > > The dynamic initializer information is stored in a new variable > dynamic_initializers. The information is originally stored in static_aggregates, > but this is nulled by calling prune_vars_needing_no_initialization in > c_parse_final_cleanups. I copy the information into a separate variable before > it is discarded - this avoids any potential problems that may be caused by > trying to change the way that static_aggregates currently works. > > With this, all the functions and variables in your example are marked correctly: > > foo () > ... > > __attribute__((omp declare target)) > bar () > ... > > __attribute__((omp declare target)) > baz () > ... > > __attribute__((omp declare target)) > qux () > ... > > .offload_var_table: >         .quad   g >         .quad   8 >         .quad   d >         .quad   8 >         .quad   b >         .quad   4 >         .quad   c >         .quad   4 >         .quad   f >         .quad   4 >         .quad   e >         .quad   4 > > Your example is now a compile test in g++.dg/gomp/. > >> Anyway, that is one thing, the other is even if the implicit declare target >> discovery handles those correctly, the question is what should we do >> afterwards.  Because the C++ FE normally creates a single function that >> performs the dynamic initialization of the TUs variables.  But that function >> shouldn't be really declare target to, it initializes not only (explicit or >> implicit) declare target to variables, but also host only variables. >> So we'll probably need to create next to that host only TU constructor >> also a device only constructor function that will only initialize the >> declare target to variables. > > Even without this patch, G++ currently accepts something like > > int foo() { return 1; } > int x = foo(); > #pragma omp declare target to(x) > > but will not generate the device-side initializer for x, even though x is now > present on the device. So this part of the implementation is broken with or > without the patch. > > Given that my patch doesn't make the current situation any worse, can I commit > this portion of it to trunk for now, and leave device-side dynamic > initialization for later? > > Bootstrapped on x86_64 with no offloading, G++ testsuite ran with no > regressions, and no regressions in the libgomp testsuite with Nvidia offloading. > > Thanks, > > Kwok