Page MenuHomePhabricator

JonChesterfield (Jon Chesterfield)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 11 2015, 1:08 PM (271 w, 4 d)

Recent Activity

Thu, Oct 22

JonChesterfield added inline comments to D89994: [libomptarget][nvptx] Undef, internal shared variables.
Thu, Oct 22, 5:12 PM · Restricted Project
JonChesterfield added a comment to D89994: [libomptarget][nvptx] Undef, internal shared variables.

The nvptx back end accepts common + zero + shared, but not common + undef + shared. I think weak_odr is conceptually right here, but given the warning that nvlink doesn't support weak symbols, internal also seems fine. Can someone see an advantage to weak over internal? It could be arch specific at the risk of a lot of test duplication.

IIRC, it supports weak symbols, but does not support weak symbols of different sizes.

Thu, Oct 22, 5:06 PM · Restricted Project
JonChesterfield added a comment to D89994: [libomptarget][nvptx] Undef, internal shared variables.

The nvptx back end accepts common + zero + shared, but not common + undef + shared. I think weak_odr is conceptually right here, but given the warning that nvlink doesn't support weak symbols, internal also seems fine. Can someone see an advantage to weak over internal? It could be arch specific at the risk of a lot of test duplication.

Thu, Oct 22, 4:25 PM · Restricted Project
JonChesterfield requested review of D89994: [libomptarget][nvptx] Undef, internal shared variables.
Thu, Oct 22, 4:22 PM · Restricted Project
JonChesterfield added a comment to rG7b3eabdcd215: [OPENMP][NVPTX]Fix PR40893: Size doesn't match for….

That's very plausible. Thanks, will experiment.

Thu, Oct 22, 1:47 PM
JonChesterfield added a comment to rG7b3eabdcd215: [OPENMP][NVPTX]Fix PR40893: Size doesn't match for….

Was that a typo? The patch changed common to internal. I'd quite like all the shared variables to have consistent linkage, with a mild preference for not-internal.

Thu, Oct 22, 1:21 PM
JonChesterfield added a comment to rG7b3eabdcd215: [OPENMP][NVPTX]Fix PR40893: Size doesn't match for….

Is this bug only for common linkage, or does it also affect weakanylinkage? If so, we should update setPropertyExecutionMode to use internal as well

Thu, Oct 22, 12:27 PM
JonChesterfield accepted D89971: [OpenMP][CUDA] Add missing overload for `remquo(float,float,int*)`.

Change obviously good. Am I right in reading this as all of OvO then passes for trunk, nvptx?

Thu, Oct 22, 10:00 AM · Restricted Project
JonChesterfield committed rG09bc755deaa6: [OpenMP] Emit calls to int64_t functions for amdgcn (authored by JonChesterfield).
[OpenMP] Emit calls to int64_t functions for amdgcn
Thu, Oct 22, 7:03 AM
JonChesterfield closed D89746: [OpenMP] Emit calls to int64_t functions for amdgcn.
Thu, Oct 22, 7:03 AM · Restricted Project

Wed, Oct 21

JonChesterfield updated the diff for D89746: [OpenMP] Emit calls to int64_t functions for amdgcn.
  • test attributes on syncwarp, thread_mask
Wed, Oct 21, 6:01 PM · Restricted Project
JonChesterfield updated the diff for D89746: [OpenMP] Emit calls to int64_t functions for amdgcn.
  • tests for i32 version of active_thread_mask, syncwarp
Wed, Oct 21, 5:44 PM · Restricted Project
JonChesterfield added a comment to D89746: [OpenMP] Emit calls to int64_t functions for amdgcn.

Good call on shadowing, those are all gone. Attributes are more difficult as the test is IR, so can't declare the function twice. Haven't managed to tag nounwind onto either yet but will try a few more things.

Wed, Oct 21, 4:52 PM · Restricted Project
JonChesterfield updated the diff for D89746: [OpenMP] Emit calls to int64_t functions for amdgcn.
  • address some review comments
