Page MenuHomePhabricator

grokos (George Rokos)
User

Projects

User does not belong to any projects.

User Details

User Since
Feb 16 2016, 1:22 PM (278 w, 2 d)

Recent Activity

Today

grokos added inline comments to D104382: [OpenMP][Offloading] Refined return value of `DeviceTy::getOrAllocTgtPtr`.
Thu, Jun 17, 12:47 AM · Restricted Project

Tue, Jun 8

grokos accepted D103927: [OpenMP] Add an information flag for device data transfers.
Tue, Jun 8, 4:47 PM · Restricted Project
grokos added inline comments to D103927: [OpenMP] Add an information flag for device data transfers.
Tue, Jun 8, 2:41 PM · Restricted Project
grokos added inline comments to D103923: [OpenMP] Add thread limit environment variable support to plugins.
Tue, Jun 8, 12:36 PM · Restricted Project

Fri, May 21

grokos committed rGd0bc04d6b91d: [libomptarget] Fix a bug whereby firstprivates are not copied over to the device (authored by grokos).
[libomptarget] Fix a bug whereby firstprivates are not copied over to the device
Fri, May 21, 10:57 AM
grokos closed D102890: [libomptarget] Fix a bug whereby firstprivates are not copied over to the device.
Fri, May 21, 10:56 AM · Restricted Project

Thu, May 20

grokos requested review of D102890: [libomptarget] Fix a bug whereby firstprivates are not copied over to the device.
Thu, May 20, 4:48 PM · Restricted Project

May 6 2021

grokos accepted D102000: [libomptarget] Add support for target memory allocators to cuda RTL.

Looks good (with one naming nit)

May 6 2021, 10:50 AM · Restricted Project

Apr 16 2021

grokos added inline comments to D99803: [openmp] Add OMPT initialization in libomptarget.
Apr 16 2021, 3:26 PM · Restricted Project

Apr 15 2021

grokos added inline comments to D100600: [OpenMP] Add info for device table changes.
Apr 15 2021, 3:32 PM · Restricted Project

Apr 13 2021

grokos added inline comments to D99803: [openmp] Add OMPT initialization in libomptarget.
Apr 13 2021, 10:49 AM · Restricted Project

Apr 6 2021

grokos accepted D99551: [clang-offload-wrapper] Add standard notes for ELF offload images.

Change looks good, so it's accepted on my end. I'll let the other reviewers have a look and post their comments. Please do not commit until we have reached an agreement for all 4 patches together (D99551, D99552, D99553, D99612).

Apr 6 2021, 7:19 PM · Restricted Project, Restricted Project, Restricted Project

Mar 25 2021

grokos added a comment to D95483: [OpenMP][WIP] Pass auxiliary YAML along with device image for OpenMP offload..

New implementation is here: https://reviews.llvm.org/D99360

Mar 25 2021, 12:02 PM

Mar 15 2021

grokos added inline comments to D97883: [libomptarget] Add allocator support for target memory.
Mar 15 2021, 11:59 AM · Restricted Project

Mar 13 2021

grokos committed rG2468fdd9af36: [libomptarget] Add allocator support for target memory (authored by grokos).
[libomptarget] Add allocator support for target memory
Mar 13 2021, 3:53 AM
grokos closed D97883: [libomptarget] Add allocator support for target memory.
Mar 13 2021, 3:53 AM · Restricted Project

Mar 12 2021

grokos updated the diff for D97883: [libomptarget] Add allocator support for target memory.

Rabsed and moved targetAllocExplicit() under omptarget.cpp.

Mar 12 2021, 11:21 AM · Restricted Project

Mar 9 2021

grokos updated the summary of D97883: [libomptarget] Add allocator support for target memory.
Mar 9 2021, 11:37 AM · Restricted Project
grokos updated the summary of D97883: [libomptarget] Add allocator support for target memory.
Mar 9 2021, 11:36 AM · Restricted Project
grokos updated the diff for D97883: [libomptarget] Add allocator support for target memory.

