Page MenuHomePhabricator

JonChesterfield (Jon Chesterfield)
User

Projects

User does not belong to any projects.

User Details

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

Recent Activity

Thu, Sep 12

JonChesterfield added a comment to D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker script.

I see some problems with using llvm-objcopy for that. First issue is that symbols created by llvm-objcopy for embedded data depend on the input file name. As you know these symbols are referenced from the offload registration code that is currently added to an object by the clang at compile time. I not sure how you can guarantee that symbol names will match.

Thu, Sep 12, 11:21 AM
JonChesterfield added a comment to D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker script.

Does this diff mix getting rid of the linker script with other changes? E.g. it looks like the metadata generation is moving from clang to the new tool, but that seems orthogonal to dropping the linker script.

Metadata is still generated by the clang, there are no changes in this area. What is moving to a wrapper tool is the generation of the offload registration code. Let me just attach the slides that I presented on the inter company meeting were the proposal was discussed. It'll probably answer most of your questions.

Thu, Sep 12, 9:24 AM
JonChesterfield added a comment to D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker script.

I'm on board with getting rid of the linker script. Gold's limited support for that seems conclusive.

Thu, Sep 12, 2:57 AM
JonChesterfield added a comment to D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker script.

Hm, I was not aware of this Linux linker feature, thanks a lot for the explanation! I see only one problem with using it as a replacement for the begin/end objects – it looks like __start_name/__stop_name symbols are created with default visibility instead of hidden. I guess it will cause problems for offload programs that use shared libraries because DSO’s __start_name/__stop_name symbols will be preempted by the executable’s symbols and that is not what we want. Is there any way to change this behavior?

Thu, Sep 12, 2:39 AM

Wed, Sep 11

JonChesterfield added a comment to D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker script.

I'm not sure copying the crtbegin/crtend mechanism from the early days of C runtime is ideal. Since the data is stored in a common section anyway, please could we rename it to __omp_offloading_entries in which case the linker will provide start/end symbols automatically?

Well, I never said that it is an ideal solution, but it is a known mechanism that works well in many cases and can also be reused for the offloading entry table.
I do not fully understand your suggestion for renaming entries section, how it will help with providing start/end symbols for the entries. Can you please provide more details?

Wed, Sep 11, 2:02 PM
JonChesterfield added a comment to D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker script.

OpenMP linker script is known to cause problems for gold and lld linkers on Linux and it will also cause problems for Windows enabling in future

What are the known problems with the linker script? I'm wondering if they can be resolved without the overhead of introducing a new tool.

They just do not support linker script. And, thus, cannot be used for offloading. Only ld supports it.

In what respect? I've used linker scripts with both gold and lld, and both instances of --help text claim to support them. In the case of lld, a very complicated script hit a few internal errors, but I believe they've all been fixed since.

Hmm, I tried it with gold some time ago and it just did not work for me. The linking failed with diagnostics that some of the commands in the script are unknown.

Wed, Sep 11, 12:35 PM
JonChesterfield added a comment to D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker script.

OpenMP linker script is known to cause problems for gold and lld linkers on Linux and it will also cause problems for Windows enabling in future

What are the known problems with the linker script? I'm wondering if they can be resolved without the overhead of introducing a new tool.

They just do not support linker script. And, thus, cannot be used for offloading. Only ld supports it.

Wed, Sep 11, 12:21 PM
JonChesterfield added a comment to D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker script.

OpenMP linker script is known to cause problems for gold and lld linkers on Linux and it will also cause problems for Windows enabling in future

Wed, Sep 11, 12:09 PM
JonChesterfield added a comment to D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker script.

I'm not sure copying the crtbegin/crtend mechanism from the early days of C runtime is ideal. Since the data is stored in a common section anyway, please could we rename it to __omp_offloading_entries in which case the linker will provide start/end symbols automatically? That removes the two object files and the link order dependency which is a hazard to bitcode libraries.

