Page MenuHomePhabricator

JonChesterfield (Jon Chesterfield)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 11 2015, 1:08 PM (266 w, 5 d)

Recent Activity

Fri, Sep 18

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

An additional repo which llvm, openmp, clang etc implicitly depend on would also work. It's roughly equivalent to treating llvm itself as that root dependency.

Fri, Sep 18, 11:31 AM · Restricted Project, Restricted Project
JonChesterfield committed rGa9be2b5cb2b3: [libomptarget] Disable build of amdgpu plugin as it doesn't build with rocm. (authored by JonChesterfield).
[libomptarget] Disable build of amdgpu plugin as it doesn't build with rocm.
Fri, Sep 18, 10:10 AM
JonChesterfield added a comment to D87841: [openmp][libomptarget] Include header from LLVM source tree.

I strongly dislike build scripts pulling files off the internet. It's bad for machines that aren't connected to the web and upsets people in security

Fri, Sep 18, 9:38 AM · Restricted Project, Restricted Project

Thu, Sep 17

JonChesterfield added inline comments to D87841: [openmp][libomptarget] Include header from LLVM source tree.
Thu, Sep 17, 6:40 PM · Restricted Project, Restricted Project
JonChesterfield updated the summary of D87841: [openmp][libomptarget] Include header from LLVM source tree.
Thu, Sep 17, 11:01 AM · Restricted Project, Restricted Project
JonChesterfield added a comment to D87841: [openmp][libomptarget] Include header from LLVM source tree.

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)

Thu, Sep 17, 10:59 AM · Restricted Project, Restricted Project
JonChesterfield updated subscribers of D87841: [openmp][libomptarget] Include header from LLVM source tree.
Thu, Sep 17, 10:20 AM · Restricted Project, Restricted Project
JonChesterfield requested review of D87841: [openmp][libomptarget] Include header from LLVM source tree.
Thu, Sep 17, 10:16 AM · Restricted Project, Restricted Project

Thu, Sep 3

JonChesterfield accepted D87084: [OpenMP][AMDGPU] Use DS_Max_Warp_Number instead of WARPSIZE.

It might be dead. Difficult to tell. The control flow being spread across codegen and the runtime obfuscates things.

Thu, Sep 3, 4:09 PM · Restricted Project
JonChesterfield added a comment to D87084: [OpenMP][AMDGPU] Use DS_Max_Warp_Number instead of WARPSIZE.

This can't break nvptx as WARPSIZE == DS_Max_Warp_Number.

Thu, Sep 3, 7:51 AM · Restricted Project

Fri, Aug 28

JonChesterfield added a comment to D86804: [OpenMP] Consolidate error handling and debug messages in Libomptarget.

Nice! Thank you, there is way too much copy and paste between these files.

Fri, Aug 28, 1:54 PM · Restricted Project

Wed, Aug 26

JonChesterfield added a comment to D75581: [libomptarget][amdgcn] Implement get_wtime.

I'm unable to find a good reference for what this frequency should be, so am considering unconditionally returning zero from this function. That won't 'work' in any useful sense, but it's an improvement on failing to link because __clock64 is undefined.

Wed, Aug 26, 5:26 PM · Restricted Project
JonChesterfield abandoned D76630: [libomptarget][nfc] Use unity build for nvcc to approximate LTO.
Wed, Aug 26, 5:25 PM · Restricted Project
JonChesterfield committed rG5d989fb37d7c: [libomptarget][amdgpu] Improve thread safety, remove dead code (authored by JonChesterfield).
[libomptarget][amdgpu] Improve thread safety, remove dead code
Wed, Aug 26, 2:05 PM
JonChesterfield committed rG28fbf422f248: [libomptarget][amdgpu] Update plugin CMake to work with latest rocr library (authored by JonChesterfield).
[libomptarget][amdgpu] Update plugin CMake to work with latest rocr library
Wed, Aug 26, 12:02 PM

Tue, Aug 25

JonChesterfield accepted D85777: [OpenMP] Support std::complex math functions in target regions.