Removed __tgt_rtl_data_alloc_explicit; instead we pass an extra kind argument to __tgt_rtl_data_alloc.

Mar 9 2021, 11:33 AM · Restricted Project

Mar 4 2021

grokos accepted D97908: [OpenMP] Encapsulate more in checkDeviceAndCtors.

LGTM, thanks!

Mar 4 2021, 4:00 AM · Restricted Project
grokos accepted D97907: [OpenMP] Fix lone target exit data.

LGTM

Mar 4 2021, 3:54 AM · Restricted Project

Mar 3 2021

grokos requested review of D97883: [libomptarget] Add allocator support for target memory.
Mar 3 2021, 12:33 PM · Restricted Project
grokos added a comment to D97616: [OpenMP] Fix support for device as host.
  • The modified functions all have a common prologue. In a separate patch, I think that most of that should be moved to CheckDeviceAndCtors. Agreed?
Mar 3 2021, 9:02 AM · Restricted Project

Mar 1 2021

grokos accepted D97616: [OpenMP] Fix support for device as host.

OK, clear. You are trying to avoid the call to HandleTargetOutcome(false,...) which erroneously says that offloading has failed. LGTM.

Mar 1 2021, 10:01 AM · Restricted Project
grokos added a comment to D97616: [OpenMP] Fix support for device as host.

This is a correct observation. The host device is not a target device, so there is no offloading available.

Mar 1 2021, 9:16 AM · Restricted Project

Feb 25 2021

grokos accepted D92195: [OPENMP50]Mapping of the subcomponents with the 'default' mappers..

Libomptarget changes look good, I'll let @jdoerfert provide feedback for the clang part.

Feb 25 2021, 2:43 PM · Restricted Project, Restricted Project
grokos added a reviewer for D92195: [OPENMP50]Mapping of the subcomponents with the 'default' mappers.: grokos.
Feb 25 2021, 2:42 PM · Restricted Project, Restricted Project

Feb 24 2021

grokos accepted D97348: [libomptarget] Fixed MSVC build fail caused by __attribute__((used))..

LGTM, thanks!

Feb 24 2021, 5:43 AM · Restricted Project

Feb 18 2021

grokos added a comment to D97003: [Clang][OpenMP] Require CUDA 9.2+ for OpenMP offloading on NVPTX target.

This change makes much sense. In fact, CUDA 8 was so problematic for use with the nvptx runtime that (if memory serves me well) we declared it unsupported. So essentially this patch drops support for CUDA version 7 (and lower), which is already six years old. If the other reviewers agree, we can accept the patch.

Feb 18 2021, 3:38 PM · Restricted Project
grokos accepted D96999: [OpenMP] Fix always,from and delete for data absent at exit.

Is this the missing behavior from D83061? Looks good, thanks!

Feb 18 2021, 3:27 PM · Restricted Project

Feb 16 2021

grokos accepted D96444: [OpenMP][NFC] Use `AsyncInfo` as the variable name for a `__tgt_async_info`.

NFC patch, naming in line with the rest of the library, good to go.

Feb 16 2021, 4:18 PM · Restricted Project

Feb 14 2021

grokos added a comment to D86119: [OPENMP50]Allow overlapping mapping in target constrcuts..

Thanks for the changes, Alexey! I tried the patch locally, and it looks stable. It handled several tests I tried, including the following case involving array section on a pointer to pointer base, and nested mappers with PTR_AND_OBJ maps successfully:

#include <stdio.h>

typedef struct { int a; double *b; } C;
#pragma omp declare mapper(id1: C s) map(to:s.a) map(from:s.b[0:2])

typedef struct { int e; C f; int h; short *g; } D;
#pragma omp declare mapper(default: D r) map(from:r.e) map(mapper(id1), tofrom:r.f) map(tofrom: r.g[0:r.h])

