Page MenuHomePhabricator

jdenny (Joel E. Denny)
User

Projects

User does not belong to any projects.

User Details

User Since
Nov 2 2017, 3:15 PM (150 w, 3 d)

Recent Activity

Aug 20 2020

jdenny added a comment to D85692: python bindings: fix DeprecationWarning.

Does this mean we officially no longer support python 2, which this change breaks?

Send a patch to support both and I'll approve it.

Aug 20 2020, 10:42 AM · Restricted Project

Aug 19 2020

jdenny added a comment to D85692: python bindings: fix DeprecationWarning.

Does this mean we officially no longer support python 2, which this change breaks?

Aug 19 2020, 6:24 PM · Restricted Project

Aug 11 2020

jdenny added inline comments to D81667: [FileCheck] Add precision to format specifier.
Aug 11 2020, 9:26 AM · Restricted Project

Aug 6 2020

jdenny added a comment to D85369: [OpenMP] Fix ref count dec for implicit map of partial data.

Thanks!

Aug 6 2020, 8:41 AM · Restricted Project
jdenny committed rG518a27e5591c: [OpenMP] Fix ref count dec for implicit map of partial data (authored by jdenny).
[OpenMP] Fix ref count dec for implicit map of partial data
Aug 6 2020, 8:40 AM
jdenny closed D85369: [OpenMP] Fix ref count dec for implicit map of partial data.
Aug 6 2020, 8:39 AM · Restricted Project

Aug 5 2020

jdenny updated the diff for D85369: [OpenMP] Fix ref count dec for implicit map of partial data.

Don't examine LIBOMPTARGET_DEBUG=1 output in the test. It doesn't work in release builds.

Aug 5 2020, 5:20 PM · Restricted Project
jdenny requested review of D85369: [OpenMP] Fix ref count dec for implicit map of partial data.
Aug 5 2020, 3:44 PM · Restricted Project
jdenny added inline comments to D85342: [OpenMP] Fix `target data` exit for array extension.
Aug 5 2020, 2:34 PM · Restricted Project
jdenny committed rG8c8bb128dfd0: [OpenMP] Fix `target data` exit for array extension (authored by jdenny).
[OpenMP] Fix `target data` exit for array extension
Aug 5 2020, 1:52 PM
jdenny committed rG41b1aefecb94: [OpenMP] Fix `present` diagnostic for array extension (authored by jdenny).
[OpenMP] Fix `present` diagnostic for array extension
Aug 5 2020, 1:52 PM
jdenny closed D85342: [OpenMP] Fix `target data` exit for array extension.
Aug 5 2020, 1:52 PM · Restricted Project
jdenny closed D85320: [OpenMP] Fix `present` diagnostic for array extension.
Aug 5 2020, 1:51 PM · Restricted Project
jdenny added a comment to D85320: [OpenMP] Fix `present` diagnostic for array extension.

Thanks for the quick reviews!

Aug 5 2020, 1:23 PM · Restricted Project
jdenny updated the diff for D85320: [OpenMP] Fix `present` diagnostic for array extension.

Add user diagnostic for mapping extension. Use PRId64 where appropriate.