Wed, Sep 11, 12:02 PM
JonChesterfield added a comment to D62393: [OPENMP][NVPTX]Mark parallel level counter as volatile..

Thanks for the explicit asm. One of the hazards of compiling cuda for amdgcn is that volatile doesn't imply atomic, so this is a clear warning that I'll have a bug on merge.

Wed, Sep 11, 9:27 AM · Restricted Project

Tue, Sep 3

JonChesterfield added a comment to D66802: [OPENMP][NVPTX]Fix parallel level counter in non-SPMD mode..

Looks good here. Apologies for the rebase induced by the recent patches to omptarget-nvptx.

Tue, Sep 3, 10:34 AM · Restricted Project, Restricted Project
JonChesterfield committed rGbbdd2823715b: [libomptarget] Refactor activemask macro to inline function (authored by JonChesterfield).
[libomptarget] Refactor activemask macro to inline function
Tue, Sep 3, 9:52 AM
JonChesterfield committed rL370781: [libomptarget] Refactor activemask macro to inline function.
[libomptarget] Refactor activemask macro to inline function
Tue, Sep 3, 9:30 AM
JonChesterfield closed D66851: [libomptarget] Refactor activemask macro to inline function.
Tue, Sep 3, 9:30 AM · Restricted Project, Restricted Project
JonChesterfield added a comment to D66851: [libomptarget] Refactor activemask macro to inline function.

Thanks guys. It's nice to see the last function style macros disappear from omptarget-nvptx. This means all the CUDA_VERSION tests are now localised to target_impl.h,

Tue, Sep 3, 9:27 AM · Restricted Project, Restricted Project

Mon, Sep 2

JonChesterfield added a comment to D66851: [libomptarget] Refactor activemask macro to inline function.

Replace types with __kmpc_impl_lanemask_t wherever they are used within a function.

Mon, Sep 2, 1:32 PM · Restricted Project, Restricted Project
JonChesterfield updated the diff for D66851: [libomptarget] Refactor activemask macro to inline function.
  • keep ULL type in IsWarpMasterActiveThread
Mon, Sep 2, 1:29 PM · Restricted Project, Restricted Project
JonChesterfield updated the diff for D66851: [libomptarget] Refactor activemask macro to inline function.
  • uint32 -> lanemask_t
Mon, Sep 2, 1:25 PM · Restricted Project, Restricted Project
JonChesterfield updated the diff for D66851: [libomptarget] Refactor activemask macro to inline function.
  • rebase
Mon, Sep 2, 1:20 PM · Restricted Project, Restricted Project
JonChesterfield committed rL370681: Request commit access for jonchesterfield.
Request commit access for jonchesterfield
Mon, Sep 2, 12:40 PM

Wed, Aug 28

JonChesterfield committed rG329442192625: Use target_impl functions to replace more inline asm (authored by JonChesterfield).
Use target_impl functions to replace more inline asm
Wed, Aug 28, 8:05 AM
JonChesterfield committed rL370216: Use target_impl functions to replace more inline asm.
Use target_impl functions to replace more inline asm
Wed, Aug 28, 8:03 AM
JonChesterfield closed D66809: Use target_impl functions to replace more inline asm.
Wed, Aug 28, 8:03 AM · Restricted Project, Restricted Project
JonChesterfield added inline comments to D66851: [libomptarget] Refactor activemask macro to inline function.
Wed, Aug 28, 7:28 AM · Restricted Project, Restricted Project
JonChesterfield committed rG80f9a38a7653: [libomptarget] Refactor syncthreads macro to inline function (authored by JonChesterfield).
[libomptarget] Refactor syncthreads macro to inline function
Wed, Aug 28, 7:22 AM
JonChesterfield committed rL370210: [libomptarget] Refactor syncthreads macro to inline function.
[libomptarget] Refactor syncthreads macro to inline function
Wed, Aug 28, 7:22 AM
JonChesterfield closed D66861: [libomptarget] Refactor syncthreads macro to inline function.
Wed, Aug 28, 7:22 AM · Restricted Project, Restricted Project

