Hi Thomas,
On 30.01.20 16:54, Thomas Schwinge wrote:
>
> [...] the 'acc_device_current' interface should work already now.
>
> [...] Please review
> the attached (Tobias the Fortran test cases, please), and test with AMD
> GCN offloading. If approving this patch, please respond with
I have tested the patch with AMD GCN offloading and I have observed no
regressions.
The new tests pass as expected and print the correct output.
Great that you have extended the Fortran tests!
> diff --git a/libgomp/oacc-init.c b/libgomp/oacc-init.c
> index ef12b4c16d01..c28c0f689ba2 100644
> --- a/libgomp/oacc-init.c
> +++ b/libgomp/oacc-init.c
> @@ -796,7 +796,9 @@ get_property_any (int ord, acc_device_t d,
> acc_device_property_t prop)
> size_t
> acc_get_property (int ord, acc_device_t d, acc_device_property_t prop)
> {
> - if (!known_device_type_p (d))
> + if (d == acc_device_current)
> + ; /* Allowed only for 'acc_get_property', 'acc_get_property_string'. */
> + else if (!known_device_type_p (d))
> unknown_device_type_error(d);
I don't like the empty if branch very much. Introducing a variable
(for instance, "bool allowed_device_type = acc_device_current
|| known_device_type(d);") would also provide a place for your comment.
You could also extract a function to avoid duplicating the explanation
in acc_get_property_string.
The patch looks good to me.
Reviewed-by: Frederik Harwath <[email protected]>
Best regards,
Frederik