Page MenuHomePhabricator

jprice (James Price)
User

Projects

User does not belong to any projects.

User Details

User Since
Feb 5 2016, 5:41 AM (210 w, 6 d)

Recent Activity

Jul 5 2019

jprice added a comment to D63097: [cmake] Don't add Support/Testing library if tests are not included.

FWIW I've just hit the same issue, and this patch works great for me.

Jul 5 2019, 2:22 PM · Restricted Project

Oct 6 2018

jprice added a comment to D52412: OpenCL: Mark printf format string argument.

This change has caused Clang to start emitting erroneous warnings when using OpenCL's vector specifier, e.g. "%v4f", which doesn't seem to be covered by the format string analysis.

Oct 6 2018, 11:23 AM

Apr 4 2017

jprice added a comment to D31569: [MemCpyOpt] Only replace memcpy with bitcast if address spaces match.

Thanks for the review! Would you be able to commit this on my behalf?

Apr 4 2017, 1:11 PM

Apr 1 2017

jprice created D31569: [MemCpyOpt] Only replace memcpy with bitcast if address spaces match.
Apr 1 2017, 8:01 AM

Mar 20 2017

jprice added a comment to D30830: [OpenCL] Fix extension guards for atomic functions.

Ping - maybe this got lost in the noise.

Mar 20 2017, 3:27 AM

Mar 10 2017

jprice added a comment to D30830: [OpenCL] Fix extension guards for atomic functions.

Thanks - could you commit this on my behalf please?

Mar 10 2017, 10:23 AM
jprice created D30830: [OpenCL] Fix extension guards for atomic functions.
Mar 10 2017, 8:10 AM

Feb 24 2017

jprice abandoned D28300: [InstCombine] Fix address space handling when removing allocas.