Wed, Oct 21, 4:49 PM · Restricted Project
JonChesterfield committed rG26790ed24887: [libomptarget] Require LLVM source tree to build libomptarget (authored by JonChesterfield).
[libomptarget] Require LLVM source tree to build libomptarget
Wed, Oct 21, 10:53 AM
JonChesterfield closed D89426: [libomptarget] Require LLVM source tree to build libomptarget.
Wed, Oct 21, 10:53 AM · Restricted Project
JonChesterfield added a comment to D80816: [OpenMP] Add unbundling of archives containing bundled object files into device specific archives..

Ping.

Wed, Oct 21, 10:39 AM · Restricted Project, Restricted Project
JonChesterfield committed rG55dc123555db: [libomptarget][amdgcn] Refactor memcpy to eliminate maps (authored by JonChesterfield).
[libomptarget][amdgcn] Refactor memcpy to eliminate maps
Wed, Oct 21, 9:00 AM
JonChesterfield closed D89888: [libomptarget][amdgcn] Refactor memcpy to eliminate maps.
Wed, Oct 21, 9:00 AM · Restricted Project
JonChesterfield added inline comments to D89888: [libomptarget][amdgcn] Refactor memcpy to eliminate maps.
Wed, Oct 21, 8:35 AM · Restricted Project
JonChesterfield requested review of D89888: [libomptarget][amdgcn] Refactor memcpy to eliminate maps.
Wed, Oct 21, 8:33 AM · Restricted Project

Tue, Oct 20

JonChesterfield added a comment to D89584: [AMDGPU][OPENMP] OpenMP AMDGCN Header Support.

Patch looks broadly reasonable to me. @jdoerfert does this align with what you want from these headers?

Tue, Oct 20, 2:51 PM
JonChesterfield added a comment to D89725: [libomptarget][amdgcn] Implement missing symbols in deviceRTL.

Yes, lets. The benchmarking to guess the frequency isn't a big piece of work, but I don't seem to have got around to it so far.

Tue, Oct 20, 3:14 AM · Restricted Project
JonChesterfield abandoned D75581: [libomptarget][amdgcn] Implement get_wtime.
Tue, Oct 20, 3:12 AM · Restricted Project
JonChesterfield accepted D89776: [libomptarget][AMDGPU][NFC] Split atmi_memcpy for h2d and d2h.

LG, thanks!

Tue, Oct 20, 2:47 AM · Restricted Project
JonChesterfield retitled D89776: [libomptarget][AMDGPU][NFC] Split atmi_memcpy for h2d and d2h from [libomptarget][AMDGPU] Split atmi_memcpy for h2d and d2h to [libomptarget][AMDGPU][NFC] Split atmi_memcpy for h2d and d2h.
Tue, Oct 20, 2:33 AM · Restricted Project

Mon, Oct 19

JonChesterfield committed rGd27b39ce1162: [libomptarget][amdgcn] Implement missing symbols in deviceRTL (authored by JonChesterfield).
[libomptarget][amdgcn] Implement missing symbols in deviceRTL
Mon, Oct 19, 4:24 PM
JonChesterfield closed D89725: [libomptarget][amdgcn] Implement missing symbols in deviceRTL.
Mon, Oct 19, 4:24 PM · Restricted Project
JonChesterfield added inline comments to D89746: [OpenMP] Emit calls to int64_t functions for amdgcn.
Mon, Oct 19, 3:23 PM · Restricted Project
JonChesterfield requested review of D89746: [OpenMP] Emit calls to int64_t functions for amdgcn.
Mon, Oct 19, 3:22 PM · Restricted Project
JonChesterfield added a reviewer for D89725: [libomptarget][amdgcn] Implement missing symbols in deviceRTL: ronlieb.
Mon, Oct 19, 1:16 PM · Restricted Project
JonChesterfield requested review of D89725: [libomptarget][amdgcn] Implement missing symbols in deviceRTL.
Mon, Oct 19, 1:15 PM · Restricted Project
JonChesterfield added inline comments to D86097: [OpenMP][AMDGCN] Generate global variables and attributes for AMDGCN.
Mon, Oct 19, 7:49 AM · Restricted Project