The duplication from libc++ doesn't feel good but I can see how it happened. Dependency breaking etc.

Tue, Aug 25, 4:22 PM · Restricted Project
JonChesterfield accepted D85735: [OpenMP] Context selector extensions for template functions.

This looks good to me. I can't claim to have read the test case in detail. Some style suggestions inline around the vector, but they're not blocking.

Tue, Aug 25, 4:13 PM · Restricted Project, Restricted Project

Mon, Aug 24

JonChesterfield added a comment to D85274: [OpenMP] Introduced a bump-like allocator into the target memory management.

The reference counting to free is interesting. I don't think I've seen that on a bump allocator before. It doesn't allow memory reuse within a slab. I wonder whether there is usually some outstanding reference into the slab for most of the execution.

Mon, Aug 24, 1:51 PM · Restricted Project

Sat, Aug 22

JonChesterfield added inline comments to D81054: [OpenMP] Introduce target memory manager.
Sat, Aug 22, 2:54 AM · Restricted Project

Aug 20 2020

JonChesterfield added inline comments to D86307: [OpenMP] Pack first-private arguments to improve efficiency of data transfer.
Aug 20 2020, 7:24 PM · Restricted Project
JonChesterfield updated subscribers of D81054: [OpenMP] Introduce target memory manager.

As a heads up, I'm told this breaks amdgpu tests. @ronlieb is looking at the merge from upstream, don't have any more details at this time. The basic idea of wrapping device alloc seems likely to be sound for all targets so I'd guess we've run into a bug in this patch.

Aug 20 2020, 6:23 PM · Restricted Project
JonChesterfield updated the summary of D86324: [libomptarget][nfc] Drop __kmpc_data_sharing_slot flexible array member.
Aug 20 2020, 6:18 PM · Restricted Project
JonChesterfield added a comment to D86324: [libomptarget][nfc] Drop __kmpc_data_sharing_slot flexible array member.

This was written as part of debugging a port of the deviceRTL to openmp. It probably isn't necessary for that goal after all, but replacing benign UB with .b in a number of places seems worth having anyway.

Aug 20 2020, 6:17 PM · Restricted Project
JonChesterfield updated the summary of D86324: [libomptarget][nfc] Drop __kmpc_data_sharing_slot flexible array member.
Aug 20 2020, 6:14 PM · Restricted Project
JonChesterfield updated the diff for D86324: [libomptarget][nfc] Drop __kmpc_data_sharing_slot flexible array member.
  • remove now-dead casts
Aug 20 2020, 6:12 PM · Restricted Project
JonChesterfield requested review of D86324: [libomptarget][nfc] Drop __kmpc_data_sharing_slot flexible array member.
Aug 20 2020, 6:08 PM · Restricted Project
JonChesterfield added inline comments to D85990: [Clang] Fix BZ47169, loader_uninitialized on incomplete types.
Aug 20 2020, 3:52 PM · Restricted Project
JonChesterfield committed rG3d82c9b6960a: Fix 32 bit build broken by D85990 by dropping align from filecheck pattern (authored by JonChesterfield).
Fix 32 bit build broken by D85990 by dropping align from filecheck pattern
Aug 20 2020, 3:51 PM
JonChesterfield added a comment to D86307: [OpenMP] Pack first-private arguments to improve efficiency of data transfer.

This is a good direction. Packing copies together is likely to be faster.

Aug 20 2020, 1:19 PM · Restricted Project
JonChesterfield added a comment to D84822: Add documentation for target ID and ClangOffloadBundlerFormat.

If you do not care about turning on/off xnack and sram-ecc, you do not need to use it. The current processor name still works.

Aug 20 2020, 3:46 AM · Restricted Project

Aug 19 2020

JonChesterfield updated subscribers of D86232: [NFC][OpenMP] Update description of OMPGridValues enums.
Aug 19 2020, 11:42 AM · Restricted Project
JonChesterfield added a comment to D86232: [NFC][OpenMP] Update description of OMPGridValues enums.