This patch is now obsolete - the core issue has now been fixed by D27283. While that fix is a little more conservative (i.e. it doesn't replace the alloca when it sometimes could), it's good enough to fix all of the issues that I was experiencing in my downstream projects, so I'm happy to leave it at that.

Feb 24 2017, 12:49 PM
jprice added a comment to D30347: [InstCombine] Fix bug in pointer replacement.

Thanks for the speedy review!

Feb 24 2017, 11:53 AM
jprice created D30347: [InstCombine] Fix bug in pointer replacement.
Feb 24 2017, 11:28 AM

Feb 14 2017

jprice added a comment to D28300: [InstCombine] Fix address space handling when removing allocas.

Ping!

Feb 14 2017, 4:40 AM

Jan 31 2017

jprice updated the diff for D28300: [InstCombine] Fix address space handling when removing allocas.
  • Remove accidental debug statement
Jan 31 2017, 6:11 AM

Jan 26 2017

jprice added a comment to D28300: [InstCombine] Fix address space handling when removing allocas.

Thanks for the feedback, responses inline.

Jan 26 2017, 8:25 AM
jprice updated the diff for D28300: [InstCombine] Fix address space handling when removing allocas.
  • Address review comments from mehdi_amini
Jan 26 2017, 8:24 AM

Jan 25 2017

jprice added a comment to D28300: [InstCombine] Fix address space handling when removing allocas.

Is this optimization safe in general if the address spaces don't match? Could, for example, some of the users being affected be some target-specific intrinsic that is only valid for certain address spaces?

Jan 25 2017, 4:24 PM
jprice updated the diff for D28300: [InstCombine] Fix address space handling when removing allocas.
  • Handle different global and function arg address spaces properly
  • Use llvm_unreachable instead of assert(false)
  • Rebase to current trunk
Jan 25 2017, 4:23 PM

Jan 20 2017

jprice added a comment to D28300: [InstCombine] Fix address space handling when removing allocas.

I don't know if it's too late, but I'd love for this to make it into the 4.0 release. Although this bug has been present for a while so perhaps isn't strictly a regression, there is a regression in downstream projects that I'm working on due to (valid) changes in the IR generated by Clang since 3.9 which expose this bug.

Jan 20 2017, 4:38 AM

Jan 6 2017

jprice added inline comments to D28300: [InstCombine] Fix address space handling when removing allocas.
Jan 6 2017, 9:15 AM
jprice updated the diff for D28300: [InstCombine] Fix address space handling when removing allocas.
  • Address review comments from arsenm
Jan 6 2017, 9:15 AM

Jan 4 2017

jprice updated the diff for D28300: [InstCombine] Fix address space handling when removing allocas.

Fix memtransfer function type deduction

Jan 4 2017, 12:41 PM
jprice updated subscribers of D28300: [InstCombine] Fix address space handling when removing allocas.
Jan 4 2017, 10:05 AM
jprice retitled D28300: [InstCombine] Fix address space handling when removing allocas from to [InstCombine] Fix address space handling when removing allocas.
Jan 4 2017, 10:03 AM

Sep 15 2016

jprice added a comment to D24580: [SE] Let users specify CUDA path.
In D24580#543785, @jhen wrote:

The hack is in now. Please let me know if it works for you. Thanks!

Sep 15 2016, 9:34 AM
jprice added a comment to D24580: [SE] Let users specify CUDA path.
In D24580#543354, @jhen wrote:

What do you think of the idea of doing a check in Python to see if the library base name is *.framework and then emitting the flag -lframework * instead of the raw path, if so? It's hacky, but it seems like it might work pretty much all the time.

Sep 15 2016, 3:32 AM

Sep 14 2016

jprice added a comment to D24580: [SE] Let users specify CUDA path.
In D24580#543239, @jhen wrote:

Thanks for bringing this up! I added logic to streamexecutor-config to add in the CUDA library for the --libs output if CUDA is enabled. It seems to work on Ubuntu. I'd definitely be glad to know if it works on OS X as well.

Sep 14 2016, 4:01 PM
jprice added a comment to D24580: [SE] Let users specify CUDA path.

Does this also need the CUDA library to be added to streamexecutor-config output for --libs?

Sep 14 2016, 2:35 PM
jprice added inline comments to D24538: [SE] Add CUDA platform.
Sep 14 2016, 5:03 AM

Sep 12 2016

jprice added a comment to D24474: [SE] Stop using llvm-config --cxxflags.

I believe LLVMConfig.cmake defines an LLVM_ENABLE_RTTI variable which can give you this information directly, without the need for the execute_process().

Sep 12 2016, 2:47 PM

Sep 9 2016

jprice added a comment to D24368: [StreamExecutor] Make SE work with an in-tree LLVM build..

I've just noticed that this patch split the Utils target into a separate installed library (libutils.a), which breaks things building against SE using streamexecutor-config.

Sep 9 2016, 4:06 PM
jprice added inline comments to D24368: [StreamExecutor] Make SE work with an in-tree LLVM build..
Sep 9 2016, 9:27 AM

Sep 8 2016

jprice added inline comments to D24353: [SE] RegisteredHostMemory for async device copies.
Sep 8 2016, 4:22 PM

Sep 7 2016

jprice accepted D24302: Add streamexecutor-config.
Sep 7 2016, 4:55 PM
jprice added a comment to D24240: [SE] Add getName method to Device class.

BTW, I don't have commit access, so I'm afraid someone else will have to commit this on my behalf.

Sep 7 2016, 3:15 PM
jprice added a comment to D24302: Add streamexecutor-config.

Maybe I'm doing something wrong, but the single-quotes wrapping the include/lib paths given by --cppflags and --ldflags are not working for me. The compiler fails to find the StreamExecutor headers and libraries, and warns that the library directory doesn't exist. It's almost as if the quotes are being taken as part of the paths.

Sep 7 2016, 3:13 PM
jprice added inline comments to D24302: Add streamexecutor-config.
Sep 7 2016, 12:07 PM

Sep 6 2016

jprice accepted D24269: [SE] Rename PlatformInterfaces to PlatformDevice.

Looks good, with one minor nit.

Sep 6 2016, 10:38 AM

Sep 5 2016

jprice retitled D24240: [SE] Add getName method to Device class from to [SE] Add getName method to Device class.
Sep 5 2016, 11:05 AM
jprice accepted D24213: [SE] Remove Platform*Handle classes.

Looks good to me.

Sep 5 2016, 2:21 AM

Sep 2 2016

jprice accepted D24198: [SE] Doc tweaks.

That all looks nice.

Sep 2 2016, 10:50 AM

Aug 31 2016

jprice added a comment to D24107: [StreamExecutor] getOrDie and dieIfError utils.

Hmm, maybe a false alarm.

Aug 31 2016, 6:10 PM
jprice added a comment to D24107: [StreamExecutor] getOrDie and dieIfError utils.

The getOrDie function seems to be generating an additional 'unchecked error' message when dying. For example, this trivial program (which should obviously generate an error):

Aug 31 2016, 4:44 PM

Aug 30 2016

jprice added inline comments to D24043: [StreamExecutor] Simplify Kernel classes.
Aug 30 2016, 1:32 PM

Aug 26 2016

jprice accepted D23941: [StreamExecutor] Fix KernelSpec Doxygen.

Aha, this explains why I couldn't find these classes in the docs yesterday.

Aug 26 2016, 12:56 PM

Aug 24 2016

jprice added a comment to D23577: [StreamExecutor] Executor add synchronous methods.

Not sure what the protocol is for finding issues in patches that have already been committed - should I be posting to parallel_libs-dev instead?

Aug 24 2016, 11:56 AM

Aug 23 2016

jprice added a comment to D23577: [StreamExecutor] Executor add synchronous methods.

Just some mild pedantry from me.

Aug 23 2016, 12:53 PM

Aug 16 2016

jprice added inline comments to D23577: [StreamExecutor] Executor add synchronous methods.
Aug 16 2016, 3:43 PM
jprice added a comment to D23577: [StreamExecutor] Executor add synchronous methods.

General question: if we have variants of these memcpy methods that take ElementCount parameters to allow for partial copies to/from the device allocations, should we also have variants with an Offset parameter as well to allow for partial copies that don't start at the origin?

Aug 16 2016, 1:31 PM

Aug 12 2016

jprice added a comment to D23333: [StreamExecutor] Add basic Stream operations.

One last nit from me.

Aug 12 2016, 11:31 AM
jprice added a comment to D23333: [StreamExecutor] Add basic Stream operations.

FWIW, this design feels a lot cleaner to me :-)