int main() {
  constexpr int N = 10;
  D s;
  s.e = 111;
  s.f.a = 222;
  double x[2]; x[1] = 20;
  short y[N]; y[1] = 30;
  s.f.b = &x[0];
  s.g = &y[0];
  s.h = N;

  D* sp = &s;
  D** spp = &sp;

  printf("%d %d %lf %p %d %p\n", spp[0][0].e, spp[0][0].f.a, spp[0][0].f.b[1], spp[0][0].f.b, spp[0][0].g[1], spp[0][0].g);
  // Expected: 111 222 20.0 <host_addr1> 30 <host_addr2>

  #pragma omp target map(tofrom:spp[0][0])
  {
    printf("%d %d %lf %p %d %p\n", spp[0][0].e, spp[0][0].f.a, spp[0][0].f.b[1], spp[0][0].f.b, spp[0][0].g[1], spp[0][0].g);
    // Expected: <not 111> 222 <not 20.0> <dev_addr1> 30 <dev_addr2>
    spp[0][0].e = 333;
    spp[0][0].f.a = 444;
    spp[0][0].f.b[1] = 40;
    spp[0][0].g[1] = 50;
  }
  printf("%d %d %lf %p %d %p\n", spp[0][0].e, spp[0][0].f.a, spp[0][0].f.b[1], spp[0][0].f.b, spp[0][0].g[1], spp[0][0].g);
  // Expected: 333 222 40.0 <host_addr1> 50 <host_addr2>
}
Feb 14 2021, 3:19 AM · Restricted Project, Restricted Project

Feb 11 2021

grokos added inline comments to D96379: [OpenMP] Unify omptarget API and usage wrt. `__tgt_async_info`.
Feb 11 2021, 7:56 AM · Restricted Project

Feb 10 2021

grokos accepted D96429: [OpenMP][NFC] Pass a DeviceTy, not the device number to `target`.
Feb 10 2021, 4:33 PM · Restricted Project
grokos accepted D96428: [OpenMP][NFC] Clang format libomptarget code (src & include).
Feb 10 2021, 12:57 PM · Restricted Project

Jan 27 2021

grokos added a comment to D95572: [libomptarget][NFC] Link plugins with threads support library due to std::call_once usage..

We need to backport this to 12, right?

Jan 27 2021, 7:03 PM · Restricted Project
grokos accepted D95572: [libomptarget][NFC] Link plugins with threads support library due to std::call_once usage..

I've tested the change and everything seems to work fine. So let's commit it. Thanks for the patch!

Jan 27 2021, 6:26 PM · Restricted Project

Jan 26 2021

grokos added a comment to D95483: [OpenMP][WIP] Pass auxiliary YAML along with device image for OpenMP offload..

My herald rules are very annoying sometimes, apologies. Can we discuss this tomorrow? I put it on the agenda https://docs.google.com/document/d/1Tz8WFN13n7yJ-SCE0Qjqf9LmjGUw0dWO9Ts1ss4YOdg/edit?usp=sharing

Jan 26 2021, 6:52 PM
grokos accepted D95486: [libomptarget][NFC] Avoid gcc 5/6 issue with lambda captures..

LGTM, thanks!

Jan 26 2021, 3:09 PM · Restricted Project
grokos accepted D95476: [libomptarget][NFC] Use portable printf format specifiers..

Looks good, thanks!

Jan 26 2021, 1:31 PM · Restricted Project
grokos retitled D95476: [libomptarget][NFC] Use portable printf format specifiers. from [NFC] Use portable printf format specifiers. to [libomptarget][NFC] Use portable printf format specifiers..
Jan 26 2021, 1:30 PM · Restricted Project
grokos added a comment to D93727: [OpenMP] Add using bit flags to select Libomptarget Information.

I tried inserting the following forward-declaration right before the function definition and the problem seems to go away:

