This is an archive of the discontinued LLVM Phabricator instance.

[Clang][OpenMP] Fix shared memory allocation on AMDGPU
AcceptedPublic

Authored by ZwFink on Feb 22 2023, 1:48 PM.

Details

Diff Detail

Event Timeline

ZwFink created this revision.Feb 22 2023, 1:48 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 22 2023, 1:48 PM
ZwFink requested review of this revision.Feb 22 2023, 1:48 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 22 2023, 1:48 PM

Full details and reproducer at this GitHub issue: https://github.com/llvm/llvm-project/issues/60345

ZwFink added subscribers: openmp-commits, Restricted Project.
tianshilei1992 accepted this revision.Feb 22 2023, 1:56 PM

I think this looks reasonable to me. @jdoerfert WDYT? I'm not sure if you need to fix some clang tests. Let's see if Buildbot is happy.

This revision is now accepted and ready to land.Feb 22 2023, 1:56 PM

Please use poison values as place holders instead of undef values as we're trying to get rid of Undef. Thank you!

tianshilei1992 retitled this revision from Fix shared memory allocation on AMDGPU to [Clang][OpenMP] Fix shared memory allocation on AMDGPU.Feb 22 2023, 1:56 PM

I wanted to follow up on this. Is anything additional needed from me?

arsenm added a subscriber: arsenm.Mar 24 2023, 2:00 PM

I wanted to follow up on this. Is anything additional needed from me?

This needs a test (I would hope there is already one that needs updating), and also should switch to using PoisonValue instead of UndefValue