Page MenuHomePhabricator

kevinsala (Kevin Sala Penadés)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 10 2022, 7:24 AM (16 w, 2 d)

Recent Activity

Tue, Nov 29

kevinsala added a comment to D138389: [OpenMP][libomptarget] Add AMDGPU NextGen plugin with asynchronous behavior.
Tue, Nov 29, 2:52 PM · Restricted Project, Unknown Object (Project)

Sun, Nov 27

kevinsala requested review of D138769: [OpenMP][libomptarget] Simplify resource managers in NextGen plugins.
Sun, Nov 27, 2:30 PM · Restricted Project, Unknown Object (Project)

Wed, Nov 23

kevinsala added a comment to D138389: [OpenMP][libomptarget] Add AMDGPU NextGen plugin with asynchronous behavior.

How was this tested?

We need a configuration for our regression tests. I thought we had one (or a patch) already. @kevinsala @jhuber6

Wed, Nov 23, 4:16 PM · Restricted Project, Unknown Object (Project)
kevinsala requested review of D138625: [OpenMP][libomptarget] Improve NextGen plugin interface for initialization.
Wed, Nov 23, 3:47 PM · Restricted Project, Unknown Object (Project)
kevinsala requested review of D138619: [OpenMP][libomptarget] Add minor fixes to NextGen plugins.
Wed, Nov 23, 3:17 PM · Restricted Project, Unknown Object (Project)
kevinsala requested review of D138604: [OpenMP][libomptarget] Allow overriding function that gets ELF symbol info.
Wed, Nov 23, 1:35 PM · Restricted Project, Unknown Object (Project)

Sun, Nov 20

kevinsala requested review of D138389: [OpenMP][libomptarget] Add AMDGPU NextGen plugin with asynchronous behavior.
Sun, Nov 20, 6:24 PM · Restricted Project, Unknown Object (Project)

Tue, Nov 15

kevinsala updated the diff for D138002: [OpenMP][libomptarget] Build plugins-nextgen/common/PluginInterface with protected visibility.

Updated to use set_target_properties instead of set_property.

Tue, Nov 15, 6:18 PM · Unknown Object (Project), Restricted Project
kevinsala added a comment to D138002: [OpenMP][libomptarget] Build plugins-nextgen/common/PluginInterface with protected visibility.

@jhuber6 Shouldn't we compile the elf_common "library" with protected visibility too?

It should be fine since everything links from a single library. The previous problem was because we had multiple libraries defining the same functions in the plugins.

Double checked, it should be an object library so it doesn't export anything on the library level, then the version script will ensure that we only export the plugin interfaces.

Tue, Nov 15, 6:15 PM · Unknown Object (Project), Restricted Project

Mon, Nov 14

kevinsala added a comment to D138002: [OpenMP][libomptarget] Build plugins-nextgen/common/PluginInterface with protected visibility.

@jhuber6 Shouldn't we compile the elf_common "library" with protected visibility too?

Mon, Nov 14, 6:55 PM · Unknown Object (Project), Restricted Project
kevinsala requested review of D138002: [OpenMP][libomptarget] Build plugins-nextgen/common/PluginInterface with protected visibility.
Mon, Nov 14, 6:47 PM · Unknown Object (Project), Restricted Project

Nov 2 2022

kevinsala added a comment to D137225: [OpenMP][libomptarget] Fix AsyncInfoTy object in omp_target_memcpy.

Thanks for reviewing it @tianshilei1992. Since I don't have access to the repo, I'd appreciate if you could commit the patch for me. Thanks!

Nov 2 2022, 8:47 AM · Restricted Project, Unknown Object (Project)

Nov 1 2022

kevinsala requested review of D137225: [OpenMP][libomptarget] Fix AsyncInfoTy object in omp_target_memcpy.
Nov 1 2022, 8:39 PM · Restricted Project, Unknown Object (Project)

Oct 27 2022

kevinsala updated the diff for D134396: [OpenMP][libomptarget] New plugin infrastructure and new CUDA plugin.