This can't be used in the libomptarget plugins or in the deviceRTL, as both are required to compile without the llvm sources available.

Aug 19 2020, 10:46 AM · Restricted Project
JonChesterfield committed rGbcaa806a4747: [Clang] Fix BZ47169, loader_uninitialized on incomplete types (authored by JonChesterfield).
[Clang] Fix BZ47169, loader_uninitialized on incomplete types
Aug 19 2020, 10:12 AM
JonChesterfield closed D85990: [Clang] Fix BZ47169, loader_uninitialized on incomplete types.
Aug 19 2020, 10:12 AM · Restricted Project
JonChesterfield added a comment to D85990: [Clang] Fix BZ47169, loader_uninitialized on incomplete types.

Works out cleanly, thanks for the suggestion.

Aug 19 2020, 10:10 AM · Restricted Project
JonChesterfield updated the diff for D85990: [Clang] Fix BZ47169, loader_uninitialized on incomplete types.
  • Use more specific diagnostic
Aug 19 2020, 10:08 AM · Restricted Project
JonChesterfield committed rG6e1b11087f08: [libomptarget][amdgpu] Support building with static rocm libraries (authored by JonChesterfield).
[libomptarget][amdgpu] Support building with static rocm libraries
Aug 19 2020, 7:45 AM
Herald added a project to D47833: [CMake] Pass additional CMake tools to external projects: Restricted Project.
Aug 19 2020, 7:20 AM · Restricted Project
JonChesterfield added a comment to D84822: Add documentation for target ID and ClangOffloadBundlerFormat.

How does one opt out of this scheme?

Aug 19 2020, 5:01 AM · Restricted Project
JonChesterfield added a comment to D85990: [Clang] Fix BZ47169, loader_uninitialized on incomplete types.

Were you able to make any progress on this?

Aug 19 2020, 1:00 AM · Restricted Project

Aug 17 2020

JonChesterfield accepted D86082: [libomptarget][NFC] Sort list of plugins in chronological order.

Well volunteered, thank you very much!

Aug 17 2020, 8:30 AM · Restricted Project

Aug 16 2020

JonChesterfield accepted D85875: [OpenMP][FIX] Do not crash trying to print a missing (demangled) user condition.

Seems OK to me as a local fix. I'm not sure about encoding 'missing condition' as 0, but that's a preexisting choice.

Aug 16 2020, 11:15 AM · Restricted Project
JonChesterfield accepted D85878: [OpenMP] Context selector extensions for return value overloading.

Link error seems reasonable to me. Thanks

Aug 16 2020, 11:11 AM · Restricted Project, Restricted Project
JonChesterfield added inline comments to D85742: [libomptarget] Implement host plugin for amdgpu.
Aug 16 2020, 11:09 AM · Restricted Project
JonChesterfield accepted D86038: [OpenMP][CUDA] Cache the maximal number of threads per block (per kernel).

LGTM

Aug 16 2020, 9:27 AM · Restricted Project
JonChesterfield accepted D86039: [OpenMP][CUDA] Keep one kernel list per device, not globally..

Patch is good, thanks.

Aug 16 2020, 9:24 AM · Restricted Project

Aug 15 2020

JonChesterfield committed rGd0b312955f12: [libomptarget] Implement host plugin for amdgpu (authored by JonChesterfield).
[libomptarget] Implement host plugin for amdgpu
Aug 15 2020, 3:59 PM
JonChesterfield closed D85742: [libomptarget] Implement host plugin for amdgpu.
Aug 15 2020, 3:58 PM · Restricted Project
JonChesterfield added a comment to D85742: [libomptarget] Implement host plugin for amdgpu.

There are some parts of this that I'm hoping to push down into the hsa/rocr layer. And other parts that can be deleted without replacement. I will make this better. Good to have clang dev unblocked in the meantime.

Aug 15 2020, 3:16 PM · Restricted Project

Aug 14 2020

JonChesterfield added inline comments to D85990: [Clang] Fix BZ47169, loader_uninitialized on incomplete types.
Aug 14 2020, 2:47 PM · Restricted Project
JonChesterfield added a comment to D74361: [Clang] Undef attribute for global variables.

