In D93525#2509796, @jdoerfert wrote:

At the moment this patch defines compatibility as exact string match of bundler entry ID.
[...]
Supporting target ID requires little more work and discussion.

Let's get this in first, then revisit target ID support as we need it.

In D93525#2495374, @saiislam wrote:

In D93525#2493752, @t-tye wrote:

In D93525#2493024, @yaxunl wrote:

can you document this in ClangOffloadBundler.rst ? I think we need a clear description about how clang-offload-bundler knows which file in the .a file belongs to which target.

How does the .a relate to bundled code objects? Does the .a have a number of bundled code objects? If so wouldn't the identity of code objects be defined by the existing bundled code object ABI already documented? If the .a is a set of non-bundled code objects then defining how they are identified is not part of the clang-offload-bundler documentation as there are no bundled code objects involved. It would seem that the documentation belongs with the OpenMP runtime/compiler that is choosing to use .a files in this manner.

Bundles (created using clang-offload-bundler) are passed to llvm-ar to create an archive of bundled objects (*.a file). An archive can have bundles for multiple device types. So, yes, the identity of code objects is defined by the existing bundled code object ABI.
This patch reads such an archive and produces a device-specific archive for each of the target devices given as input. Each device-specific archive contains all the code objects corresponding to that particular device and are written as per llvm archive format.

Here is a snippet of relevant lit run lines:

// RUN: %clang -O0 -target %itanium_abi_triple %s -c -o %t.o

// RUN: echo 'Content of device file 1' > %t.tgt1
// RUN: clang-offload-bundler -type=o -targets=host-%itanium_abi_triple,openmp-amdgcn-amd-amdhsa-gfx900 -inputs=%t.o,%t.tgt1 -outputs=%t.abundle1.o
 
// RUN: echo 'Content of device file 2' > %t.tgt2
// RUN: clang-offload-bundler -type=o -targets=host-%itanium_abi_triple,openmp-amdgcn-amd-amdhsa-gfx900 -inputs=%t.o,%t.tgt2 -outputs=%t.abundle2.o
 
// RUN: llvm-ar cr %t.lib.a %t.abundle1.o %t.abundle2.o

This patch ==>
// RUN: clang-offload-bundler -unbundle -type=a -targets=openmp-amdgcn-amd-amdhsa-gfx900 -inputs=%t.lib.a -outputs=%t.devicelib.a

%t.devicelib.a will contain all devices objects corresponding to gfx900

Though my interest originates from OpenMP side, Device-specific Archive Libraries created like this can be used by other offloading languages like HIP, CUDA, and OpenCL. Pelase refer D81109 for the an earlier patch in the series of patches which will enable this.

In D93525#2493024, @yaxunl wrote:

can you document this in ClangOffloadBundler.rst ? I think we need a clear description about how clang-offload-bundler knows which file in the .a file belongs to which target.

LGTM

LGTM Thanks.

I like what Scott describes as it seems intuitively obvious and cleaner. Are there any concerns for making these changes?

In D92483#2466989, @rampitec wrote:

In D92483#2466967, @t-tye wrote:

In D92483#2466961, @rampitec wrote:

In D92483#2466959, @t-tye wrote:

We did discuss this further and decided that gfx6* should report it supports amdhsa with the restriction that it does not support generic addresses. AMDGPUUsage already notes the restriction in all relevant sections. The LIT tests would also need updating accordingly. The COMgr tests would also need updating.

AMDGPUUsage does not list amdhsa for all 3 SI targets, yet it is going to be accepted with this patch. If that is so then amdhsa has to be listed in the table for these targets.

The table is indicating what runtimes support which ABIs. Since ROCm does not support gfx6* it would still be correct to not list it in the table. It may be useful to have another column to indicate which ABIs are supported by which processors.

Pfff... I do not like it, but OK.

In D92483#2466961, @rampitec wrote:

In D92483#2466959, @t-tye wrote:

We did discuss this further and decided that gfx6* should report it supports amdhsa with the restriction that it does not support generic addresses. AMDGPUUsage already notes the restriction in all relevant sections. The LIT tests would also need updating accordingly. The COMgr tests would also need updating.

AMDGPUUsage does not list amdhsa for all 3 SI targets, yet it is going to be accepted with this patch. If that is so then amdhsa has to be listed in the table for these targets.