@jhuber6 This is the patch generated as you commented. I tried to apply to main and applied with no conflicts. If it reports any error, please share the output and I'll fix it. Thanks!

Oct 27 2022, 9:34 AM · Restricted Project, Restricted Project, Unknown Object (Project)

Oct 26 2022

kevinsala updated the diff for D134396: [OpenMP][libomptarget] New plugin infrastructure and new CUDA plugin.

This version has been rebased onto a recent main branch commit. Also, it fixes an error in GenericDeviceTy::dataAlloc that produced two memory allocations per dataAlloc call. Seems that I introduced that bug in one of the intermediate revisions.

Oct 26 2022, 9:05 AM · Restricted Project, Restricted Project, Unknown Object (Project)

Oct 24 2022

kevinsala added inline comments to D134396: [OpenMP][libomptarget] New plugin infrastructure and new CUDA plugin.
Oct 24 2022, 11:35 PM · Restricted Project, Restricted Project, Unknown Object (Project)
kevinsala updated the diff for D134396: [OpenMP][libomptarget] New plugin infrastructure and new CUDA plugin.

This version fixes the types of some variables, such as LoopTripCount.

Oct 24 2022, 11:28 PM · Restricted Project, Restricted Project, Unknown Object (Project)
kevinsala added inline comments to D134396: [OpenMP][libomptarget] New plugin infrastructure and new CUDA plugin.
Oct 24 2022, 7:40 AM · Restricted Project, Restricted Project, Unknown Object (Project)
kevinsala added a comment to D134396: [OpenMP][libomptarget] New plugin infrastructure and new CUDA plugin.

I haven't reviewed this but the following comment stuck out:

where some threads may be consulting an already inserted ELFObjectFile (without holding the map lock) while another potential thread may be inserting a new one (with the lock acquired).

It's never safe to have one thread reading from a hashtable while another writes to it. Even if the keys were always different there is table resizing and hash collisions to worry about.

Either we could use a persistent structure and CAS the new one in place (bit unusual for this codebase) or a single writer multi reader lock, such that multiple readers can make progress at a time but the whole world grinds to a halt for each writer.

Oct 24 2022, 7:07 AM · Restricted Project, Restricted Project, Unknown Object (Project)

Oct 23 2022

kevinsala updated the diff for D134396: [OpenMP][libomptarget] New plugin infrastructure and new CUDA plugin.

This version fixes the last issues commented by @jhuber6.

Oct 23 2022, 10:32 PM · Restricted Project, Restricted Project, Unknown Object (Project)

Oct 22 2022

kevinsala added inline comments to D134396: [OpenMP][libomptarget] New plugin infrastructure and new CUDA plugin.
Oct 22 2022, 5:58 PM · Restricted Project, Restricted Project, Unknown Object (Project)

Oct 18 2022

kevinsala updated the diff for D134396: [OpenMP][libomptarget] New plugin infrastructure and new CUDA plugin.

Updating the dynamic_cuda/cuda.h file to define CUDA_ERROR_TOO_MANY_PEERS and CU_DEVICE_INVALID. The building was failing when no CUDA installation is provided.

Oct 18 2022, 5:13 PM · Restricted Project, Restricted Project, Unknown Object (Project)
kevinsala updated the diff for D134396: [OpenMP][libomptarget] New plugin infrastructure and new CUDA plugin.

This is updated version, rebased on the recent commit 7caae244730eb49b9e3f632225f53902fd956940.

Oct 18 2022, 8:00 AM · Restricted Project, Restricted Project, Unknown Object (Project)

Oct 17 2022

kevinsala added inline comments to D134396: [OpenMP][libomptarget] New plugin infrastructure and new CUDA plugin.
Oct 17 2022, 8:31 AM · Restricted Project, Restricted Project, Unknown Object (Project)
kevinsala updated the diff for D134396: [OpenMP][libomptarget] New plugin infrastructure and new CUDA plugin.

This update fixes the comments of the reviewers. The major changes with respect to the latest version is the removal of TgtError class and use of llvm::Error directly. It also replaces the std::unordered_map with llvm::DenseMap and removes the map's lock too. Additionally, the patch adds a few changes needed by the future "nextgen" AMD plugin, now under development.

