From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from esa1.mentor.iphmx.com (esa1.mentor.iphmx.com [68.232.129.153]) by sourceware.org (Postfix) with ESMTPS id 46D9D3858408 for ; Wed, 6 Jul 2022 14:19:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 46D9D3858408 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mentor.com X-IronPort-AV: E=Sophos;i="5.92,250,1650960000"; d="scan'208";a="81053641" Received: from orw-gwy-02-in.mentorg.com ([192.94.38.167]) by esa1.mentor.iphmx.com with ESMTP; 06 Jul 2022 06:19:50 -0800 IronPort-SDR: xUctwW5OzOJ28/xICA7LZ0Ggu/Rup5j5HfMkmLPl2yutBr8j7CdxPAW7xKvM5M9GQVC2SCz6vy 2AtWRg+z7/G0jITgocFGy15N/BS9HK7XKfvGpcA8ds5PTcSd457yHLyVYE7R8aWMZ9NgliXEib PuWVfpeiLQnBaN8Q0g6Yoh3qCVO/yb0JsnDV3jbuBpcRfe5XP5VzZIgbw52cdpQomnfaT5LO+5 Xyv23hBHf3mHgwWp40pJO81FP0gHr6/Qi+Y/WvCSKvq8HeDqSUg8JsMDAKiB+HaLfK9WBG7f00 sHQ= Message-ID: <96669115-20a2-4962-01e3-e37f672f3b7b@codesourcery.com> Date: Wed, 6 Jul 2022 16:19:37 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Subject: Re: Fix Intel MIC 'mkoffload' for OpenMP 'requires' (was: [Patch] OpenMP: Move omp requires checks to libgomp) Content-Language: en-US To: Thomas Schwinge , CC: Jakub Jelinek References: <07fec82a-41cf-fdc5-6307-c068dd95ef1a@mentor.com> <87r12y4i3u.fsf@euler.schwinge.homeip.net> From: Tobias Burnus In-Reply-To: <87r12y4i3u.fsf@euler.schwinge.homeip.net> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: quoted-printable X-Originating-IP: [137.202.0.90] X-ClientProxiedBy: svr-ies-mbx-15.mgc.mentorg.com (139.181.222.15) To svr-ies-mbx-12.mgc.mentorg.com (139.181.222.12) X-Spam-Status: No, score=-11.4 required=5.0 tests=BAYES_00, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, NICE_REPLY_A, SPF_HELO_PASS, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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: Wed, 06 Jul 2022 14:19:53 -0000 Hi Thomas, hello all, On 06.07.22 13:04, Thomas Schwinge wrote: > On 2022-06-08T05:56:02+0200, Tobias Burnus > wrote: >> PS: I have not fully tested the intelmic version. ... > Subject: [PATCH] Fix Intel MIC 'mkoffload' for OpenMP 'requires' ... > This also means finally switching Intel MIC 'mkoffload' to > 'GOMP_offload_register_ver', 'GOMP_offload_unregister_ver', > making 'GOMP_offload_register', 'GOMP_offload_unregister' > legacy entry points. ... > gcc/ > * config/i386/intelmic-mkoffload.cc (generate_host_descr_file) > (prepare_target_image, main): Handle OpenMP 'requires'. > (generate_host_descr_file): Switch to 'GOMP_offload_register_ver', > 'GOMP_offload_unregister_ver'. > libgomp/ > * target.c (GOMP_offload_register, GOMP_offload_unregister): > Denote as legacy entry points. > * testsuite/libgomp.c-c++-common/requires-1.c: Enable for all > 'target offloading_enabled'. > * testsuite/libgomp.c-c++-common/requires-5.c: Likewise. > * testsuite/libgomp.c-c++-common/requires-7.c: Likewise. > * testsuite/libgomp.fortran/requires-1.f90: Likewise. ... > diff --git a/gcc/config/i386/intelmic-mkoffload.cc b/gcc/config/i386/inte= lmic-mkoffload.cc > index c683d6f473e..596f6f107b8 100644 > --- a/gcc/config/i386/intelmic-mkoffload.cc > +++ b/gcc/config/i386/intelmic-mkoffload.cc > @@ -370,7 +370,7 @@ generate_target_offloadend_file (const char *target_c= ompiler) > > /* Generates object file with the host side descriptor. */ > static const char * > -generate_host_descr_file (const char *host_compiler) > +generate_host_descr_file (const char *host_compiler, uint32_t omp_requir= es) > { > char *dump_filename =3D concat (dumppfx, "_host_descr.c", NULL); > const char *src_filename =3D save_temps > @@ -386,39 +386,50 @@ generate_host_descr_file (const char *host_compiler= ) > if (!src_file) > fatal_error (input_location, "cannot open '%s'", src_filename); > > + fprintf (src_file, "#include \n\n"); > + > fprintf (src_file, > "extern const void *const __OFFLOAD_TABLE__;\n" > "extern const void *const __offload_image_intelmic_start;\n" > "extern const void *const __offload_image_intelmic_end;\n\n" > > - "static const void *const __offload_target_data[] =3D {\n" > + "static const struct intelmic_data {\n" > + " uintptr_t omp_requires_mask;\n" > + " const void *const image_start;\n" > + " const void *const image_end;\n" > + "} intelmic_data =3D {\n" > + " %d,\n" > " &__offload_image_intelmic_start, &__offload_image_intelmic_en= d\n" > - "};\n\n"); > + "};\n\n", omp_requires); > > fprintf (src_file, > "#ifdef __cplusplus\n" > "extern \"C\"\n" > "#endif\n" > - "void GOMP_offload_register (const void *, int, const void *);\n= " > + "void GOMP_offload_register_ver (unsigned, const void *, int, co= nst void *);\n" This line now ends in column 90, I think you want to wrap it. > "#ifdef __cplusplus\n" > "extern \"C\"\n" > "#endif\n" > - "void GOMP_offload_unregister (const void *, int, const void *);= \n\n" > + "void GOMP_offload_unregister_ver (unsigned, const void *, int, = const void *);\n\n" Likewise - before col 80, now col 94. > > "__attribute__((constructor))\n" > "static void\n" > "init (void)\n" > "{\n" > - " GOMP_offload_register (&__OFFLOAD_TABLE__, %d, __offload_targ= et_data);\n" > - "}\n\n", GOMP_DEVICE_INTEL_MIC); > + " GOMP_offload_register_ver (%#x, &__OFFLOAD_TABLE__, %d, &inte= lmic_data);\n" Likewise - albeit before already col 87, now col 89. > + "}\n\n", > + GOMP_VERSION_PACK (GOMP_VERSION, GOMP_VERSION_INTEL_MIC), > + GOMP_DEVICE_INTEL_MIC); > > fprintf (src_file, > "__attribute__((destructor))\n" > "static void\n" > "fini (void)\n" > "{\n" > - " GOMP_offload_unregister (&__OFFLOAD_TABLE__, %d, __offload_ta= rget_data);\n" > - "}\n", GOMP_DEVICE_INTEL_MIC); > > + "}\n", > + GOMP_VERSION_PACK (GOMP_VERSION, GOMP_VERSION_INTEL_MIC), > + GOMP_DEVICE_INTEL_MIC); > > fclose (src_file); > > @@ -462,7 +473,7 @@ generate_host_descr_file (const char *host_compiler) > } > > static const char * > -prepare_target_image (const char *target_compiler, int argc, char **argv= ) > +prepare_target_image (const char *target_compiler, int argc, char **argv= , uint32_t *omp_requires) Likewise now too long. > { > const char *target_descr_filename > =3D generate_target_descr_file (target_compiler); > @@ -509,8 +520,26 @@ prepare_target_image (const char *target_compiler, i= nt argc, char **argv) > obstack_ptr_grow (&argv_obstack, ""); > obstack_ptr_grow (&argv_obstack, "-o"); > obstack_ptr_grow (&argv_obstack, target_so_filename); > + > + char *omp_requires_file; > + if (save_temps) > + omp_requires_file =3D concat (dumppfx, ".mkoffload.omp_requires", NU= LL); > + else > + omp_requires_file =3D make_temp_file (".mkoffload.omp_requires"); > + xputenv (concat ("GCC_OFFLOAD_OMP_REQUIRES_FILE=3D", omp_requires_file= , NULL)); > + > compile_for_target (&argv_obstack); > > + unsetenv("GCC_OFFLOAD_OMP_REQUIRES_FILE"); > + FILE *in =3D fopen (omp_requires_file, "rb"); > + if (!in) > + fatal_error (input_location, "cannot open omp_requires file %qs", > + omp_requires_file); > + if (fread (omp_requires, sizeof (*omp_requires), 1, in) !=3D 1) > + fatal_error (input_location, "cannot read omp_requires file %qs", > + omp_requires_file); > + fclose (in); > + > /* Run objcopy. */ > char *rename_section_opt > =3D XALLOCAVEC (char, sizeof (".data=3D") + strlen (image_section_n= ame)); > @@ -643,10 +672,13 @@ main (int argc, char **argv) > if (!dumppfx) > dumppfx =3D out_obj_filename; > > + uint32_t omp_requires; > + > const char *target_so_filename > - =3D prepare_target_image (target_compiler, argc, argv); > + =3D prepare_target_image (target_compiler, argc, argv, &omp_requires= ); > > - const char *host_descr_filename =3D generate_host_descr_file (host_com= piler); > + const char *host_descr_filename > + =3D generate_host_descr_file (host_compiler, omp_requires); > > /* Perform partial linking for the target image and host side descrip= tor. > As a result we'll get a finalized object file with all offload dat= a. */ > diff --git a/libgomp/target.c b/libgomp/target.c > index 288b748b9e8..18c5f6e27db 100644 > --- a/libgomp/target.c > +++ b/libgomp/target.c > @@ -2410,6 +2410,8 @@ GOMP_offload_register_ver (unsigned version, const = void *host_table, > gomp_mutex_unlock (®ister_lock); > } > > +/* Legacy entry point. */ > + > void > GOMP_offload_register (const void *host_table, int target_type, > const void *target_data) > @@ -2465,6 +2467,8 @@ GOMP_offload_unregister_ver (unsigned version, cons= t void *host_table, > gomp_mutex_unlock (®ister_lock); > } > > +/* Legacy entry point. */ > + > void > GOMP_offload_unregister (const void *host_table, int target_type, > const void *target_data) Except for the too-long lines, it LGTM up to here. However, regarding the following, I prefer to have something like 'offload_= target_any' (or similar), instead of (ab)using 'offloading_enabled', as discussed elsew= here in this thread. While it does not make currently any difference, I could imagine to do the = same for OpenMP as done in libgomp.oacc-*/*exp. Namely: Compile/run all 'omp tar= get' testcases in addition with -foffload=3Ddisable. As we don't do this, yet, I think it is fine to apply the patch for now. Ho= wever, if you don't want to fix it as follow up (please do), can you at least open a = PR about it? > diff --git a/libgomp/testsuite/libgomp.c-c++-common/requires-1.c b/libgom= p/testsuite/libgomp.c-c++-common/requires-1.c > index fedf9779769..8eaac54e187 100644 > --- a/libgomp/testsuite/libgomp.c-c++-common/requires-1.c > +++ b/libgomp/testsuite/libgomp.c-c++-common/requires-1.c > @@ -1,4 +1,4 @@ > -/* { dg-do link { target { offload_target_nvptx || offload_target_amdgcn= } } } */ > +/* { dg-do link { target offloading_enabled } } */ > /* { dg-additional-sources requires-1-aux.c } */ > > /* Check diagnostic by device-compiler's lto1. > diff --git a/libgomp/testsuite/libgomp.c-c++-common/requires-5.c b/libgom= p/testsuite/libgomp.c-c++-common/requires-5.c > index c1e5540cfc5..5aa04a5a604 100644 > --- a/libgomp/testsuite/libgomp.c-c++-common/requires-5.c > +++ b/libgomp/testsuite/libgomp.c-c++-common/requires-5.c > @@ -1,4 +1,4 @@ > -/* { dg-do run { target { offload_target_nvptx || offload_target_amdgcn = } } } */ > +/* { dg-do run { target offloading_enabled } } */ > /* { dg-additional-sources requires-5-aux.c } */ > > #pragma omp requires unified_shared_memory, unified_address, reverse_of= fload > diff --git a/libgomp/testsuite/libgomp.c-c++-common/requires-7.c b/libgom= p/testsuite/libgomp.c-c++-common/requires-7.c > index c94a4c10846..31c6f73c6da 100644 > --- a/libgomp/testsuite/libgomp.c-c++-common/requires-7.c > +++ b/libgomp/testsuite/libgomp.c-c++-common/requires-7.c > @@ -1,4 +1,4 @@ > -/* { dg-do link { target { offload_target_nvptx || offload_target_amdgcn= } } } */ > +/* { dg-do link { target offloading_enabled } } */ > /* { dg-additional-sources requires-7-aux.c } */ > > /* Check diagnostic by device-compiler's lto1. > diff --git a/libgomp/testsuite/libgomp.fortran/requires-1.f90 b/libgomp/t= estsuite/libgomp.fortran/requires-1.f90 > index 33741af15f1..1020ebb4277 100644 > --- a/libgomp/testsuite/libgomp.fortran/requires-1.f90 > +++ b/libgomp/testsuite/libgomp.fortran/requires-1.f90 > @@ -1,4 +1,4 @@ > -! { dg-do link { target { offload_target_nvptx || offload_target_amdgcn = } } } > +! { dg-do link { target offloading_enabled } } > ! { dg-additional-sources requires-1-aux.f90 } > > ! Check diagnostic by device-compiler's lto1. Thanks, Tobias ----------------- Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstra=C3=9Fe 201= , 80634 M=C3=BCnchen; Gesellschaft mit beschr=C3=A4nkter Haftung; Gesch=C3= =A4ftsf=C3=BChrer: Thomas Heurung, Frank Th=C3=BCrauf; Sitz der Gesellschaf= t: M=C3=BCnchen; Registergericht M=C3=BCnchen, HRB 106955