Tue, Aug 27

JonChesterfield created D66861: [libomptarget] Refactor syncthreads macro to inline function.
Tue, Aug 27, 7:22 PM · Restricted Project, Restricted Project
JonChesterfield abandoned D66855: [libomptarget] Refactor syncthreads macro to inline function.

Phabricator has decided to mix this patch in with the other patches that have landed on master since initially creating it. Presumably my mistake for rebasing. I'll abandon this and create a new diff. Sorry for the noise.

Tue, Aug 27, 7:15 PM · Restricted Project
JonChesterfield updated the diff for D66855: [libomptarget] Refactor syncthreads macro to inline function.
  • remove fixme, add comment, rebase on master
Tue, Aug 27, 7:13 PM · Restricted Project
JonChesterfield committed rGbe3d48731300: [libomptarget] Refactor syncwarp macro to inline function (authored by JonChesterfield).
[libomptarget] Refactor syncwarp macro to inline function
Tue, Aug 27, 7:03 PM
JonChesterfield added a comment to D66855: [libomptarget] Refactor syncthreads macro to inline function.

Thanks. I wasn't sure whether to leave the FIXMEs in place or not, will drop them.

Tue, Aug 27, 7:03 PM · Restricted Project
JonChesterfield committed rL370149: [libomptarget] Refactor syncwarp macro to inline function.
[libomptarget] Refactor syncwarp macro to inline function
Tue, Aug 27, 7:02 PM
JonChesterfield closed D66857: [libomptarget] Refactor syncwarp macro to inline function.
Tue, Aug 27, 7:02 PM · Restricted Project, Restricted Project
JonChesterfield abandoned D66846: [libomptarget] Refactor macros from omptarget-nvptx to inline functions.

Abandoning in favour of split revisions (one per macro)

Tue, Aug 27, 6:59 PM · Restricted Project
JonChesterfield committed rGe73e3013a646: Fix build break due to close brace lost in merge (authored by JonChesterfield).
Fix build break due to close brace lost in merge
Tue, Aug 27, 6:56 PM
JonChesterfield committed rL370148: Fix build break due to close brace lost in merge.
Fix build break due to close brace lost in merge
Tue, Aug 27, 6:56 PM
JonChesterfield committed rG327aa811234d: [libomptarget] Refactor shfl_down_sync macro to inline function (authored by JonChesterfield).
[libomptarget] Refactor shfl_down_sync macro to inline function
Tue, Aug 27, 6:48 PM
JonChesterfield committed rL370146: [libomptarget] Refactor shfl_down_sync macro to inline function.
[libomptarget] Refactor shfl_down_sync macro to inline function
Tue, Aug 27, 6:48 PM
JonChesterfield closed D66853: [libomptarget] Refactor shfl_down_sync macro to inline function.
Tue, Aug 27, 6:48 PM · Restricted Project, Restricted Project
JonChesterfield committed rGb9b712df82fa: [libomptarget] Refactor shfl_sync macro to inline function (authored by JonChesterfield).
[libomptarget] Refactor shfl_sync macro to inline function
Tue, Aug 27, 6:32 PM
JonChesterfield committed rL370144: [libomptarget] Refactor shfl_sync macro to inline function.
[libomptarget] Refactor shfl_sync macro to inline function
Tue, Aug 27, 6:30 PM
JonChesterfield closed D66852: [libomptarget] Refactor shfl_sync macro to inline function.
Tue, Aug 27, 6:30 PM · Restricted Project, Restricted Project
JonChesterfield updated the diff for D66852: [libomptarget] Refactor shfl_sync macro to inline function.
  • comment end of #if block
Tue, Aug 27, 6:29 PM · Restricted Project, Restricted Project
JonChesterfield updated the diff for D66846: [libomptarget] Refactor macros from omptarget-nvptx to inline functions.
  • drop spurious return