In D92483#2466180, @pvellien wrote:

In D92483#2466144, @scott.linder wrote:

In D92483#2463773, @rampitec wrote:

In D92483#2463314, @pvellien wrote:

In D92483#2458755, @rampitec wrote:

But I still do not think this is a right thing to do to accept amdhsa on SI, even if you turn off flat instructions. SI does not support HSA.

As far as I the support of HSA in SI processors. Tony suggested that gfx60x processors should support AMDHSA OS because it is capable of supporting OpenCL 1.2 which not need generic pointers. This topic was discussed in amdgcn weekly and using MUBUF instructions for global address space in gfx60x is decided. The same is also documented in AMDGPU User Guide earlier.

But it is not. In fact AMDGPUUsage.rst says that SI does NOT support amdhsa and it is only supported starting from gfx700:

**GCN GFX6 (Southern Islands (SI))** [AMD-GCN-GFX6]_
-----------------------------------------------------------------------------------------------------------------------
``gfx600``  - ``tahiti``    ``amdgcn``   dGPU                    - Does not      - *pal-amdpal*
                                                                   support
                                                                   generic
                                                                   address
                                                                   space
``gfx601``  - ``pitcairn``  ``amdgcn``   dGPU                    - Does not      - *pal-amdpal*
            - ``verde``                                            support
                                                                   generic
                                                                   address
                                                                   space
``gfx602``  - ``hainan``    ``amdgcn``   dGPU                    - Does not      - *pal-amdpal*
            - ``oland``                                            support
                                                                   generic
                                                                   address
                                                                   space

It is only amdpal listed here.

What was discussed is that we can use buffer instructions for global on SI even if amdhsa is not supported.

At one point, while you were on vacation, the consensus was "support amdhsa OS with gfx6, and fail to compile in the presence of e.g. generic pointers".

Has there been more discussion since then?

Yeah, support amdhsa OS

In D92483#2466151, @rampitec wrote:

In D92483#2466144, @scott.linder wrote:

In D92483#2463773, @rampitec wrote:

In D92483#2463314, @pvellien wrote:

In D92483#2458755, @rampitec wrote:

But I still do not think this is a right thing to do to accept amdhsa on SI, even if you turn off flat instructions. SI does not support HSA.

As far as I the support of HSA in SI processors. Tony suggested that gfx60x processors should support AMDHSA OS because it is capable of supporting OpenCL 1.2 which not need generic pointers. This topic was discussed in amdgcn weekly and using MUBUF instructions for global address space in gfx60x is decided. The same is also documented in AMDGPU User Guide earlier.

But it is not. In fact AMDGPUUsage.rst says that SI does NOT support amdhsa and it is only supported starting from gfx700:

**GCN GFX6 (Southern Islands (SI))** [AMD-GCN-GFX6]_
-----------------------------------------------------------------------------------------------------------------------
``gfx600``  - ``tahiti``    ``amdgcn``   dGPU                    - Does not      - *pal-amdpal*
                                                                   support
                                                                   generic
                                                                   address
                                                                   space
``gfx601``  - ``pitcairn``  ``amdgcn``   dGPU                    - Does not      - *pal-amdpal*
            - ``verde``                                            support
                                                                   generic
                                                                   address
                                                                   space
``gfx602``  - ``hainan``    ``amdgcn``   dGPU                    - Does not      - *pal-amdpal*
            - ``oland``                                            support
                                                                   generic
                                                                   address
                                                                   space

It is only amdpal listed here.

What was discussed is that we can use buffer instructions for global on SI even if amdhsa is not supported.

At one point, while you were on vacation, the consensus was "support amdhsa OS with gfx6, and fail to compile in the presence of e.g. generic pointers".

Has there been more discussion since then?

I see that is not what we document. @t-tye whatis your opinion?

@scott.linder there is no discussion since then.
@rampitec the final consensus is to use buffer instructions for global address space accesses in SI in presence of amdhsa. In other OS types for SI, backend generates buffer instructions for global addr space access already.

LGTM

LGTM

Reorganize scratch description.

Correct typo.

LGTM

LGTM

Fix missing blank line.

LGTM

Update commit message.

LGTM

Reviewed script that produces the new organization of the tests.

Correct typos.