Oct 17 2022, 8:30 AM · Restricted Project, Restricted Project, Unknown Object (Project)

Oct 14 2022

kevinsala added inline comments to D134396: [OpenMP][libomptarget] New plugin infrastructure and new CUDA plugin.
Oct 14 2022, 12:57 PM · Restricted Project, Restricted Project, Unknown Object (Project)

Sep 27 2022

kevinsala added inline comments to D134396: [OpenMP][libomptarget] New plugin infrastructure and new CUDA plugin.
Sep 27 2022, 8:26 PM · Restricted Project, Restricted Project, Unknown Object (Project)
kevinsala added inline comments to D134396: [OpenMP][libomptarget] New plugin infrastructure and new CUDA plugin.
Sep 27 2022, 8:04 PM · Restricted Project, Restricted Project, Unknown Object (Project)
kevinsala updated the diff for D134396: [OpenMP][libomptarget] New plugin infrastructure and new CUDA plugin.

This update applies the reviewers' fixes and some other minor fixes. The main change is that now it uses the TgtError class that inherits from llvm::Error for returning and handling code errors.

Sep 27 2022, 8:03 PM · Restricted Project, Restricted Project, Unknown Object (Project)

Sep 23 2022

kevinsala added inline comments to D134396: [OpenMP][libomptarget] New plugin infrastructure and new CUDA plugin.
Sep 23 2022, 12:21 PM · Restricted Project, Restricted Project, Unknown Object (Project)
kevinsala added inline comments to D134396: [OpenMP][libomptarget] New plugin infrastructure and new CUDA plugin.
Sep 23 2022, 12:12 PM · Restricted Project, Restricted Project, Unknown Object (Project)

Sep 22 2022

kevinsala added a comment to D134396: [OpenMP][libomptarget] New plugin infrastructure and new CUDA plugin.

Thanks @jhuber6 @tianshilei1992 for your comments! I'll be addressing those issues. I added inline replies to some specific comments.

Sep 22 2022, 11:59 AM · Restricted Project, Restricted Project, Unknown Object (Project)

Sep 21 2022

kevinsala requested review of D134396: [OpenMP][libomptarget] New plugin infrastructure and new CUDA plugin.
Sep 21 2022, 4:10 PM · Restricted Project, Restricted Project, Unknown Object (Project)

Sep 14 2022

kevinsala abandoned D133655: [OpenMP][libomptarget] Add mutex to safely read/modify HostPinnedAllocs map.

Great! Then I'm closing this review. Thanks @jhuber6

Sep 14 2022, 11:23 AM · Restricted Project, Unknown Object (Project)

Sep 12 2022

kevinsala added a comment to D133655: [OpenMP][libomptarget] Add mutex to safely read/modify HostPinnedAllocs map.

Is this solving an immediate error you're experiencing? If not we should wait for a consensus on removing this map entirely, otherwise if it's urgent we could probably land this temporarily.

Sep 12 2022, 8:40 PM · Restricted Project, Unknown Object (Project)

Sep 10 2022

kevinsala requested review of D133655: [OpenMP][libomptarget] Add mutex to safely read/modify HostPinnedAllocs map.
Sep 10 2022, 6:56 PM · Restricted Project, Unknown Object (Project)

Aug 15 2022

kevinsala added a comment to D131782: [OpenMP][libomptarget] Fix run region async condition.

My GitHub account has linked the following email: ksalapenades@anl.gov

Aug 15 2022, 7:24 AM · Unknown Object (Project), Restricted Project
kevinsala added a comment to D131782: [OpenMP][libomptarget] Fix run region async condition.

Hi Shilei Tian,
I don't have access to push commits to the LLVM repository. Would you mind pushing this fix commit for me?
Thank you in advance

Aug 15 2022, 5:49 AM · Unknown Object (Project), Restricted Project

Aug 12 2022

kevinsala requested review of D131782: [OpenMP][libomptarget] Fix run region async condition.
Aug 12 2022, 8:07 AM · Unknown Object (Project), Restricted Project