Aug 5 2020, 1:17 PM · Restricted Project
jdenny requested review of D85342: [OpenMP] Fix `target data` exit for array extension.
Aug 5 2020, 12:30 PM · Restricted Project
jdenny added inline comments to D85320: [OpenMP] Fix `present` diagnostic for array extension.
Aug 5 2020, 12:13 PM · Restricted Project
jdenny added inline comments to D85320: [OpenMP] Fix `present` diagnostic for array extension.
Aug 5 2020, 12:04 PM · Restricted Project
jdenny requested review of D85320: [OpenMP] Fix `present` diagnostic for array extension.
Aug 5 2020, 8:45 AM · Restricted Project
jdenny committed rG5ab43989c353: [OpenMP] Fix `omp target update` for array extension (authored by jdenny).
[OpenMP] Fix `omp target update` for array extension
Aug 5 2020, 7:04 AM
jdenny committed rG03bb545b68c2: [OpenMP][Docs] Mark `present` map type modifier as done (authored by jdenny).
[OpenMP][Docs] Mark `present` map type modifier as done
Aug 5 2020, 7:04 AM
jdenny committed rG002d61db2b77: [OpenMP] Fix `present` for exit from `omp target data` (authored by jdenny).
[OpenMP] Fix `present` for exit from `omp target data`
Aug 5 2020, 7:04 AM
jdenny committed rG26cf9c170445: [OpenMP][Docs] Add map clause reordering status as unclaimed (authored by jdenny).
[OpenMP][Docs] Add map clause reordering status as unclaimed
Aug 5 2020, 7:04 AM
jdenny closed D85246: [OpenMP] Fix `omp target update` for array extension.
Aug 5 2020, 7:04 AM · Restricted Project
jdenny closed D84422: [OpenMP] Fix `present` for exit from `omp target data`.
Aug 5 2020, 7:04 AM · Restricted Project, Restricted Project, Restricted Project

Aug 4 2020

jdenny added a comment to D85246: [OpenMP] Fix `omp target update` for array extension.

This patch fixes DeviceTy::getTgtPtrBegin to return null when
target_data_update is the caller. I'm wondering if it should do the
same for more callers.

Aug 4 2020, 5:40 PM · Restricted Project
jdenny added a comment to D85246: [OpenMP] Fix `omp target update` for array extension.

Maybe change the commit title to OpenMP to confuse less people.

Aug 4 2020, 4:57 PM · Restricted Project
jdenny retitled D85246: [OpenMP] Fix `omp target update` for array extension from [OpenACC] Fix `omp target update` for array extension to [OpenMP] Fix `omp target update` for array extension.
Aug 4 2020, 4:56 PM · Restricted Project
jdenny added a comment to D85246: [OpenMP] Fix `omp target update` for array extension.

Thanks for the quick review.

Aug 4 2020, 3:49 PM · Restricted Project
jdenny added reviewers for D85246: [OpenMP] Fix `omp target update` for array extension: grokos, RaviNarayanaswamy.
Aug 4 2020, 2:07 PM · Restricted Project
jdenny requested review of D85246: [OpenMP] Fix `omp target update` for array extension.
Aug 4 2020, 2:06 PM · Restricted Project

Jul 30 2020

jdenny committed rG3d06fc0049c6: [OpenMP][Docs] Mark `present` motion modifier as done (authored by jdenny).
[OpenMP][Docs] Mark `present` motion modifier as done
Jul 30 2020, 9:23 AM
jdenny added a comment to D84422: [OpenMP] Fix `present` for exit from `omp target data`.

Thanks for the review.

Jul 30 2020, 8:56 AM · Restricted Project, Restricted Project, Restricted Project

Jul 29 2020

jdenny added inline comments to D84711: [OpenMP] Implement TR8 `present` motion modifier in Clang (1/2).
Jul 29 2020, 9:33 AM · Restricted Project
jdenny committed rGcee52dd02672: [OpenMP] Implement TR8 `present` motion modifier in runtime (2/2) (authored by jdenny).
[OpenMP] Implement TR8 `present` motion modifier in runtime (2/2)
Jul 29 2020, 9:21 AM
jdenny committed rG9f2f3b9de631: [OpenMP] Implement TR8 `present` motion modifier in Clang (1/2) (authored by jdenny).
[OpenMP] Implement TR8 `present` motion modifier in Clang (1/2)
Jul 29 2020, 9:21 AM

Jul 28 2020