Fix proposed at D85990. Thanks for the detailed bug report!

Aug 14 2020, 12:44 PM · Restricted Project
JonChesterfield requested review of D85990: [Clang] Fix BZ47169, loader_uninitialized on incomplete types.
Aug 14 2020, 12:43 PM · Restricted Project
JonChesterfield added a comment to D74361: [Clang] Undef attribute for global variables.

Yep! Declaring a global variable that isn't 'extern' with an incomplete type is disallowed anyway, so if you call RequireCompleteType, you're likely just diagnosing that early.

You MIGHT have to do some work to allow:

struct S;
extern S foo __attribute__((loader_uninitialized));
Aug 14 2020, 10:50 AM · Restricted Project
JonChesterfield added a comment to D74361: [Clang] Undef attribute for global variables.

I did a little debugging, and the problem is the template itself isn't a complete type

Aug 14 2020, 8:49 AM · Restricted Project
JonChesterfield added a comment to D74361: [Clang] Undef attribute for global variables.

Also, see this bug: This crashes immediately when used on a template instantiation: https://bugs.llvm.org/show_bug.cgi?id=47169

Aug 14 2020, 7:43 AM · Restricted Project

Aug 13 2020

JonChesterfield added inline comments to D81054: [OpenMP] Introduce target memory manager.
Aug 13 2020, 8:45 PM · Restricted Project
JonChesterfield added a comment to D85878: [OpenMP] Context selector extensions for return value overloading.

If I recall correctly, &foo with variants of foo returns a pointer to the base. If we have no base, and disable_implicit_base, what does &foo yield? It should probably be a compilation error with some descriptive message

Aug 13 2020, 8:34 PM · Restricted Project, Restricted Project
JonChesterfield accepted D85877: [OpenMP] Support nested OpenMP context selectors (declare variant).

Thanks! Probably good to implement this just at the time math headers needs it as that gives us a real world example of the change working.

Aug 13 2020, 8:31 PM · Restricted Project
JonChesterfield accepted D85879: [OpenMP] Overload `std::isnan` and friends multiple times for the GPU.
Aug 13 2020, 8:31 PM · Restricted Project
JonChesterfield added a comment to D85879: [OpenMP] Overload `std::isnan` and friends multiple times for the GPU.

I think this is reasonable. It's unfortunate to have isnan return bool or int depending on the system headers, but considering we have that in a language that doesn't mangle the return type into the name the workaround seems OK.

Aug 13 2020, 8:25 PM · Restricted Project

Aug 12 2020

JonChesterfield added inline comments to D81054: [OpenMP] Introduce target memory manager.
Aug 12 2020, 10:14 PM · Restricted Project
JonChesterfield added a comment to D81054: [OpenMP] Introduce target memory manager.

It definitely can and should be tested. Instantiate on a device that uses host malloc/free for the functions and stress test it under valgrind.

Aug 12 2020, 10:22 AM · Restricted Project
JonChesterfield accepted D81054: [OpenMP] Introduce target memory manager.

LGTM then. Calling into the plugin to do the bulk alloc/free is nice.

Aug 12 2020, 8:47 AM · Restricted Project
JonChesterfield added a comment to D81054: [OpenMP] Introduce target memory manager.

OK, cool. If we're open to changing the implementation later this is fine by me. An instance per host thread is likely to be better than all the internal locks. Couple of minor comments above.

Aug 12 2020, 7:56 AM · Restricted Project
JonChesterfield added inline comments to D85742: [libomptarget] Implement host plugin for amdgpu.
Aug 12 2020, 7:29 AM · Restricted Project
JonChesterfield updated the diff for D85742: [libomptarget] Implement host plugin for amdgpu.
  • Fix typo in comment
Aug 12 2020, 7:25 AM · Restricted Project
JonChesterfield updated the diff for D85742: [libomptarget] Implement host plugin for amdgpu.
  • Insert plugin in chronological order
