Page MenuHomePhabricator

AlexEichenberger (Alexandre Eichenberger)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 1 2014, 1:52 PM (285 w, 4 d)

Recent Activity

Jul 31 2019

AlexEichenberger accepted D65340: [OpenMP][libomptarget] Add support for close map modifier.

LGTM

Jul 31 2019, 1:49 PM · Restricted Project, Restricted Project

Jul 29 2019

AlexEichenberger added inline comments to D65001: [OpenMP][libomptarget] Add support for unified memory for regular maps.
Jul 29 2019, 2:36 PM · Restricted Project, Restricted Project

Jun 14 2019

AlexEichenberger added inline comments to D63009: [OpenMP] Add target task alloc function with device ID.
Jun 14 2019, 12:02 PM · Restricted Project, Restricted Project, Restricted Project

Jun 12 2019

AlexEichenberger accepted D63009: [OpenMP] Add target task alloc function with device ID.

LGTM

Jun 12 2019, 10:44 AM · Restricted Project, Restricted Project, Restricted Project

Jun 11 2019

AlexEichenberger accepted D63010: [OpenMP] Add task alloc function.

LGTM

Jun 11 2019, 1:59 PM · Restricted Project, Restricted Project, Restricted Project

Jun 10 2019

AlexEichenberger added inline comments to D63010: [OpenMP] Add task alloc function.
Jun 10 2019, 8:35 AM · Restricted Project, Restricted Project, Restricted Project
AlexEichenberger added inline comments to D63009: [OpenMP] Add target task alloc function with device ID.
Jun 10 2019, 8:35 AM · Restricted Project, Restricted Project, Restricted Project

Apr 16 2019

AlexEichenberger accepted D60568: [OpenMP] Add support for registering requires directives with the runtime.

fantastic

Apr 16 2019, 12:18 PM · Restricted Project, Restricted Project
AlexEichenberger added a comment to D60223: [OpenMP][libomptarget] Enable requires flags for target libraries..

response to Alexey's comment

Apr 16 2019, 11:16 AM · Restricted Project
AlexEichenberger accepted D60223: [OpenMP][libomptarget] Enable requires flags for target libraries..

LGTM, just made one minor suggestion

Apr 16 2019, 11:15 AM · Restricted Project

Apr 12 2019

AlexEichenberger requested changes to D60568: [OpenMP] Add support for registering requires directives with the runtime.

see note above; apologies if it is already done and hiding somewhere I did not see

Apr 12 2019, 6:17 AM · Restricted Project, Restricted Project

Apr 8 2019

AlexEichenberger requested changes to D60223: [OpenMP][libomptarget] Enable requires flags for target libraries..

See suggested changes, should be pretty straightforward to do, let me know if you need help

Apr 8 2019, 2:28 PM · Restricted Project

Apr 3 2019

AlexEichenberger requested changes to D60223: [OpenMP][libomptarget] Enable requires flags for target libraries..

see comments

Apr 3 2019, 12:23 PM · Restricted Project

Sep 4 2018

AlexEichenberger added a comment to D51623: [libomptarget] PR38704: Fix erase of ShadowPtrMap.

make sense, thanks

Sep 4 2018, 10:05 AM

Aug 27 2018

AlexEichenberger added a comment to D51285: Fix a build issue on Debian Jessie.

Yes, we should have done this right away. I did a rewrite of the functionality that uses header files already included; it was posted in D50522 and the changes were approved and pushed to svn.

Aug 27 2018, 8:01 PM
AlexEichenberger committed rL340767: [OpenMP][libomptarget] rework of fatal error reporting.
[OpenMP][libomptarget] rework of fatal error reporting
Aug 27 2018, 11:22 AM
AlexEichenberger committed rOMP340767: [OpenMP][libomptarget] rework of fatal error reporting.
[OpenMP][libomptarget] rework of fatal error reporting
Aug 27 2018, 11:22 AM
AlexEichenberger closed D51226: [OpenMP][libomptarget] rework of fatal error reporting.
Aug 27 2018, 11:22 AM
AlexEichenberger added inline comments to D51226: [OpenMP][libomptarget] rework of fatal error reporting.
Aug 27 2018, 8:10 AM
AlexEichenberger updated the diff for D51226: [OpenMP][libomptarget] rework of fatal error reporting.
  • Merge branch 'unpatched-master' into unpatched-master-target-offload-envvar
  • while (0) edit
Aug 27 2018, 8:09 AM

Aug 26 2018

AlexEichenberger updated the diff for D51226: [OpenMP][libomptarget] rework of fatal error reporting.
  • added do while(1) for macros
Aug 26 2018, 7:07 PM

Aug 24 2018

AlexEichenberger updated the diff for D51226: [OpenMP][libomptarget] rework of fatal error reporting.
  • put fatal message for default tgt test
