Hi Thomas,
On 03.12.19 13:14, Thomas Schwinge wrote:
> You once had this patch separate, but then merged into the upstream
> submission of 'acc_get_property'; let's please keep this separate.
>
> With changes as indicated below, please commit this to trunk [...]
Ok, I have committed the patch as revision 278936. You can find the committed
version in the attachment. Thank you for the review!
> Generally, does usage of these functions obsolete some existing usage of
> 'acc_dev_num_out_of_range'? (OK to address later.)
I think it does. I am going to verify this.
>> @@ -168,7 +184,7 @@ resolve_device (acc_device_t d, bool fail_is_error)
>> break;
>>
>> default:
>> - if (d > _ACC_device_hwm)
>> + if (!acc_known_device_type (d))
>> {
>> if (fail_is_error)
>> goto unsupported_device;
>
> Note that this had 'd > _ACC_device_hwm', not '>=' as it now does, that
> is, previously didn't reject 'd == _ACC_device_hwm' but now does -- but I
> suppose this was an (minor) bug that existed before, so OK to change as
> you did?
Right, I do not see any reasons why it should accept ACC_device_hwm and
the change did not cause any regressions.
Best regards,
Frederik
------------------------------------------------------------------------
r278937 | frederik | 2019-12-03 15:38:54 +0100 (Di, 03 Dez 2019) | 25 lines
Validate acc_device_t uses
Check that function arguments of type acc_device_t
are valid enumeration values in all publicly visible
functions from oacc-init.c.
2019-12-03 Frederik Harwath <frede...@codesourcery.com>
libgomp/
* oacc-init.c (acc_known_device_type): Add function.
(unknown_device_type_error): Add function.
(name_of_acc_device_t): Change to call unknown_device_type_error
on unknown type.
(resolve_device): Use acc_known_device_type.
(acc_init): Fail if acc_device_t argument is not valid.
(acc_shutdown): Likewise.
(acc_get_num_devices): Likewise.
(acc_set_device_type): Likewise.
(acc_get_device_num): Likewise.
(acc_set_device_num): Likewise.
(acc_on_device): Add comment that argument validity is not checked.
Reviewed-by: Thomas Schwinge <tho...@codesourcery.com>
------------------------------------------------------------------------
Index: libgomp/oacc-init.c
===================================================================
--- libgomp/oacc-init.c (revision 278936)
+++ libgomp/oacc-init.c (working copy)
@@ -82,6 +82,18 @@
gomp_mutex_unlock (&acc_device_lock);
}
+static bool
+known_device_type_p (acc_device_t d)
+{
+ return d >= 0 && d < _ACC_device_hwm;
+}
+
+static void
+unknown_device_type_error (acc_device_t invalid_type)
+{
+ gomp_fatal ("unknown device type %u", invalid_type);
+}
+
/* OpenACC names some things a little differently. */
static const char *
@@ -103,8 +115,9 @@
case acc_device_host: return "host";
case acc_device_not_host: return "not_host";
case acc_device_nvidia: return "nvidia";
- default: gomp_fatal ("unknown device type %u", (unsigned) type);
+ default: unknown_device_type_error (type);
}
+ __builtin_unreachable ();
}
/* ACC_DEVICE_LOCK must be held before calling this function. If FAIL_IS_ERROR
@@ -123,7 +136,7 @@
if (goacc_device_type)
{
/* Lookup the named device. */
- while (++d != _ACC_device_hwm)
+ while (known_device_type_p (++d))
if (dispatchers[d]
&& !strcasecmp (goacc_device_type,
get_openacc_name (dispatchers[d]->name))
@@ -147,7 +160,7 @@
case acc_device_not_host:
/* Find the first available device after acc_device_not_host. */
- while (++d != _ACC_device_hwm)
+ while (known_device_type_p (++d))
if (dispatchers[d] && dispatchers[d]->get_num_devices_func () > 0)
goto found;
if (d_arg == acc_device_default)
@@ -168,7 +181,7 @@
break;
default:
- if (d > _ACC_device_hwm)
+ if (!known_device_type_p (d))
{
if (fail_is_error)
goto unsupported_device;
@@ -505,6 +518,9 @@
void
acc_init (acc_device_t d)
{
+ if (!known_device_type_p (d))
+ unknown_device_type_error (d);
+
gomp_init_targets_once ();
gomp_mutex_lock (&acc_device_lock);
@@ -519,6 +535,9 @@
void
acc_shutdown (acc_device_t d)
{
+ if (!known_device_type_p (d))
+ unknown_device_type_error (d);
+
gomp_init_targets_once ();
gomp_mutex_lock (&acc_device_lock);
@@ -533,6 +552,9 @@
int
acc_get_num_devices (acc_device_t d)
{
+ if (!known_device_type_p (d))
+ unknown_device_type_error (d);
+
int n = 0;
struct gomp_device_descr *acc_dev;
@@ -564,6 +586,9 @@
void
acc_set_device_type (acc_device_t d)
{
+ if (!known_device_type_p (d))
+ unknown_device_type_error (d);
+
struct gomp_device_descr *base_dev, *acc_dev;
struct goacc_thread *thr = goacc_thread ();
@@ -647,12 +672,12 @@
int
acc_get_device_num (acc_device_t d)
{
+ if (!known_device_type_p (d))
+ unknown_device_type_error (d);
+
const struct gomp_device_descr *dev;
struct goacc_thread *thr = goacc_thread ();
- if (d >= _ACC_device_hwm)
- gomp_fatal ("unknown device type %u", (unsigned) d);
-
acc_prof_info prof_info;
acc_api_info api_info;
bool profiling_p = GOACC_PROFILING_SETUP_P (thr, &prof_info, &api_info);
@@ -682,6 +707,9 @@
void
acc_set_device_num (int ord, acc_device_t d)
{
+ if (!known_device_type_p (d))
+ unknown_device_type_error (d);
+
struct gomp_device_descr *base_dev, *acc_dev;
int num_devices;
@@ -728,8 +756,11 @@
version.
Compile this with optimization, so that the compiler expands
- this, rather than generating infinitely recursive code. */
+ this, rather than generating infinitely recursive code.
+ The function just forwards its argument to __builtin_acc_on_device. It does
+ not verify that the argument is a valid acc_device_t enumeration value. */
+
int __attribute__ ((__optimize__ ("O2")))
acc_on_device (acc_device_t dev)
{
Index: libgomp/ChangeLog
===================================================================
--- libgomp/ChangeLog (revision 278936)
+++ libgomp/ChangeLog (working copy)
@@ -1,3 +1,18 @@
+2019-12-03 Frederik Harwath <frede...@codesourcery.com>
+
+ * oacc-init.c (acc_known_device_type): Add function.
+ (unknown_device_type_error): Add function.
+ (name_of_acc_device_t): Change to call unknown_device_type_error
+ on unknown type.
+ (resolve_device): Use acc_known_device_type.
+ (acc_init): Fail if acc_device_t argument is not valid.
+ (acc_shutdown): Likewise.
+ (acc_get_num_devices): Likewise.
+ (acc_set_device_type): Likewise.
+ (acc_get_device_num): Likewise.
+ (acc_set_device_num): Likewise.
+ (acc_on_device): Add comment that argument validity is not checked.
+
2019-12-03 Andrew Stubbs <a...@codesourcery.com>
* testsuite/lib/libgomp.exp (offload_target_to_openacc_device_type):