jdenny added inline comments to D84711: [OpenMP] Implement TR8 `present` motion modifier in Clang (1/2).
Jul 28 2020, 5:54 PM · Restricted Project
jdenny added a reverting change for rG3c3faae49704: [OpenMP] Implement TR8 `present` motion modifier in Clang (1/2): rG69fc33f0cd13: Revert "[OpenMP] Implement TR8 `present` motion modifier in Clang (1/2)".
Jul 28 2020, 5:30 PM
jdenny committed rG69fc33f0cd13: Revert "[OpenMP] Implement TR8 `present` motion modifier in Clang (1/2)" (authored by jdenny).
Revert "[OpenMP] Implement TR8 `present` motion modifier in Clang (1/2)"
Jul 28 2020, 5:30 PM
jdenny added a reverting change for rG2cb926a447d2: [OpenMP] Implement TR8 `present` motion modifier in runtime (2/2): rG65564e5eaf14: Revert "[OpenMP] Implement TR8 `present` motion modifier in runtime (2/2)".
Jul 28 2020, 5:30 PM
jdenny added a reverting change for D84711: [OpenMP] Implement TR8 `present` motion modifier in Clang (1/2): rG69fc33f0cd13: Revert "[OpenMP] Implement TR8 `present` motion modifier in Clang (1/2)".
Jul 28 2020, 5:30 PM · Restricted Project
jdenny committed rG65564e5eaf14: Revert "[OpenMP] Implement TR8 `present` motion modifier in runtime (2/2)" (authored by jdenny).
Revert "[OpenMP] Implement TR8 `present` motion modifier in runtime (2/2)"
Jul 28 2020, 5:30 PM
jdenny added a reverting change for D84712: [OpenMP] Implement TR8 `present` motion modifier in runtime (2/2): rG65564e5eaf14: Revert "[OpenMP] Implement TR8 `present` motion modifier in runtime (2/2)".
Jul 28 2020, 5:30 PM · Restricted Project
jdenny added inline comments to D83650: [FileCheck] Extend -dump-input with substitutions.
Jul 28 2020, 4:38 PM · Restricted Project
jdenny committed rG9f86b8ec41f0: [FileCheck] Report captured variables (authored by jdenny).
[FileCheck] Report captured variables
Jul 28 2020, 4:16 PM
jdenny committed rG2cb926a447d2: [OpenMP] Implement TR8 `present` motion modifier in runtime (2/2) (authored by jdenny).
[OpenMP] Implement TR8 `present` motion modifier in runtime (2/2)
Jul 28 2020, 4:16 PM
jdenny committed rGd680711b94e9: [FileCheck] Extend -dump-input with substitutions (authored by jdenny).
[FileCheck] Extend -dump-input with substitutions
Jul 28 2020, 4:16 PM
jdenny committed rG3c3faae49704: [OpenMP] Implement TR8 `present` motion modifier in Clang (1/2) (authored by jdenny).
[OpenMP] Implement TR8 `present` motion modifier in Clang (1/2)
Jul 28 2020, 4:16 PM
jdenny closed D83651: [FileCheck] Report captured variables.
Jul 28 2020, 4:16 PM · Restricted Project
jdenny closed D83650: [FileCheck] Extend -dump-input with substitutions.
Jul 28 2020, 4:16 PM · Restricted Project
jdenny committed rGa3d1f88fa7da: [OpenMP][NFC] Consolidate `to` and `from` clause modifiers (authored by jdenny).
[OpenMP][NFC] Consolidate `to` and `from` clause modifiers
Jul 28 2020, 4:16 PM
jdenny closed D84712: [OpenMP] Implement TR8 `present` motion modifier in runtime (2/2).
Jul 28 2020, 4:16 PM · Restricted Project
jdenny closed D84711: [OpenMP] Implement TR8 `present` motion modifier in Clang (1/2).
Jul 28 2020, 4:16 PM · Restricted Project
jdenny closed D84710: [OpenMP][NFC] Consolidate `to` and `from` clause modifiers.
Jul 28 2020, 4:16 PM · Restricted Project
jdenny added a comment to D83650: [FileCheck] Extend -dump-input with substitutions.

Thanks for the review.

Jul 28 2020, 2:26 PM · Restricted Project
jdenny updated the diff for D83650: [FileCheck] Extend -dump-input with substitutions.

Adjust the fixme as requested.

