Hi Julian! On Thu, 7 May 2015 16:56:11 +0100, Julian Brown <jul...@codesourcery.com> wrote: > On Tue, 5 May 2015 16:09:18 +0200 > Thomas Schwinge <tho...@codesourcery.com> wrote: > > On Tue, 5 May 2015 08:43:48 -0400, John David Anglin > > <dave.ang...@bell.net> wrote: > > > On 2015-05-05 5:43 AM, Thomas Schwinge wrote: > > > >> FAIL: libgomp.oacc-c/../libgomp.oacc-c-c++-common/lib-62.c > > > >> >-DACC_DEVICE_TYPE_hos > > > >> >t=1 -DACC_MEM_SHARED=1 output pattern test, is , should match > > > >> >invalid size > > > > With this one I'll need your help: please cite from libgomp.log > > > > (or, from a manual run) the actual output message that you're > > > > getting. > > > There's no output message: > > > # ./lib-62.exe > > > Segmentation fault (core dumped) > > > As this is a PA-RISC HP-UX system, I feel certain that you don't > > actually have nvptx offloading available (so, the nvptx libgomp > > plugin is not being built). However, this test case, contains an > > unconditional acc_init call for acc_device_nvidia, and I would then > > guess that this situation is not (not anymore?) correctly handled > > (abort with »offloading to [...] not possible«, or similar; see > > libgomp.oacc-c-c++-common/lib-4.c) in libgomp -- Julian, could this be > > due to your recent libgomp OpenACC initialization changes? (When > > working on this in a build that does have nvptx offloading > > configured, I think you should be able to simulate the situation by > > "hiding" (temporarily deleting, or similar) the nvptx libgomp > > plugin?) > > The attached patch contains (what I hope should be) a fix for this, > tested by running the libgomp testsuite (with nvptx offloading), and by > deleting the nvptx plugin, with the patch applied, and ensuring that > lib-62.c no longer segfaults in that case. > > The patch also tidies up a few other error paths around resolve_device, > and de-duplicates some error message reporting code. > > > Then, I don't know why libgomp.oacc-c-c++-common/lib-62.c contains > > this explicit acc_init call with acc_device_nvidia -- generally, the > > test cases should not contain such unconditional statements. So, > > let's then please remove this. See > > libgomp/testsuite/libgomp.oacc-c-c++-common/lib-66.c for a very > > similar test case, which does this differently. > > I've not touched this test though -- but I have tweaked > libgomp.oacc-c-c++-common/lib-4.c that should now expect a slightly > different error output. > > OK for trunk?
Thanks, looks good to me -- Jakub? Grüße, Thomas > libgomp/ > * oacc-init.c (resolve_device): Add FAIL_IS_ERROR argument. Update > function comment. Only call gomp_fatal if new argument is true. > (acc_dev_num_out_of_range): New function. > (acc_init_1, acc_shutdown_1): Update call to resolve_device. Call > acc_dev_num_out_of_range as appropriate. > (acc_get_num_devices, acc_set_device_type, acc_get_device_type) > (acc_get_device_num, acc_set_device_num): Update calls to resolve_device. > * testsuite/libgomp.oacc-c-c++-common/lib-4.c: Update expected test > output. > commit 221b5dea47cdb7611456ca3cf28d180d3ff1156a > Author: Julian Brown <jul...@codesourcery.com> > Date: Thu May 7 08:39:16 2015 -0700 > > Clean up initialisation when no devices of a particular type are > available. > > diff --git a/libgomp/oacc-init.c b/libgomp/oacc-init.c > index f2c60ec..cd50521 100644 > --- a/libgomp/oacc-init.c > +++ b/libgomp/oacc-init.c > @@ -109,10 +109,12 @@ name_of_acc_device_t (enum acc_device_t type) > } > } > > -/* ACC_DEVICE_LOCK should be held before calling this function. */ > +/* ACC_DEVICE_LOCK must be held before calling this function. If > FAIL_IS_ERROR > + is true, this function raises an error if there are no devices of type D, > + otherwise it returns NULL in that case. */ > > static struct gomp_device_descr * > -resolve_device (acc_device_t d) > +resolve_device (acc_device_t d, bool fail_is_error) > { > acc_device_t d_arg = d; > > @@ -130,7 +132,13 @@ resolve_device (acc_device_t d) > && dispatchers[d]->get_num_devices_func () > 0) > goto found; > > - gomp_fatal ("device type %s not supported", goacc_device_type); > + if (fail_is_error) > + { > + gomp_mutex_unlock (&acc_device_lock); > + gomp_fatal ("device type %s not supported", goacc_device_type); > + } > + else > + return NULL; > } > > /* No default device specified, so start scanning for any non-host > @@ -149,7 +157,13 @@ resolve_device (acc_device_t d) > d = acc_device_host; > goto found; > } > - gomp_fatal ("no device found"); > + if (fail_is_error) > + { > + gomp_mutex_unlock (&acc_device_lock); > + gomp_fatal ("no device found"); > + } > + else > + return NULL; > break; > > case acc_device_host: > @@ -157,7 +171,12 @@ resolve_device (acc_device_t d) > > default: > if (d > _ACC_device_hwm) > - gomp_fatal ("device %u out of range", (unsigned)d); > + { > + if (fail_is_error) > + goto unsupported_device; > + else > + return NULL; > + } > break; > } > found: > @@ -166,12 +185,30 @@ resolve_device (acc_device_t d) > && d != acc_device_default > && d != acc_device_not_host); > > + if (dispatchers[d] == NULL && fail_is_error) > + { > + unsupported_device: > + gomp_mutex_unlock (&acc_device_lock); > + gomp_fatal ("device type %s not supported", name_of_acc_device_t (d)); > + } > + > return dispatchers[d]; > } > > +/* Emit a suitable error if no device of a particular type is available, or > + the given device number is out-of-range. */ > +static void > +acc_dev_num_out_of_range (acc_device_t d, int ord, int ndevs) > +{ > + if (ndevs == 0) > + gomp_fatal ("no devices of type %s available", name_of_acc_device_t (d)); > + else > + gomp_fatal ("device %u out of range", ord); > +} > + > /* This is called when plugins have been initialized, and serves to call > (indirectly) the target's device_init hook. Calling multiple times > without > - an intervening acc_shutdown_1 call is an error. ACC_DEVICE_LOCK should be > + an intervening acc_shutdown_1 call is an error. ACC_DEVICE_LOCK must be > held before calling this function. */ > > static struct gomp_device_descr * > @@ -180,12 +217,12 @@ acc_init_1 (acc_device_t d) > struct gomp_device_descr *base_dev, *acc_dev; > int ndevs; > > - base_dev = resolve_device (d); > + base_dev = resolve_device (d, true); > > ndevs = base_dev->get_num_devices_func (); > > - if (!base_dev || ndevs <= 0 || goacc_device_num >= ndevs) > - gomp_fatal ("device %s not supported", name_of_acc_device_t (d)); > + if (ndevs <= 0 || goacc_device_num >= ndevs) > + acc_dev_num_out_of_range (d, goacc_device_num, ndevs); > > acc_dev = &base_dev[goacc_device_num]; > > @@ -202,7 +239,7 @@ acc_init_1 (acc_device_t d) > return base_dev; > } > > -/* ACC_DEVICE_LOCK should be held before calling this function. */ > +/* ACC_DEVICE_LOCK must be held before calling this function. */ > > static void > acc_shutdown_1 (acc_device_t d) > @@ -213,10 +250,7 @@ acc_shutdown_1 (acc_device_t d) > bool devices_active = false; > > /* Get the base device for this device type. */ > - base_dev = resolve_device (d); > - > - if (!base_dev) > - gomp_fatal ("device %s not supported", name_of_acc_device_t (d)); > + base_dev = resolve_device (d, true); > > gomp_mutex_lock (&goacc_thread_lock); > > @@ -364,7 +398,8 @@ goacc_attach_host_thread_to_device (int ord) > > num_devices = base_dev->get_num_devices_func (); > if (num_devices <= 0 || ord >= num_devices) > - gomp_fatal ("device %u out of range", ord); > + acc_dev_num_out_of_range (acc_device_type (base_dev->type), ord, > + num_devices); > > if (!thr) > thr = goacc_new_thread (); > @@ -424,7 +459,7 @@ acc_get_num_devices (acc_device_t d) > gomp_init_targets_once (); > > gomp_mutex_lock (&acc_device_lock); > - acc_dev = resolve_device (d); > + acc_dev = resolve_device (d, false); > gomp_mutex_unlock (&acc_device_lock); > > if (!acc_dev) > @@ -454,7 +489,7 @@ acc_set_device_type (acc_device_t d) > if (!cached_base_dev) > gomp_init_targets_once (); > > - cached_base_dev = base_dev = resolve_device (d); > + cached_base_dev = base_dev = resolve_device (d, true); > acc_dev = &base_dev[goacc_device_num]; > > gomp_mutex_lock (&acc_dev->lock); > @@ -492,7 +527,7 @@ acc_get_device_type (void) > gomp_init_targets_once (); > > gomp_mutex_lock (&acc_device_lock); > - dev = resolve_device (acc_device_default); > + dev = resolve_device (acc_device_default, true); > gomp_mutex_unlock (&acc_device_lock); > res = acc_device_type (dev->type); > } > @@ -512,16 +547,14 @@ acc_get_device_num (acc_device_t d) > struct goacc_thread *thr = goacc_thread (); > > if (d >= _ACC_device_hwm) > - gomp_fatal ("device %u out of range", (unsigned)d); > + gomp_fatal ("unknown device type %u", (unsigned) d); > > if (!cached_base_dev) > gomp_init_targets_once (); > > gomp_mutex_lock (&acc_device_lock); > - dev = resolve_device (d); > + dev = resolve_device (d, true); > gomp_mutex_unlock (&acc_device_lock); > - if (!dev) > - gomp_fatal ("device %s not supported", name_of_acc_device_t (d)); > > if (thr && thr->base_dev == dev && thr->dev) > return thr->dev->target_id; > @@ -552,12 +585,12 @@ acc_set_device_num (int ord, acc_device_t d) > { > gomp_mutex_lock (&acc_device_lock); > > - cached_base_dev = base_dev = resolve_device (d); > + cached_base_dev = base_dev = resolve_device (d, true); > > num_devices = base_dev->get_num_devices_func (); > > - if (ord >= num_devices) > - gomp_fatal ("device %u out of range", ord); > + if (num_devices <= 0 || ord >= num_devices) > + acc_dev_num_out_of_range (d, ord, num_devices); > > acc_dev = &base_dev[ord]; > > diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/lib-4.c > b/libgomp/testsuite/libgomp.oacc-c-c++-common/lib-4.c > index 35f9440..3bb9ea5 100644 > --- a/libgomp/testsuite/libgomp.oacc-c-c++-common/lib-4.c > +++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/lib-4.c > @@ -10,5 +10,5 @@ main (int argc, char **argv) > return 0; > } > > -/* { dg-output "device \[0-9\]+ out of range" } */ > +/* { dg-output "unknown device type \[0-9\]+" } */ > /* { dg-shouldfail "" } */
signature.asc
Description: PGP signature