Tue, Aug 27, 6:24 PM · Restricted Project
JonChesterfield added inline comments to D66846: [libomptarget] Refactor macros from omptarget-nvptx to inline functions.
Tue, Aug 27, 6:24 PM · Restricted Project
JonChesterfield created D66857: [libomptarget] Refactor syncwarp macro to inline function.
Tue, Aug 27, 6:23 PM · Restricted Project, Restricted Project
JonChesterfield created D66855: [libomptarget] Refactor syncthreads macro to inline function.
Tue, Aug 27, 6:20 PM · Restricted Project
JonChesterfield created D66853: [libomptarget] Refactor shfl_down_sync macro to inline function.
Tue, Aug 27, 6:08 PM · Restricted Project, Restricted Project
JonChesterfield created D66852: [libomptarget] Refactor shfl_sync macro to inline function.
Tue, Aug 27, 6:04 PM · Restricted Project, Restricted Project
JonChesterfield created D66851: [libomptarget] Refactor activemask macro to inline function.
Tue, Aug 27, 5:56 PM · Restricted Project, Restricted Project
JonChesterfield added a comment to D66846: [libomptarget] Refactor macros from omptarget-nvptx to inline functions.

Will split as recommended

Tue, Aug 27, 5:52 PM · Restricted Project
JonChesterfield updated the diff for D66846: [libomptarget] Refactor macros from omptarget-nvptx to inline functions.
  • from review - fix bug when compiling with clang
Tue, Aug 27, 5:48 PM · Restricted Project
JonChesterfield added a comment to D66846: [libomptarget] Refactor macros from omptarget-nvptx to inline functions.

I'm not sure about the signal to noise of the C style __kmpc_impl namespace. Perhaps C++ namespace __kmpc_impl {} and using namespace __kmpc_impl; in the source would be better?

Tue, Aug 27, 4:46 PM · Restricted Project
JonChesterfield created D66846: [libomptarget] Refactor macros from omptarget-nvptx to inline functions.
Tue, Aug 27, 4:37 PM · Restricted Project
JonChesterfield created D66809: Use target_impl functions to replace more inline asm.
Tue, Aug 27, 9:55 AM · Restricted Project, Restricted Project

Sat, Aug 24

JonChesterfield added a comment to D66672: [OPENMP][NVPTX]Add __kmpc_syncwarp(int32_t) function..

Looks good. Delighted to see extensions to target_impl.

Sat, Aug 24, 1:11 PM · Restricted Project

Aug 14 2019

JonChesterfield added a comment to D65836: Factor architecture dependent code out of loop.cu.

Jon, please subscribe to openmp-commits so that commit emails get through immediately. Thanks!

Aug 14 2019, 3:15 AM · Restricted Project, Restricted Project

Aug 13 2019

JonChesterfield committed rGed3324f6b6e3: Factor architecture dependent code out of loop.cu (authored by JonChesterfield).
Factor architecture dependent code out of loop.cu
Aug 13 2019, 2:43 PM
JonChesterfield committed rL368751: Factor architecture dependent code out of loop.cu.
Factor architecture dependent code out of loop.cu
Aug 13 2019, 2:42 PM
JonChesterfield closed D65836: Factor architecture dependent code out of loop.cu.
Aug 13 2019, 2:41 PM · Restricted Project, Restricted Project
JonChesterfield added a comment to D65836: Factor architecture dependent code out of loop.cu.

What about namong convention here? Shall we use capital letters for the var names or it is fine as is?

Long term it's probably worth moving to the LLVM conventions throughout. Short term, I'd rather keep roughly the same style as the surrounding code. I believe that's what this patch does.

Aug 13 2019, 11:57 AM · Restricted Project, Restricted Project
JonChesterfield added a comment to D65836: Factor architecture dependent code out of loop.cu.

Ping. I'd like to land this - Alexey?

Aug 13 2019, 10:07 AM · Restricted Project, Restricted Project

