Page MenuHomePhabricator

AMDGPU: Add support for cross address space synchronization scopes (clang)
ClosedPublic

Authored by kzhuravl on Mar 18 2019, 8:26 AM.

Diff Detail

Repository
rC Clang

Event Timeline

kzhuravl created this revision.Mar 18 2019, 8:26 AM

Backend change will be posted later today. I am currently working on adding backend tests.

jfb added a subscriber: __simt__.Mar 18 2019, 9:34 AM
rampitec added inline comments.Mar 18 2019, 10:21 AM
lib/CodeGen/TargetInfo.cpp
7999

if (!Name.empty())

8002

I think subgroup is in the single address space even if sequentially consistent.

kzhuravl updated this revision to Diff 191193.Mar 18 2019, 3:35 PM
kzhuravl marked an inline comment as done.

Address review feedback.

lib/CodeGen/TargetInfo.cpp
8002

I have synced with @t-tye, and it seems like it might not be. @b-sumner, do you know what opencl spec states? Thanks.

kzhuravl retitled this revision from AMDGPU: Add support for cross address space synchronization scopes to AMDGPU: Add support for cross address space synchronization scopes (clang).Mar 18 2019, 3:36 PM
lib/CodeGen/TargetInfo.cpp
8002

As I understand the spec, memory order seq_cst must be consistent with both local- and global-happens-before, so I would say even subgroup is not in the single address space for OpenCL seq_cst.

rampitec added inline comments.Mar 18 2019, 4:28 PM
lib/CodeGen/TargetInfo.cpp
8002

OK. Is it always one-as if not sequentially consistent? I thought we are about to change sequentially consistent case, and not everything else except it.

b-sumner added inline comments.Mar 18 2019, 5:15 PM
lib/CodeGen/TargetInfo.cpp
8002

Right. In OpenCL, only seq_cst can tie together address spaces. But other languages without explicit address spaces will want them tied for other memory orders

This revision is now accepted and ready to land.Mar 18 2019, 5:19 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 25 2019, 1:55 PM
Herald added a subscriber: ebevhan. · View Herald Transcript