Aug 24 2018, 5:55 PM
AlexEichenberger added inline comments to D51226: [OpenMP][libomptarget] rework of fatal error reporting.
Aug 24 2018, 2:12 PM
AlexEichenberger added inline comments to D51226: [OpenMP][libomptarget] rework of fatal error reporting.
Aug 24 2018, 12:00 PM
AlexEichenberger added a comment to D51226: [OpenMP][libomptarget] rework of fatal error reporting.

Alexey was not happy about adding an include, and we already used the define approach for the debug messages. So we reuse the same functionality that we already used for debug.
Agreed that Johnas suggested it, and given the problem with the include, Alexey urged me to change the code to what we have here.

Aug 24 2018, 11:40 AM
AlexEichenberger added a comment to D51226: [OpenMP][libomptarget] rework of fatal error reporting.

I don't understand this: In the last review you said that we need the complicated FatalMessage and now you are reverting that?!?

Aug 24 2018, 10:49 AM
AlexEichenberger updated the diff for D51226: [OpenMP][libomptarget] rework of fatal error reporting.
  • format
Aug 24 2018, 10:46 AM
AlexEichenberger added a comment to D51219: [OpenMP] Add missing header. va_start, va_end undeclared in gcc 5.5.0.

I reworked the solution to avoid varargs, and proposed a more robust solution in D51226

Aug 24 2018, 10:38 AM
AlexEichenberger retitled D51226: [OpenMP][libomptarget] rework of fatal error reporting from [OpenMP][libomptarget] Bringing up to spec with respect to OMP_TARGET_OFFLOAD env var to [OpenMP][libomptarget] rework of fatal error reporting.
Aug 24 2018, 10:36 AM
AlexEichenberger created D51226: [OpenMP][libomptarget] rework of fatal error reporting.
Aug 24 2018, 10:34 AM

Aug 23 2018

AlexEichenberger added a comment to D51107: [LIBOMPTARGET] Add support for mapping of lambda captures..
Aug 23 2018, 8:05 PM
AlexEichenberger committed rL340542: [OpenMP][libomptarget] Bringing up to spec with respect to OMP_TARGET_OFFLOAD….
[OpenMP][libomptarget] Bringing up to spec with respect to OMP_TARGET_OFFLOAD…
Aug 23 2018, 9:23 AM
AlexEichenberger committed rOMP340542: [OpenMP][libomptarget] Bringing up to spec with respect to OMP_TARGET_OFFLOAD….
[OpenMP][libomptarget] Bringing up to spec with respect to OMP_TARGET_OFFLOAD…
Aug 23 2018, 9:23 AM
AlexEichenberger closed D50522: [OpenMP][libomptarget] Bringing up to spec with respect to OMP_TARGET_OFFLOAD env var.
Aug 23 2018, 9:23 AM
AlexEichenberger added inline comments to D51107: [LIBOMPTARGET] Add support for mapping of lambda captures..
Aug 23 2018, 7:20 AM

Aug 16 2018

AlexEichenberger updated the diff for D50522: [OpenMP][libomptarget] Bringing up to spec with respect to OMP_TARGET_OFFLOAD env var.

Thanks to all for your valuable comments, much appreciated, really contributed to the quality of the patch

Aug 16 2018, 6:26 AM

Aug 15 2018

AlexEichenberger added a comment to D50522: [OpenMP][libomptarget] Bringing up to spec with respect to OMP_TARGET_OFFLOAD env var.

updated comments status

Aug 15 2018, 8:30 AM
AlexEichenberger updated the diff for D50522: [OpenMP][libomptarget] Bringing up to spec with respect to OMP_TARGET_OFFLOAD env var.

fixed clang-format and moved fatal message function

Aug 15 2018, 8:28 AM

Aug 14 2018

AlexEichenberger added inline comments to D50522: [OpenMP][libomptarget] Bringing up to spec with respect to OMP_TARGET_OFFLOAD env var.
Aug 14 2018, 8:48 PM
AlexEichenberger added a comment to D50522: [OpenMP][libomptarget] Bringing up to spec with respect to OMP_TARGET_OFFLOAD env var.

responded to comments

Aug 14 2018, 1:00 PM
AlexEichenberger updated the diff for D50522: [OpenMP][libomptarget] Bringing up to spec with respect to OMP_TARGET_OFFLOAD env var.

implemented suggest changes

Aug 14 2018, 12:55 PM
AlexEichenberger added a comment to D50522: [OpenMP][libomptarget] Bringing up to spec with respect to OMP_TARGET_OFFLOAD env var.

responded to comments.

Aug 14 2018, 9:31 AM
AlexEichenberger updated the diff for D50522: [OpenMP][libomptarget] Bringing up to spec with respect to OMP_TARGET_OFFLOAD env var.

Addressed all remaining issues

Aug 14 2018, 9:28 AM

Aug 13 2018

AlexEichenberger added a comment to D50522: [OpenMP][libomptarget] Bringing up to spec with respect to OMP_TARGET_OFFLOAD env var.