Jul 28 2020, 2:24 PM · Restricted Project
jdenny added inline comments to D84422: [OpenMP] Fix `present` for exit from `omp target data`.
Jul 28 2020, 1:54 PM · Restricted Project, Restricted Project, Restricted Project
jdenny added a comment to D84711: [OpenMP] Implement TR8 `present` motion modifier in Clang (1/2).

Thanks!

Jul 28 2020, 1:47 PM · Restricted Project
jdenny accepted D84612: [openmp][openacc][NFC] Add wrapper for records in DirectiveEmitter.

Should this patch be marked NFC?

Jul 28 2020, 1:45 PM · Restricted Project
jdenny added inline comments to D84711: [OpenMP] Implement TR8 `present` motion modifier in Clang (1/2).
Jul 28 2020, 1:17 PM · Restricted Project
jdenny updated the diff for D84711: [OpenMP] Implement TR8 `present` motion modifier in Clang (1/2).

Adjusted logic for rejecting present as requested. Rebased.

Jul 28 2020, 1:16 PM · Restricted Project
jdenny added inline comments to D84711: [OpenMP] Implement TR8 `present` motion modifier in Clang (1/2).
Jul 28 2020, 12:49 PM · Restricted Project
jdenny added a comment to D84710: [OpenMP][NFC] Consolidate `to` and `from` clause modifiers.

Thanks!

Jul 28 2020, 12:43 PM · Restricted Project
jdenny updated the diff for D84422: [OpenMP] Fix `present` for exit from `omp target data`.

Replaced SeparateBeginEnd parameter with new TargetDataInfo field as requested. Rebased.

Jul 28 2020, 12:41 PM · Restricted Project, Restricted Project, Restricted Project
jdenny added a comment to D84612: [openmp][openacc][NFC] Add wrapper for records in DirectiveEmitter.

Are you following an existing TableGen emitter as a precedent for this kind of thin wrapper design?

Jul 28 2020, 9:14 AM · Restricted Project
jdenny added a comment to D83650: [FileCheck] Extend -dump-input with substitutions.

Sure. Thanks for the reply.

Jul 28 2020, 6:30 AM · Restricted Project
jdenny added a comment to D84712: [OpenMP] Implement TR8 `present` motion modifier in runtime (2/2).

Thanks!

Jul 28 2020, 6:27 AM · Restricted Project
jdenny added a comment to D84557: [OpenMP][Tests] Enable nvptx64 testing for most libomptarget tests.

Broken because of D83963, @jdenny proposed a fix, which works for me:

libomptarget :: mapping/alloc_fail.c
libomptarget :: mapping/present/target.c
libomptarget :: mapping/present/target_data.c
libomptarget :: mapping/present/target_enter_data.c
libomptarget :: mapping/present/target_exit_data.c
libomptarget :: mapping/present/zero_length_array_section.c
libomptarget :: mapping/present/zero_length_array_section_exit.c
Jul 28 2020, 6:09 AM · Restricted Project
jdenny added a comment to D83963: [OpenMP] Use `abort` not `error` for fatal runtime exceptions.

I've pushed it to master as 9b4826d18b5f.

Jul 28 2020, 6:03 AM · Restricted Project
jdenny committed rG9b4826d18b5f: [OpenMP] Fix libomptarget negative tests to expect abort (authored by jdenny).
[OpenMP] Fix libomptarget negative tests to expect abort
Jul 28 2020, 6:02 AM

Jul 27 2020

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

not expects a non-zero exit not a signal. Adding --crash adjusts the expectation.

Jul 27 2020, 6:54 PM · Restricted Project
jdenny added a comment to D83650: [FileCheck] Extend -dump-input with substitutions.

Ping.