Aug 12 2020, 7:24 AM · Restricted Project

Aug 11 2020

JonChesterfield added inline comments to D85742: [libomptarget] Implement host plugin for amdgpu.
Aug 11 2020, 10:12 AM · Restricted Project
JonChesterfield added a comment to D85735: [OpenMP] Context selector extensions for template functions.

Nice. What makes it an extension? 5.0 / 2.3.5 claims "and where variant-func-id is the name of a function variant that is either a base language identifier or, for C++, a template-id." which suggests this could be always-on

Aug 11 2020, 9:50 AM · Restricted Project, Restricted Project
JonChesterfield edited reviewers for D85742: [libomptarget] Implement host plugin for amdgpu, added: ronlieb; removed: ronl.
Aug 11 2020, 8:52 AM · Restricted Project
JonChesterfield abandoned D71384: [libomptarget] Implement hsa plugin.
Aug 11 2020, 8:51 AM · Restricted Project
JonChesterfield requested review of D85742: [libomptarget] Implement host plugin for amdgpu.
Aug 11 2020, 8:50 AM · Restricted Project
JonChesterfield added a comment to D81054: [OpenMP] Introduce target memory manager.

I'm still doubtful about this. Bump allocate + no-op free is fast unless the GPU runs out of memory before the arena can be dropped. The list and mutex construction is unusual for an allocator.

Aug 11 2020, 12:11 AM · Restricted Project

Aug 5 2020

JonChesterfield added inline comments to D84822: Add documentation for target ID and ClangOffloadBundlerFormat.
Aug 5 2020, 8:17 AM · Restricted Project
JonChesterfield added inline comments to D84822: Add documentation for target ID and ClangOffloadBundlerFormat.
Aug 5 2020, 7:01 AM · Restricted Project

Jul 30 2020

JonChesterfield added inline comments to D82719: [OpenMPOpt][SplitMemTransfer][WIP] Getting values stored in offload arrays.
Jul 30 2020, 7:16 AM · Restricted Project, Restricted Project
JonChesterfield added a comment to D83316: [OpenMPOpt][WIP] Structure for unittests.

I'm not totally sure about this. There's a lot of test setup relative to the test case itself, and I can't work out by reading the test itself what it's checking. What are all the mock classes for?

Jul 30 2020, 7:06 AM · Restricted Project, Restricted Project

Jul 29 2020

JonChesterfield added a comment to D84743: [Clang][AMDGCN] Universal device offloading macros header.

We probably do want a macro to indicate 'compiling for amdgcn as the device half of a combined host+device language'. I'm having a tough time with the control flow in this header so we probably want tests to check the overall behaviour is as intended. E.g. static assert + various language modes.

Jul 29 2020, 4:04 AM · Restricted Project

Jul 28 2020

JonChesterfield added inline comments to D80917: [OpenMP][AMDGCN] Support OpenMP offloading for AMDGCN architecture - Part 2.
Jul 28 2020, 5:53 AM · Restricted Project, Restricted Project
JonChesterfield added a reviewer for D84743: [Clang][AMDGCN] Universal device offloading macros header: arsenm.
Jul 28 2020, 5:32 AM · Restricted Project

Jul 27 2020

JonChesterfield accepted D84557: [OpenMP][Tests] Enable nvptx64 testing for most libomptarget tests.

Not Johannes, but LGTM. I didn't realise our tests could pick up a different runtime to the one just built, that was a nasty failure mode. Thanks for the patch

Jul 27 2020, 6:03 AM · Restricted Project

Jul 26 2020

JonChesterfield added a comment to D82087: AMDGPU/clang: Add builtins for llvm.amdgcn.ballot.

A macro for wavefront size would make targeting gfx10 for openmp easier.

Jul 26 2020, 4:03 PM

Jul 24 2020