static inline uint32_t getInfoLevel() __attribute__((used));

I guess it's better to have the above rather than a dummy call?

Jan 26 2021, 1:06 PM · Restricted Project
grokos committed rG94cf89d1c2c5: [libomptarget][NFC] Fixed obsolete function names in comments (authored by grokos).
[libomptarget][NFC] Fixed obsolete function names in comments
Jan 26 2021, 7:43 AM
grokos added inline comments to D95282: [OpenMP] Add source location information to the libomptarget profile.
Jan 26 2021, 5:09 AM · Restricted Project

Jan 25 2021

grokos added inline comments to D95415: [libomptarget][cuda] Handle missing _v2 symbols gracefully.
Jan 25 2021, 7:21 PM · Restricted Project

Jan 12 2021

grokos added a comment to D94573: [OpenMP] Drop the static library libomptarget-nvptx.

Also, since D94565 is merged now, this patch needs rebasing :)

Jan 12 2021, 8:03 PM · Restricted Project
grokos added a comment to D94573: [OpenMP] Drop the static library libomptarget-nvptx.

One question is, why do we need two libraries? The only difference is, the static library contains omp_data.cu and the bitcode library doesn't. It's unclear why they were implemented in this way

Jan 12 2021, 7:49 PM · Restricted Project

Jan 5 2021

grokos committed rGdec02904d267: [libomptarget] Allow calls to omp_target_memcpy with 0 size. (authored by grokos).
[libomptarget] Allow calls to omp_target_memcpy with 0 size.
Jan 5 2021, 4:07 PM
grokos closed D94095: [libomptarget] Allow calls to omp_target_memcpy with 0 size..
Jan 5 2021, 4:07 PM · Restricted Project
grokos updated the diff for D94095: [libomptarget] Allow calls to omp_target_memcpy with 0 size..

Slight optimization - removed condition from the hot path.

Jan 5 2021, 12:06 PM · Restricted Project
grokos requested review of D94095: [libomptarget] Allow calls to omp_target_memcpy with 0 size..
Jan 5 2021, 8:39 AM · Restricted Project

Jan 4 2021

grokos accepted D93727: [OpenMP] Add using bit flags to select Libomptarget Information.

Thanks for the changes! LGTM.

Jan 4 2021, 6:46 AM · Restricted Project

Dec 28 2020

grokos added inline comments to D93727: [OpenMP] Add using bit flags to select Libomptarget Information.
Dec 28 2020, 10:07 AM · Restricted Project

Dec 23 2020

grokos added inline comments to D93727: [OpenMP] Add using bit flags to select Libomptarget Information.
Dec 23 2020, 2:27 PM · Restricted Project

Nov 17 2020

grokos added inline comments to D86119: [OPENMP50]Allow overlapping mapping in target constrcuts..
Nov 17 2020, 1:13 PM · Restricted Project, Restricted Project

Nov 6 2020

grokos accepted D82245: [libomptarget] Add support for target update non-contiguous.

LGTM. Thanks!

Nov 6 2020, 4:22 PM · Restricted Project, Restricted Project

Nov 5 2020

grokos added a comment to D82245: [libomptarget] Add support for target update non-contiguous.

The link in the description for the clang patch is outdated (that patch has been abandoned), can you replace it with https://reviews.llvm.org/D84192? Thanks!

Nov 5 2020, 12:27 PM · Restricted Project, Restricted Project

Oct 20 2020

grokos added a comment to D89725: [libomptarget][amdgcn] Implement missing symbols in deviceRTL.

Can we now abandon D75581?

Oct 20 2020, 2:24 AM · Restricted Project

Oct 16 2020