Fri, Oct 16

JonChesterfield accepted D89597: [libomptarget] Fix copy-to motion for PTR_AND_OBJ entries where PTR is a struct member.

Yep, that seems clear cut. Thanks!

Fri, Oct 16, 4:07 PM · Restricted Project
JonChesterfield added inline comments to D89584: [AMDGPU][OPENMP] OpenMP AMDGCN Header Support.
Fri, Oct 16, 12:50 PM
JonChesterfield added a comment to D89426: [libomptarget] Require LLVM source tree to build libomptarget.

New spin. Attempts to handle LLVM_MAIN_INCLUDE_DIR missing by looking for a plausible source directory, and avoids setting a variable which isn't LIBOMPTARGET_ prefixed.

Fri, Oct 16, 10:19 AM · Restricted Project
JonChesterfield updated the diff for D89426: [libomptarget] Require LLVM source tree to build libomptarget.
  • review comments
  • Try harder to find LLVM on disk
Fri, Oct 16, 10:16 AM · Restricted Project
JonChesterfield added a comment to D89426: [libomptarget] Require LLVM source tree to build libomptarget.

set(LLVM_MAIN_INCLUDE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/../llvm/include) is interesting.

Fri, Oct 16, 9:05 AM · Restricted Project
JonChesterfield added a comment to D89426: [libomptarget] Require LLVM source tree to build libomptarget.

Answering my own questions about cmake semantics by trial and error, the above revision handles the missing definition by not building libomptarget.

Fri, Oct 16, 8:03 AM · Restricted Project
JonChesterfield updated the diff for D89426: [libomptarget] Require LLVM source tree to build libomptarget.
  • review comments
Fri, Oct 16, 8:00 AM · Restricted Project
JonChesterfield added a comment to D89426: [libomptarget] Require LLVM source tree to build libomptarget.

I'm unclear on what that cmake is doing. Copied here:

set(ENABLE_LIBOMPTARGET ON)
# Currently libomptarget cannot be compiled on Windows or MacOS X.                                                                                                                                                 
# Since the device plugins are only supported on Linux anyway,                                                                                                                                                     
# there is no point in trying to compile libomptarget on other OSes.                                                                                                                                               
if (APPLE OR WIN32 OR NOT OPENMP_HAVE_STD_CPP14_FLAG)
  set(ENABLE_LIBOMPTARGET OFF)
endif()
Fri, Oct 16, 7:30 AM · Restricted Project
JonChesterfield added a comment to D89426: [libomptarget] Require LLVM source tree to build libomptarget.

This test is in libomptarget, so that should work. I don't know if there's an existing hook in libomp to disable libomptarget, or whether the former actually works without the latter. Can take a look.

Fri, Oct 16, 6:34 AM · Restricted Project

Thu, Oct 15

JonChesterfield added inline comments to D86097: [OpenMP][AMDGCN] Generate global variables and attributes for AMDGCN.
Thu, Oct 15, 8:56 AM · Restricted Project
JonChesterfield added reviewers for D89426: [libomptarget] Require LLVM source tree to build libomptarget: jlpeyton, AndreyChurbanov, protze.joachim, ABataev, gtbercea, grokos, tianshilei1992, pawosm01, dim, jdenny, mgorny, jcownie, hans, hfinkel, ye-luo.
Thu, Oct 15, 8:01 AM · Restricted Project
JonChesterfield committed rG7d2ecef5ed11: [openmp][libomptarget] Include header from LLVM source tree (authored by JonChesterfield).
[openmp][libomptarget] Include header from LLVM source tree
Thu, Oct 15, 7:47 AM
JonChesterfield closed D87841: [openmp][libomptarget] Include header from LLVM source tree.
Thu, Oct 15, 7:46 AM · Restricted Project, Restricted Project
JonChesterfield added inline comments to D87841: [openmp][libomptarget] Include header from LLVM source tree.
Thu, Oct 15, 7:45 AM · Restricted Project, Restricted Project
JonChesterfield updated the diff for D87841: [openmp][libomptarget] Include header from LLVM source tree.