JonChesterfield committed rG679158e662aa: Make hip math headers easier to use from C (authored by JonChesterfield).
Make hip math headers easier to use from C
Jul 24 2020, 12:51 PM
JonChesterfield closed D84476: Make hip math headers easier to use from C.
Jul 24 2020, 12:51 PM · Restricted Project
JonChesterfield added inline comments to D84476: Make hip math headers easier to use from C.
Jul 24 2020, 11:14 AM · Restricted Project
JonChesterfield updated the diff for D84476: Make hip math headers easier to use from C.
  • Fix missing underscores
Jul 24 2020, 11:13 AM · Restricted Project

Jul 23 2020

Herald added a project to D84476: Make hip math headers easier to use from C: Restricted Project.
Jul 23 2020, 4:35 PM · Restricted Project

Jul 15 2020

JonChesterfield accepted D83832: [OpenMP] Provide a flag to disable safety checks for GPU optimizations.

I think this is good. It's dangerous, but it's also undocumented and has unsafe in the name.

Jul 15 2020, 7:45 AM · Restricted Project, Restricted Project

Jul 14 2020

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

Nice, thanks. All my concerns were addressed by the above revision.

Jul 14 2020, 5:28 PM · Restricted Project
JonChesterfield added a comment to D83832: [OpenMP] Provide a flag to disable safety checks for GPU optimizations.

I think there's an unfortunate interaction with link time optimisation here. If there are external regions, but their code is combined with llvm-link before codegen, then a user could reasonably assume this flag is safe.

Jul 14 2020, 5:25 PM · Restricted Project, Restricted Project
JonChesterfield added a comment to D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`.

Agreed on tests. I like the mechanism - passing a string through to the backend as a way to dispatch between isa properties looks cleanly extensible. We probably do want to emit a warning when the backend claims it doesn't know anything about said string as it'll be prone to typos.

Jul 14 2020, 10:13 AM · Restricted Project, Restricted Project
JonChesterfield added inline comments to D83723: [OpenMP][NFC] Generalize CGOpenMPRuntimeNVPTX as CGOpenMPRuntimeGPU.
Jul 14 2020, 5:37 AM · Restricted Project

Jul 13 2020

JonChesterfield added a comment to D83723: [OpenMP][NFC] Generalize CGOpenMPRuntimeNVPTX as CGOpenMPRuntimeGPU.

This is indeed the direction I had in mind. Broad strokes look right. I probably wouldn't notice an accidental change amidst the whitespace reshuffle. Very happy to read through line by line if you can split the whitespace change out.

Jul 13 2020, 3:37 PM · Restricted Project

Jul 10 2020

JonChesterfield added a reviewer for D83591: [OpenMP][CUDA] Fix std::complex in GPU regions: yaxunl.
Jul 10 2020, 5:45 PM · Restricted Project
JonChesterfield added a comment to D83591: [OpenMP][CUDA] Fix std::complex in GPU regions.

I did not know they are using __clang_cuda headers. (Site note, we should rename them then.)

Jul 10 2020, 5:44 PM · Restricted Project
JonChesterfield accepted D83591: [OpenMP][CUDA] Fix std::complex in GPU regions.

Fine by me. Let's get nvptx working properly in tree now and work out how to wire up amdgcn subsequently. I'm sure a reasonable abstraction will present itself.

Jul 10 2020, 4:27 PM · Restricted Project
JonChesterfield accepted D83269: [OpenMP] Identify GPU kernels (aka. OpenMP target regions).

Fair enough, stateful it is then.

Jul 10 2020, 8:28 AM · Restricted Project

Jul 9 2020

JonChesterfield updated subscribers of D77472: [OpenMP][PoC][WIP] An OpenMP-based OpenMP device runtime [NOT FOR COMMIT].
Jul 9 2020, 10:48 AM · Restricted Project
JonChesterfield added a reviewer for D83492: [OpenMP] Use common interface to access GPU Grid Values: grokos.
Jul 9 2020, 10:16 AM · Restricted Project
JonChesterfield added a comment to D83492: [OpenMP] Use common interface to access GPU Grid Values.

Changing to getGridValue would be useful for sharing parts of this with amdgcn.

Jul 9 2020, 10:15 AM · Restricted Project