grokos committed rG5adb3a6d86ee: [libomptarget] Fix copy-to motion for PTR_AND_OBJ entries where PTR is a struct… (authored by grokos).
[libomptarget] Fix copy-to motion for PTR_AND_OBJ entries where PTR is a struct…
Oct 16 2020, 4:17 PM
grokos closed D89597: [libomptarget] Fix copy-to motion for PTR_AND_OBJ entries where PTR is a struct member.
Oct 16 2020, 4:17 PM · Restricted Project
grokos requested review of D89597: [libomptarget] Fix copy-to motion for PTR_AND_OBJ entries where PTR is a struct member.
Oct 16 2020, 3:01 PM · Restricted Project

Oct 12 2020

grokos added a comment to D87946: [OpenMP] Add Location Fields to Libomptarget Runtime for Debugging.

Current build, fails offloading/target_depend_nowait for an unknown reason after calling cuStreamSynchronize in __tgt_target_teams_mapper_nowait.

Oct 12 2020, 1:21 PM · Restricted Project, Restricted Project, Restricted Project

Oct 5 2020

grokos added a comment to D88829: [OpenMP][RTL] Remove dead code.

Rolling the reduction in leading whitespace in nvptx_target_parallel_reduction_codegen.cpp in with the patch might be contentious, added a couple more reviewers to see if other people would prefer that part split out. I'll accept in a day or so if there are no comments on the whitespace.

Oct 5 2020, 6:46 AM · Restricted Project, Restricted Project
grokos added a comment to D85274: [OpenMP] Introduced a bump-like allocator into the target memory management.

I just revisited this patch. It seems it's based on a very early implementation of the base memory manager patch (D81054). Can you rebase the patch so that we review it? Thanks!

Oct 5 2020, 4:45 AM · Restricted Project

Sep 29 2020

grokos added a comment to D87259: [OpenMPOpt][SplitMemTransfer] Implementation of the real issue and wait api functions..

I think these API functions should also include the source location pointer from https://reviews.llvm.org/D87946. We need to consider renaming the *_issue and *_wait functions to extend the *_loc API the aforementioned patch is introducing. E.g. after D87946 the "current" data begin API function will be __tgt_target_data_begin_mapper_loc, so this patch should extend that name as __tgt_target_data_begin_mapper_loc_issue and __tgt_target_data_begin_mapper_loc_wait. Because both patches make changes to the API, I think it's better to wait until the former patch has been committed.

Sep 29 2020, 9:07 AM · Restricted Project
grokos added inline comments to D87946: [OpenMP] Add Location Fields to Libomptarget Runtime for Debugging.
Sep 29 2020, 5:37 AM · Restricted Project, Restricted Project, Restricted Project

Sep 25 2020

grokos accepted D88149: [OpenMP][libomptarget] make omp_get_initial_device 5.1 compliant.

LGTM as well.

Sep 25 2020, 2:53 PM · Restricted Project

Sep 23 2020

grokos added inline comments to D88149: [OpenMP][libomptarget] make omp_get_initial_device 5.1 compliant.
Sep 23 2020, 6:19 AM · Restricted Project
grokos added a reviewer for D87946: [OpenMP] Add Location Fields to Libomptarget Runtime for Debugging: grokos.
Sep 23 2020, 6:13 AM · Restricted Project, Restricted Project, Restricted Project
grokos added a comment to D87946: [OpenMP] Add Location Fields to Libomptarget Runtime for Debugging.

Seems like a hacky solution to just keep adding suffixed whenever we want a new interface though.

Sep 23 2020, 6:13 AM · Restricted Project, Restricted Project, Restricted Project

Sep 21 2020

grokos added a comment to D87946: [OpenMP] Add Location Fields to Libomptarget Runtime for Debugging.

I wasn't aware they were explicitly deprecated. If we're keeping around old interfaces for backwards compatibility I should also add in the old mapper functions without the ident_t pointer and call into the new functions with a nullptr.

Sep 21 2020, 4:25 PM · Restricted Project, Restricted Project, Restricted Project
grokos added a comment to D87946: [OpenMP] Add Location Fields to Libomptarget Runtime for Debugging.