Aug 12 2016, 3:12 AM

Aug 10 2016

jprice added a comment to D23333: [StreamExecutor] Add basic Stream operations.

I think I understand the motivation for restricting the use of StreamExecutor::launch/memcpy etc to only the Stream class via the StreamKey struct, but I'm not sure I see the need for the StreamExecutorKey struct. It seems like this is currently just protecting the Stream constructor, but is it really a problem if someone can write new Stream(Executor, nullptr) as well as Executor->createStream()? Don't have strong opinions about this, just curious.

Aug 10 2016, 3:43 AM

Aug 5 2016

jprice added inline comments to D23211: [StreamExecutor] Add DeviceMemory and kernel arg packing.
Aug 5 2016, 12:35 PM

Aug 4 2016

jprice added inline comments to D23138: [StreamExecutor] Add kernel types.
Aug 4 2016, 5:10 AM

May 16 2016

jprice added inline comments to D18369: [OpenCL] Upstreaming khronos OpenCL header file..
May 16 2016, 9:43 AM

Mar 31 2016

jprice added a comment to D18369: [OpenCL] Upstreaming khronos OpenCL header file..

Is there any chance we could factor out the common bits into separate files to avoid large code repetition? I would imagine it should be quite doable as libs of each standard contain incremental changes.

I saw some inconsistencies in the common part of the 1.2 and 2.0 headers. I will try to consolidate them first then try to split.

Mar 31 2016, 8:57 AM

Mar 9 2016

jprice updated subscribers of D9888: [OPENMP] Driver support for OpenMP offloading.
Mar 9 2016, 9:03 AM

Mar 4 2016

jprice updated subscribers of D14253: [OpenMP] Initial implementation of OpenMP offloading library - libomptarget plugins..
Mar 4 2016, 5:23 AM
jprice updated subscribers of D14254: [OpenMP] Initial implementation of OpenMP offloading library - libomptarget device RTLs..
Mar 4 2016, 5:23 AM · Restricted Project, Restricted Project

Feb 5 2016

jprice updated subscribers of D14031: [OpenMP] Initial implementation of OpenMP offloading library - libomptarget..
Feb 5 2016, 9:18 AM