-rebase

Thu, Oct 15, 7:45 AM · Restricted Project, Restricted Project
JonChesterfield committed rGc66e091023b8: [NFC] Fix license header from D87841 (authored by JonChesterfield).
[NFC] Fix license header from D87841
Thu, Oct 15, 7:41 AM

Wed, Oct 14

JonChesterfield requested review of D89426: [libomptarget] Require LLVM source tree to build libomptarget.
Wed, Oct 14, 2:26 PM · Restricted Project
JonChesterfield added a comment to D87841: [openmp][libomptarget] Include header from LLVM source tree.

Can we move the cmake stuff in the main openmptarget logic so we can use llvm in all of openmp target.

Wed, Oct 14, 12:44 PM · Restricted Project, Restricted Project
JonChesterfield updated the diff for D87841: [openmp][libomptarget] Include header from LLVM source tree.
  • rebase
Wed, Oct 14, 7:24 AM · Restricted Project, Restricted Project

Mon, Oct 12

JonChesterfield added inline comments to D88602: [libomptarget][amdgcn] Implement partial barrier.
Mon, Oct 12, 1:28 PM · Restricted Project
JonChesterfield committed rG8b6cd1524267: [libomptarget][amdgcn] Implement partial barrier (authored by JonChesterfield).
[libomptarget][amdgcn] Implement partial barrier
Mon, Oct 12, 1:27 PM
JonChesterfield closed D88602: [libomptarget][amdgcn] Implement partial barrier.
Mon, Oct 12, 1:27 PM · Restricted Project
JonChesterfield added a comment to D88602: [libomptarget][amdgcn] Implement partial barrier.

Any objections? This has been through a bunch of internal testing now and seems to hang together. Hopefully the extra __kmpc_impl_target_init() function is acceptable.

Mon, Oct 12, 8:23 AM · Restricted Project

Thu, Oct 8

JonChesterfield added a comment to D88929: [OpenMP] Change CMake Configuration to Build for Highest CUDA Architecture by Default.

Nice patch. Thanks!

Thu, Oct 8, 2:19 AM · Restricted Project, Restricted Project

Tue, Oct 6

JonChesterfield added a comment to D88929: [OpenMP] Change CMake Configuration to Build for Highest CUDA Architecture by Default.

An alternative approach is to build the deviceRTL for multiple cuda versions and then pick whichever one is the best fit when compiling application code. That has advantages when building the deviceRTL libraries on a different machine to the one that intends to use it.

Tue, Oct 6, 4:42 PM · Restricted Project, Restricted Project

Mon, Oct 5

JonChesterfield added a comment to D88837: [HIP] Restructure hip headers to add cmath.

Thanks for this. LGTM.

Mon, Oct 5, 2:59 PM · Restricted Project
JonChesterfield accepted D88829: [OpenMP][RTL] Remove dead code.

Thanks guys, will remember that as the local convention on rolling whitespace changes into other stuff.

Mon, Oct 5, 7:09 AM · Restricted Project, Restricted Project
JonChesterfield added a comment to D88829: [OpenMP][RTL] Remove dead code.

Change looks great to me.

Mon, Oct 5, 6:42 AM · Restricted Project, Restricted Project
JonChesterfield added reviewers for D88829: [OpenMP][RTL] Remove dead code: ABataev, grokos.
Mon, Oct 5, 6:39 AM · Restricted Project, Restricted Project

Thu, Oct 1

JonChesterfield added a comment to D83394: [AMDGPU] Avoid splitting FLAT offsets in unsafe ways.

I'm seeing (difficult to minimise) failure modes with gfx10 that look the same as those on gfx9 before this patch. I also see the comment:

Thu, Oct 1, 4:06 AM · Restricted Project

Wed, Sep 30

JonChesterfield added a reviewer for D88602: [libomptarget][amdgcn] Implement partial barrier: hfinkel.