Added ident_t structs to additional runtime functions.

Sep 21 2020, 3:56 PM · Restricted Project, Restricted Project, Restricted Project

Sep 18 2020

grokos added a comment to D87841: [openmp][libomptarget] Include header from LLVM source tree.

As openmp and llvm are now developed in the same monorepo, the only uses this
dependency can break are those that download the monorepo, then delete the llvm
directory before calling cmake. That is not a compelling use case.

That is factually not accurate.
There are standalone tarballs, including one for omp:
https://releases.llvm.org/download.html#10.0.1 OpenMP Source code (.sig)

Right. You can download the source for the sub-projects.
Though, others, e.g., clang, are already not compileable in isolation.

I see three options here:

  1. Continue to copy code from llvm-core to openmp and back. This is "good" for people that compile libomp in isolation and "bad" for developers. Arguably, it is also "bad" for people that use libomp with clang because there is an increased risk we have an unnoticed mismatch between the two. This is not theoretical but already happened.
  2. Copy the code from llvm-core to openmp during packaging time. This is a (small?) burden on the package maintainers but has all the advantages for now. However, it will get more complicated the more llvm-core features we want to use, e.g., tablegen.
  3. Require users to download llvm-core and point to it. This is a small burden on the users but has all the advantages now and in the future.

This patch, on its own, picks 3). I'm happy with that.

Sep 18 2020, 4:23 AM · Restricted Project, Restricted Project

Sep 1 2020

grokos added inline comments to D86804: [OpenMP] Consolidate error handling and debug messages in Libomptarget.
Sep 1 2020, 8:26 AM · Restricted Project
grokos added a comment to D86307: [OpenMP] Pack first-private arguments to improve efficiency of data transfer.

Minor comments (about typos) to be taken into account in case of a future patch.

Sep 1 2020, 7:40 AM · Restricted Project

Aug 28 2020

grokos accepted D86781: [LIBOMPTARGET]Do not try to optimize bases for the next parameters..

Looks good!

Aug 28 2020, 7:12 AM · Restricted Project

Aug 17 2020

grokos committed rG32ebdc70f3af: [libomptarget][NFC] Sort list of plugins in chronological order (authored by grokos).
[libomptarget][NFC] Sort list of plugins in chronological order
Aug 17 2020, 8:37 AM
grokos closed D86082: [libomptarget][NFC] Sort list of plugins in chronological order.
Aug 17 2020, 8:36 AM · Restricted Project
grokos requested review of D86082: [libomptarget][NFC] Sort list of plugins in chronological order.
Aug 17 2020, 8:29 AM · Restricted Project

Aug 6 2020

grokos accepted D85369: [OpenMP] Fix ref count dec for implicit map of partial data.

Nice catch! Looks good.

Aug 6 2020, 12:24 AM · Restricted Project

Aug 5 2020

grokos committed rG40470eb27a5c: [libomptarget][NFC] Replace `%ld` with PRId64 for data of type int64_t. (authored by grokos).
[libomptarget][NFC] Replace `%ld` with PRId64 for data of type int64_t.
Aug 5 2020, 1:32 PM
grokos closed D85353: [libomptarget][NFC] Replace `%ld` with PRId64 for data of type int64_t..
Aug 5 2020, 1:31 PM · Restricted Project
grokos accepted D85320: [OpenMP] Fix `present` diagnostic for array extension.

LGTM. Thanks!

Aug 5 2020, 1:19 PM · Restricted Project
grokos requested review of D85353: [libomptarget][NFC] Replace `%ld` with PRId64 for data of type int64_t..
Aug 5 2020, 1:17 PM · Restricted Project
grokos accepted D85342: [OpenMP] Fix `target data` exit for array extension.

Looks good.