Aug 12 2019

JonChesterfield added a comment to D62046: [OpenMP][bugfix] Add missing math functions variants for log and abs..

I'm not sure about this diff. I think it's breaking <iostream> and <algorithm>. Raised bug https://bugs.llvm.org/show_bug.cgi?id=42972

Aug 12 2019, 8:51 AM · Restricted Project

Aug 8 2019

JonChesterfield added a comment to D65836: Factor architecture dependent code out of loop.cu.

I'm wary of refactoring to support multiple architectures while there is only one in tree. It's too difficult to see where the abstractions should be, and it's usually easier to introduce said abstractions than to move them later.

Aug 8 2019, 9:35 AM · Restricted Project, Restricted Project
JonChesterfield added inline comments to D65836: Factor architecture dependent code out of loop.cu.
Aug 8 2019, 3:16 AM · Restricted Project, Restricted Project

Aug 7 2019

JonChesterfield added inline comments to D65836: Factor architecture dependent code out of loop.cu.
Aug 7 2019, 1:01 PM · Restricted Project, Restricted Project
JonChesterfield added a comment to D65867: [RFC] [OpenMP] Turn on -Wall compiler warnings by default.

I like this a lot. Thanks

Aug 7 2019, 12:15 PM · Restricted Project, Restricted Project
JonChesterfield updated the diff for D65836: Factor architecture dependent code out of loop.cu.
  • drop omptarget-nvptx include
  • address review comments
  • rebase, use INLINE from D65876
Aug 7 2019, 8:45 AM · Restricted Project, Restricted Project
JonChesterfield committed rGae0178bee72c: Use forceinline. Necessary for nvcc to inline small functions within the… (authored by JonChesterfield).
Use forceinline. Necessary for nvcc to inline small functions within the…
Aug 7 2019, 8:25 AM
JonChesterfield committed rL368177: Use forceinline. Necessary for nvcc to inline small functions within the….
Use forceinline. Necessary for nvcc to inline small functions within the…
Aug 7 2019, 8:25 AM
JonChesterfield closed D65876: Use forceinline. Necessary for nvcc to inline small functions within the bitcode library.
Aug 7 2019, 8:25 AM · Restricted Project, Restricted Project
JonChesterfield added inline comments to D65836: Factor architecture dependent code out of loop.cu.
Aug 7 2019, 7:55 AM · Restricted Project, Restricted Project
JonChesterfield created D65876: Use forceinline. Necessary for nvcc to inline small functions within the bitcode library.
Aug 7 2019, 7:53 AM · Restricted Project, Restricted Project
JonChesterfield added a comment to D65836: Factor architecture dependent code out of loop.cu.

I would suggest at first to come to an agreement on the design of this reworked library at first.

Aug 7 2019, 7:38 AM · Restricted Project, Restricted Project
JonChesterfield updated the diff for D65836: Factor architecture dependent code out of loop.cu.
  • address review comments
Aug 7 2019, 7:31 AM · Restricted Project, Restricted Project

Aug 6 2019

JonChesterfield updated the diff for D65836: Factor architecture dependent code out of loop.cu.
  • drop omptarget-nvptx include
Aug 6 2019, 5:51 PM · Restricted Project, Restricted Project
JonChesterfield added a comment to D65836: Factor architecture dependent code out of loop.cu.

Couple of comments from me inline.

Aug 6 2019, 5:45 PM · Restricted Project, Restricted Project
JonChesterfield created D65836: Factor architecture dependent code out of loop.cu.
Aug 6 2019, 5:15 PM · Restricted Project, Restricted Project
JonChesterfield updated the summary of D65836: Factor architecture dependent code out of loop.cu.
Aug 6 2019, 5:15 PM · Restricted Project, Restricted Project
JonChesterfield added inline comments to D64217: [OpenMP][NFCI] Cleanup the target state queue implementation.
Aug 6 2019, 10:32 AM · Restricted Project
JonChesterfield added inline comments to D64217: [OpenMP][NFCI] Cleanup the target state queue implementation.
Aug 6 2019, 9:11 AM · Restricted Project
JonChesterfield added inline comments to D64218: [OpenMP][NFCI] Cleanup the target synchronization implementation.
Aug 6 2019, 3:17 AM · Restricted Project