This was written carefully and reviewed internally. It'll be horrendous to debug in the field if it's wrong so feedback is very welcome.

Wed, Sep 30, 11:50 AM · Restricted Project
JonChesterfield requested review of D88602: [libomptarget][amdgcn] Implement partial barrier.
Wed, Sep 30, 11:46 AM · Restricted Project

Tue, Sep 29

JonChesterfield committed rGd256797c9035: [nfc][libomptarget] Drop parameter to named_sync (authored by JonChesterfield).
[nfc][libomptarget] Drop parameter to named_sync
Tue, Sep 29, 3:12 PM
JonChesterfield closed D88474: [nfc][libomptarget] Drop parameter to named_sync.
Tue, Sep 29, 3:12 PM · Restricted Project
JonChesterfield added inline comments to D88474: [nfc][libomptarget] Drop parameter to named_sync.
Tue, Sep 29, 3:28 AM · Restricted Project
JonChesterfield requested review of D88474: [nfc][libomptarget] Drop parameter to named_sync.
Tue, Sep 29, 3:21 AM · Restricted Project

Mon, Sep 28

JonChesterfield accepted D88384: [OpenMP][FIX] Verify compatible types for declare variant calls.

Change looks reasonable in itself too

Mon, Sep 28, 8:34 AM · Restricted Project

Sep 24 2020

JonChesterfield accepted D88185: [OpenMP] cmake option LIBOMPTARGET_NVPTX_MAX_SM for nvptx device RTL .

LGTM. Change looks safe as-is and we can add finer granularity later.

Sep 24 2020, 7:46 AM · Restricted Project

Sep 23 2020

JonChesterfield added inline comments to D88185: [OpenMP] cmake option LIBOMPTARGET_NVPTX_MAX_SM for nvptx device RTL .
Sep 23 2020, 6:13 PM · Restricted Project
JonChesterfield added inline comments to D88185: [OpenMP] cmake option LIBOMPTARGET_NVPTX_MAX_SM for nvptx device RTL .
Sep 23 2020, 6:02 PM · Restricted Project
JonChesterfield added a comment to D88185: [OpenMP] cmake option LIBOMPTARGET_NVPTX_MAX_SM for nvptx device RTL .

Change seems reasonable. Amdgcn could benefit from the same, e.g. for trying to get apu systems with about 8 CU to run openmp code. Suggest we do that in a different patch if someone asks for it.

Sep 23 2020, 5:02 PM · Restricted Project
JonChesterfield added a comment to D87841: [openmp][libomptarget] Include header from LLVM source tree.

Ping. Can we have the include from llvm now, and postpone choosing a new top level directory until we have a longer list of things we'd like in it?

Sep 23 2020, 3:45 PM · Restricted Project, Restricted Project

Sep 18 2020

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.

Sep 18 2020, 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.
Sep 18 2020, 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

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

Sep 17 2020

JonChesterfield added inline comments to D87841: [openmp][libomptarget] Include header from LLVM source tree.
Sep 17 2020, 6:40 PM · Restricted Project, Restricted Project
JonChesterfield updated the summary of D87841: [openmp][libomptarget] Include header from LLVM source tree.
Sep 17 2020, 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)

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

Sep 3 2020

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.

Sep 3 2020, 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.

Sep 3 2020, 7:51 AM · Restricted Project

Aug 28 2020

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.

Aug 28 2020, 1:54 PM · Restricted Project

Aug 26 2020

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.

Aug 26 2020, 5:26 PM · Restricted Project
JonChesterfield abandoned D76630: [libomptarget][nfc] Use unity build for nvcc to approximate LTO.
Aug 26 2020, 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
Aug 26 2020, 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
Aug 26 2020, 12:02 PM

Aug 25 2020

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.

Aug 25 2020, 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.

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

Aug 24 2020

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.

Aug 24 2020, 1:51 PM · Restricted Project

Aug 22 2020

JonChesterfield added inline comments to D81054: [OpenMP] Introduce target memory manager.
Aug 22 2020, 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