Jul 27 2020, 4:28 PM · Restricted Project
jdenny committed rGf250eb37cd4f: [OpenMP][Docs] Update `present` modifier status (authored by jdenny).
[OpenMP][Docs] Update `present` modifier status
Jul 27 2020, 4:24 PM
Herald added a project to D84712: [OpenMP] Implement TR8 `present` motion modifier in runtime (2/2): Restricted Project.
Jul 27 2020, 4:10 PM · Restricted Project
Herald added a project to D84711: [OpenMP] Implement TR8 `present` motion modifier in Clang (1/2): Restricted Project.
Jul 27 2020, 4:10 PM · Restricted Project
Herald added a project to D84710: [OpenMP][NFC] Consolidate `to` and `from` clause modifiers: Restricted Project.
Jul 27 2020, 4:10 PM · Restricted Project
jdenny updated the summary of D84422: [OpenMP] Fix `present` for exit from `omp target data`.
Jul 27 2020, 3:40 PM · Restricted Project, Restricted Project, Restricted Project
jdenny updated the diff for D84422: [OpenMP] Fix `present` for exit from `omp target data`.

Rewrite patch as discussed: instead of generating different runtime calls for the end of an omp target data vs. the beginning of an omp target exit data so that the runtime can determine when to ignore present, change Clang to filter present from the map types at the end of an omp target data.

Jul 27 2020, 3:39 PM · Restricted Project, Restricted Project, Restricted Project
jdenny 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.

Thinking about it, I don't think there can be a case where something is present upon entering a target region and not be present when we're exiting. Whatever code comprises the target region is code executed on the device - it cannot modify the state of host objects (i.e. libomptarget) in any possible way. E.g. the kernel cannot invoke libomptarget functions, allocate memory, map/unmap data etc.