Aug 5 2020, 12:51 PM · Restricted Project
grokos added inline comments to D85320: [OpenMP] Fix `present` diagnostic for array extension.
Aug 5 2020, 12:10 PM · Restricted Project
grokos added inline comments to D85320: [OpenMP] Fix `present` diagnostic for array extension.
Aug 5 2020, 11:19 AM · Restricted Project

Aug 4 2020

grokos accepted D85216: [LIBOMPTARGET]Fix order of mapper data for targetDataEnd function..

OK, now it makes sense. LGTM

Aug 4 2020, 3:42 PM · Restricted Project
grokos accepted D85246: [OpenMP] Fix `omp target update` for array extension.

I suppose the same must be applied to all cases where present is used. Without a modification like this, present just confirms that the starting address of the object is already mapped, although a size may be specified in the map clause.

#pragma omp target map(present: p[0:10]) // present will succeed even if only p[0] is mapped and p[1]-p[9] are not

So the question here is: should present apply to the size as well as the begin address? I would say yes.

Aug 4 2020, 3:06 PM · Restricted Project
grokos added inline comments to D82245: [libomptarget] Add support for target update non-contiguous.
Aug 4 2020, 12:19 PM · Restricted Project, Restricted Project
grokos added a comment to D85216: [LIBOMPTARGET]Fix order of mapper data for targetDataEnd function..

So does the mapper function emit entries in reverse order upon exiting a target/target data region?

Aug 4 2020, 10:35 AM · Restricted Project

Aug 3 2020

grokos added a comment to D68100: [OpenMP 5.0] declare mapper runtime implementation.

This is an optimization brought up by Deepak. I guess you were in that meeting too but forgot. It could be quite useful when you map an array of struct/class. Assume you map 1000 of this structure, with this optimization most memory allocation can be done in a single allocation, instead of allocation 12 bytes memory 1000 times.

Thinking about it, it's actually important for correctness too. Assume you map C a[2]. If you map separately, a[0] and a[1] could be mapped to not contiguous locations, and it will cause error/segfault when GPU kernel access this array. If you allocate the whole array a[2] together, such problem won't happen.

Aug 3 2020, 6:15 PM · Restricted Project

Jul 29 2020

grokos added inline comments to D84767: [OPENMP]Fix PR46824: Global declare target pointer cannot be accessed in target region..
Jul 29 2020, 11:27 AM · Restricted Project, Restricted Project

Jul 27 2020

grokos added a comment to D83963: [OpenMP] Use `abort` not `error` for fatal runtime exceptions.

Thanks, it makes sense. Can you submit a patch?

Jul 27 2020, 7:54 PM · Restricted Project
grokos added a comment to D83963: [OpenMP] Use `abort` not `error` for fatal runtime exceptions.

Any idea why this change broke some tests? And how does --crash fix the problem?

Jul 27 2020, 6:42 PM · Restricted Project
grokos added a comment to D84422: [OpenMP] Fix `present` for exit from `omp target data`.

This looks much better now. I don't have any other comments. Since this patch is now essentially a clang-only patch, I'll let @ABataev accept it or post comments.

Jul 27 2020, 6:33 PM · Restricted Project, Restricted Project, Restricted Project
grokos accepted D84712: [OpenMP] Implement TR8 `present` motion modifier in runtime (2/2).
Jul 27 2020, 6:29 PM · Restricted Project
grokos added a comment to D84712: [OpenMP] Implement TR8 `present` motion modifier in runtime (2/2).

(Earlier I posted here an answer for another patch). Looks good, obviously it must be committed after the clang patch.

Jul 27 2020, 6:28 PM · Restricted Project
grokos added a comment to D84422: [OpenMP] Fix `present` for exit from `omp target data`.

I've added a comment to the runtime code that performs the check. As you can see, the check is performed regardless. It's just a question of whether the runtime treats it as an error. I don't think performance is an issue.

My concern here is that it will be hard to justify changes to the runtime if I cannot formulate a use case.

Jul 27 2020, 12:28 PM · Restricted Project, Restricted Project, Restricted Project