Good arguments, I see why the consistency of why linking the number of devices returned and OMP_TARGET_OFFLOAD environment is good.
Implemented the requested changes

Aug 13 2018, 11:39 AM
AlexEichenberger updated the diff for D50522: [OpenMP][libomptarget] Bringing up to spec with respect to OMP_TARGET_OFFLOAD env var.

Default policy is now selected when registering the libraries, and will default to mandatory/disabled depending of whether there are one or more devices/none.

Aug 13 2018, 11:35 AM

Aug 10 2018

AlexEichenberger added inline comments to D50522: [OpenMP][libomptarget] Bringing up to spec with respect to OMP_TARGET_OFFLOAD env var.
Aug 10 2018, 3:18 PM
AlexEichenberger updated the diff for D50522: [OpenMP][libomptarget] Bringing up to spec with respect to OMP_TARGET_OFFLOAD env var.

Added a mutex around changing DEFAULT to MANDATORY or DISABLED
Added proper action when discovering failure (which is immediate abort of function being performed)
Failure to locate data during an update is now tolerated (a warning could be issued)

Aug 10 2018, 3:12 PM
AlexEichenberger added a comment to D50522: [OpenMP][libomptarget] Bringing up to spec with respect to OMP_TARGET_OFFLOAD env var.

Suppose we have 2 devices plugged into the system, and the first one cannot be used (for whatever reason: hardware failure, exclusive configuration and somebody else is running, etc.).
Now a clever application sees the two (because omp_get_num_devices() returns 2) and does:

#pragma omp parallel num_threads(omp_get_num_devices())
{
  #pragma omp target device(omp_get_thread_num())
  { }
}

I think the runtime behaviour with this patch depends on the execution order (and exposes a race condition in handle_target_outcome on TargetOffloadPolicy; let's ignore that for now):

  • If target device(0) executes first, libomptarget will notice the error and silently disable offloading. All target regions will execute on the host.
  • If however target device(1) executes first and returns successfully, libomptarget will raise OMP_TARGET_OFFLOAD to MANDATORY and will abort execution when catching the error of target device(0).

    I don't think that makes much sense. IMO the runtime should detect two "visible" -> "available" devices and abort execution in all cases.

Did you consider this example?

Aug 10 2018, 2:09 PM
AlexEichenberger added a comment to D50522: [OpenMP][libomptarget] Bringing up to spec with respect to OMP_TARGET_OFFLOAD env var.

I disagree. As it is currently written, omp_get_num_devices() can also grow over time, so you can also have it return zero, only to increase later to a larger number. What does that do to DEFAULT?
I believe the current policy is ok; if any device fails for any reason on first use, it becomes disabled. Anyone that want to rely on devices being there should use the MANDATORY policy.
I am happy to add a lock around the change of DEFAULT to MANDATORY or DISABLED.

Aug 10 2018, 8:30 AM

Aug 9 2018

AlexEichenberger added a comment to D50522: [OpenMP][libomptarget] Bringing up to spec with respect to OMP_TARGET_OFFLOAD env var.

You are right, "available" is not defined in the standard. I've always though of "plugged into the system", ie all devices that are visible to the CUDA runtime. That would match the current implementation of omp_get_num_devices which is defined to return "the number of available target devices".
Actually this behaviour would be important for us as we have our GPUs configured exclusively. So when there is already a process running all other users get a runtime error. In that case it would be very helpful to have libomptarget abort the program.

Aug 9 2018, 8:20 PM
AlexEichenberger added a comment to D50522: [OpenMP][libomptarget] Bringing up to spec with respect to OMP_TARGET_OFFLOAD env var.

Does this patch supersede D44522?

Aug 9 2018, 11:54 AM
AlexEichenberger created D50522: [OpenMP][libomptarget] Bringing up to spec with respect to OMP_TARGET_OFFLOAD env var.
Aug 9 2018, 10:55 AM

Aug 3 2018

AlexEichenberger added a comment to D50158: [OpenMP] Add placeholder functions for the depend and nowait depend clauses for target data directives..

agreed

Aug 3 2018, 9:03 AM

Aug 2 2018

AlexEichenberger added a comment to D50158: [OpenMP] Add placeholder functions for the depend and nowait depend clauses for target data directives..

First, there missing sync for the _tgt_target_data_begin_depend, tgt_target_data_begin_depend_nowait, tgt_target_data_update_depend, tgt_target_data_update_depend_nowait,
Look at the end versions of the call for a possible implementation.
Second, it would be better IMO to use
kmpc_omp_wait_deps instead of the broader task wait.

Aug 2 2018, 11:10 AM

Nov 22 2017

AlexEichenberger accepted D40175: Fix for OMP doacross implementation on Power.

Changes are correct, a memory barrier is needed between allocating/reseting values to zero and publishing the pointer. That way, another thread see the published pointer only once all of the zeroing is complete.

Nov 22 2017, 8:06 AM