Hi Tobias,
I just had a short look at your PR. Besides that it did not git-am for me (see
below), I have only one question (see also below). Please note, that I have
only user-level experience in OpenMP and can say nothing about completeness or
soundness of your PR. I hope that a first check on overall style motivates a
"pro" to have a more in-depth look.
So my "ok" is just for style and overall applicability.
Regards,
Andre
On Thu, 22 Aug 2024 09:14:58 +0200
Tobias Burnus <tbur...@baylibre.com> wrote:
> This is nearly identical to v2, except that I presumably used 'git add
> testsuite' when intending to use 'git add -u testsuite' in a last-minute
> change as it contained a bunch of unrelated test files …
>
> The only other change besides removing unrelated files is that for the
> generic part of omp_get_interop_type_desc, the data types ('int' for
> fr_id, vendor, device_num; const char*' for fr_name, vendor_name) are
> now returned in target.c while the specific types (for device,
> device_context, targetsync platform) will eventually be handled by the
> plugin function.
>
> Tobias
>
> Am 21.08.24 um 20:27 schrieb Tobias Burnus:
> > Nearly identical to v1, except that I realized that OpenMP permits to
> > call those functions also from target regions.
> >
> > Hence, those also got those functions, including a use of
> > omp_irc_other to make clear why it will fail …
> >
> > In addition, two (nonhost) target-region test files were added.
> >
> > Comments, remarks, suggestions before I commit it?
Attachment:
> libgomp: Add interop types and routines to OpenMP's headers and module
git am did not work for me (sorry for the German):
$ git am interop-1v2a.diff
Wende an: This commit adds OpenMP 5.1+'s interop enumeration, type and routine
/mnt/work_store/gcc/gcc/.git/worktrees/gcc.test/rebase-apply/patch:839: indent
with spaces.
"const char*", /* fr_name */
/mnt/work_store/gcc/gcc/.git/worktrees/gcc.test/rebase-apply/patch:840: indent
with spaces.
"int", /* vendor */
/mnt/work_store/gcc/gcc/.git/worktrees/gcc.test/rebase-apply/patch:841: indent
with spaces.
"const char *", /* vendor_name */
/mnt/work_store/gcc/gcc/.git/worktrees/gcc.test/rebase-apply/patch:842: indent
with spaces.
"int"}; /* device_num */
/mnt/work_store/gcc/gcc/.git/worktrees/gcc.test/rebase-apply/patch:1332: space
before tab in indent.
"omp_interop_none")); /* GCC implementation
choice. */
Warnung: 5 Zeilen fügen Whitespace-Fehler hinzu.
Schwerwiegend: Leerer Name in Identifikation (für <>) nicht erlaubt.
> diff --git a/libgomp/config/gcn/target.c b/libgomp/config/gcn/target.c
> index 9cafea4e2cc..e9141f20ef3 100644
> --- a/libgomp/config/gcn/target.c
> +++ b/libgomp/config/gcn/target.c
> @@ -185,3 +185,102 @@ GOMP_target_enter_exit_data (int device, size_t mapnum,
<SNIP>
> +omp_intptr_t
> +omp_get_interop_int (const omp_interop_t interop,
> + omp_interop_property_t property_id,
> + omp_interop_rc_t *ret_code)
> +{
> + if (property_id < omp_ipr_first || property_id >= 0)
> + *ret_code = omp_irc_out_of_range;
> + else if (interop == omp_interop_none)
> + *ret_code = omp_irc_empty;
> + else
> + *ret_code = omp_irc_other;
> + return 0;
Do I get this correct, that omp_intptr_t is a pointer to an integer? Would it
not be more intuitive to return nullptr here?
> +}
<snipp>
--
Andre Vehreschild * Email: vehre ad gmx dot de