The only case where something like this would be possible is if we have multiple host threads executing async offloading. In such a case, one thread may launch a target region at a moment when the requested mapping is present on the device and while the kernel is executing some other thread performs a target data exit on the desired mapping. Upon exiting the kernel, the mapping will no longer be present but this is clearly a race condition (user's fault), so I don't think we should pay attention to such a scenario.

Jul 27 2020, 12:40 PM · Restricted Project, Restricted Project, Restricted Project
jdenny accepted D84233: [lit] Remove ANSI control character from xunit output.

Other than the nit I just added about comment duplication, this LGTM. Thanks. However, let's a wait a couple of days in case anyone else has an opinion.

Jul 27 2020, 10:27 AM · Restricted Project
jdenny added a comment to D83963: [OpenMP] Use `abort` not `error` for fatal runtime exceptions.

This change broke a number of libomptarget tests. Adding --crash to the %libomptarget-run-fail-* definitions in openmp/libomptarget/test/lit.cfg fixes that for me. Any objections?

Jul 27 2020, 8:17 AM · Restricted Project

Jul 24 2020

jdenny added a comment to D84422: [OpenMP] Fix `present` for exit from `omp target data`.

Has anyone clarified the motivation for this behavior?

I meant, is there any insight into why the spec specifies this behavior?

Instead of introducing new API functions and making all these changes in all these files, wouldn't it be easier if we just unset the PRESENT flag from arg_types in clang when we generate the call to __tgt_target_data_end_* if we are exiting from a scoped environment?

Ah, that does sound simpler. Thanks. I'll look into it.

Suppressing the presence check on exit from omp target would require a runtime change in addition to the Clang change you suggest for omp target data. However, I've so far failed to formulate a reasonable test case. Specifically, I don't yet see a way to guarantee that the data will definitely be present at the start of omp target but might not be present by the end. Is it possible? If not, then maybe we should leave the check in place for omp target.

I would rather not have a check if not required by the spec as it would an unnecessary overhead to performance.

Jul 24 2020, 3:34 PM · Restricted Project, Restricted Project, Restricted Project
jdenny added a comment to D84422: [OpenMP] Fix `present` for exit from `omp target data`.

Has anyone clarified the motivation for this behavior?

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

So is the test case that motivated this patch illegal OpenMP code?

#pragma omp target enter data map(alloc:i)
#pragma omp target data map(present, alloc: i)
{
  #pragma omp target exit data map(delete:i) // you cannot delete that object in the scope, illegal code?
} // fails presence check here

According to spec the test should work. ie should not check for presence on exit from a blocked openmp pragma scope.

Jul 24 2020, 11:38 AM · Restricted Project, Restricted Project, Restricted Project
jdenny added a comment to D84233: [lit] Remove ANSI control character from xunit output.

Thanks for working on this. I've been wanting to see this fixed too. I agree that lit is the culprit.

Jul 24 2020, 11:04 AM · Restricted Project
jdenny accepted D84329: [openmp] Clean up OMPKinds.def remove OMP_DIRECTIVE.

LGTM

Jul 24 2020, 9:25 AM · Restricted Project

Jul 23 2020

jdenny added a comment to D84422: [OpenMP] Fix `present` for exit from `omp target data`.

I don't know if the OpenMP committee has any documented rationale for this behavior. I can say that the OpenACC committee is considering the same semantics. However, the issues to consider are not identical. For example, OpenACC has a separate structured reference counter, meaning it should be impossible for such data not to be present at the exit of a data construct unless you've shut down the runtime.

Jul 23 2020, 1:01 PM · Restricted Project, Restricted Project, Restricted Project
Herald added projects to D84422: [OpenMP] Fix `present` for exit from `omp target data`: Restricted Project, Restricted Project, Restricted Project.
Jul 23 2020, 9:06 AM · Restricted Project, Restricted Project, Restricted Project

Jul 22 2020

jdenny committed rG708752b2f6c5: [OpenMP] Implement TR8 `present` map type modifier in runtime (2/2) (authored by jdenny).
[OpenMP] Implement TR8 `present` map type modifier in runtime (2/2)
Jul 22 2020, 11:09 AM
jdenny committed rGfc247c8f3c61: Revert "[OpenMP] Implement TR8 `present` map type modifier in runtime (2/2)" (authored by jdenny).
Revert "[OpenMP] Implement TR8 `present` map type modifier in runtime (2/2)"
Jul 22 2020, 8:24 AM
jdenny added a reverting change for rG45b8f7ec35ef: [OpenMP] Implement TR8 `present` map type modifier in runtime (2/2): rGfc247c8f3c61: Revert "[OpenMP] Implement TR8 `present` map type modifier in runtime (2/2)".
Jul 22 2020, 8:24 AM
jdenny added inline comments to D83062: [OpenMP] Implement TR8 `present` map type modifier in runtime (2/2).
Jul 22 2020, 8:15 AM · Restricted Project
jdenny committed rG45b8f7ec35ef: [OpenMP] Implement TR8 `present` map type modifier in runtime (2/2) (authored by jdenny).
[OpenMP] Implement TR8 `present` map type modifier in runtime (2/2)
Jul 22 2020, 7:16 AM
jdenny committed rGaa82c40f0a0a: [OpenMP] Implement TR8 `present` map type modifier in Clang (1/2) (authored by jdenny).
[OpenMP] Implement TR8 `present` map type modifier in Clang (1/2)
Jul 22 2020, 7:15 AM
jdenny closed D83062: [OpenMP] Implement TR8 `present` map type modifier in runtime (2/2).
Jul 22 2020, 7:15 AM · Restricted Project
jdenny closed D83061: [OpenMP] Implement TR8 `present` map type modifier in Clang (1/2).
Jul 22 2020, 7:15 AM · Restricted Project

Jul 21 2020

jdenny updated the diff for D83061: [OpenMP] Implement TR8 `present` map type modifier in Clang (1/2).

Rebased. Due to D84182, updated the map types expected by my new tests in clang/test/OpenMP/target_data_codegen.cpp.

Jul 21 2020, 5:58 PM · Restricted Project
jdenny added a comment to D83062: [OpenMP] Implement TR8 `present` map type modifier in runtime (2/2).

Thanks again for the review.

Jul 21 2020, 3:12 PM · Restricted Project
jdenny added inline comments to D83062: [OpenMP] Implement TR8 `present` map type modifier in runtime (2/2).
Jul 21 2020, 2:57 PM · Restricted Project
jdenny updated the diff for D83062: [OpenMP] Implement TR8 `present` map type modifier in runtime (2/2).

Tried to fix comments on PTR_AND_OBJ.

Jul 21 2020, 2:56 PM · Restricted Project