Jul 24 2019

JonChesterfield added inline comments to D64217: [OpenMP][NFCI] Cleanup the target state queue implementation.
Jul 24 2019, 8:03 AM · Restricted Project

May 21 2019

JonChesterfield added a comment to D62132: [RFC] Intrinsics for Hardware Loops.

I've implemented this out of tree (for Graphcore), based loosely on the PPC implementation. IR pass based on SCEV inserts intrinsics, SDag patches them up a little, MIR pass picks appropriate instructions or falls back to a decrement and branch loop.

May 21 2019, 7:30 AM

Feb 11 2019

Herald added a project to D55851: Implement basic loop fusion pass: Restricted Project.
Feb 11 2019, 1:14 PM · Restricted Project

Jan 10 2019

JonChesterfield added a comment to D55940: Detect incorrect FileCheck variable CLI definition.

It looks like the changes to support/commandline are useful independent of the file check use case. Would it be worth submitting the command line enhancement by itself before updating filecheck to take advantage?

Jan 10 2019, 3:09 AM

Jul 18 2017

JonChesterfield accepted rL308339: Fix errors in r308335 and add "REQUIRES: x86" to one more file..
Jul 18 2017, 12:33 PM
JonChesterfield added a comment to rL308339: Fix errors in r308335 and add "REQUIRES: x86" to one more file..

Thank you. I've just seen the buildbot failure but you've beaten me to the patch. Embarrassing error, sorry.

Jul 18 2017, 12:33 PM
JonChesterfield committed rL308335: [LLD] Mark a number of x86 only tests to require x86.
[LLD] Mark a number of x86 only tests to require x86
Jul 18 2017, 11:41 AM
JonChesterfield closed D34685: Mark a number of x86 only tests to require x86 by committing rL308335: [LLD] Mark a number of x86 only tests to require x86.
Jul 18 2017, 11:41 AM
JonChesterfield added a comment to D34685: Mark a number of x86 only tests to require x86.
In D34685#813125, @ruiu wrote:

LGTM

Jul 18 2017, 9:27 AM

Jul 4 2017

JonChesterfield added a comment to D34685: Mark a number of x86 only tests to require x86.
In D34685#796846, @ruiu wrote:

You can distinguish by filename, as target-specific tests start with target name.

Jul 4 2017, 4:13 AM
JonChesterfield added a comment to D34685: Mark a number of x86 only tests to require x86.
In D34685#796778, @ruiu wrote:

I'm still waiting for you to update this patch to address Rafael's comment.

Jul 4 2017, 2:55 AM

Jun 27 2017

JonChesterfield added a comment to D34685: Mark a number of x86 only tests to require x86.

Thanks Paul. This is my first time looking at lld, the guidance is appreciated.

Jun 27 2017, 7:37 AM
JonChesterfield added a reviewer for D34685: Mark a number of x86 only tests to require x86: ruiu.
Jun 27 2017, 7:35 AM
JonChesterfield created D34685: Mark a number of x86 only tests to require x86.
Jun 27 2017, 7:01 AM

Feb 6 2017

JonChesterfield committed rL294230: [TableGen] Use less stack in DAGISelMatcherOpt.
[TableGen] Use less stack in DAGISelMatcherOpt
Feb 6 2017, 11:53 AM
JonChesterfield closed D29080: Use less stack in DAGISelMatcherOpt by committing rL294230: [TableGen] Use less stack in DAGISelMatcherOpt.
Feb 6 2017, 11:53 AM
JonChesterfield committed rL294229: Revert r294228.
Revert r294228
Feb 6 2017, 11:52 AM