AMDGPU: Don't assert on unknown address spaces

[arsenm]

Summary

Assume unknown address spaces behave like some flavor of global
memory.

[rampitec]

How do we get these address spaces? I'd rather error out.

[arsenm]

In D79580#2025685, @rampitec wrote:

How do we get these address spaces? I'd rather error out.

You can use attribute((address_space)) or hand write IR. We should not error on valid IR. Someday a frontend may wish to track special information with a custom address space number, which we can handle as a global alias similar to how x86 accepts any arbitrary address space

[rampitec]

In D79580#2025725, @arsenm wrote:

In D79580#2025685, @rampitec wrote:

How do we get these address spaces? I'd rather error out.

You can use attribute((address_space)) or hand write IR. We should not error on valid IR. Someday a frontend may wish to track special information with a custom address space number, which we can handle as a global alias similar to how x86 accepts any arbitrary address space

It sounds dangerous. An unknown address space has unknown semantics. How could we assume anything about it?

[arsenm]

In D79580#2025743, @rampitec wrote:

In D79580#2025725, @arsenm wrote:

In D79580#2025685, @rampitec wrote:

How do we get these address spaces? I'd rather error out.

You can use attribute((address_space)) or hand write IR. We should not error on valid IR. Someday a frontend may wish to track special information with a custom address space number, which we can handle as a global alias similar to how x86 accepts any arbitrary address space

It sounds dangerous. An unknown address space has unknown semantics. How could we assume anything about it?

It doesn't have unknown semantics, it has target defined semantics. We can interpret them however we want. At a minimum any pointer computation on the values should work, even if load and store don't select

[rampitec]

In D79580#2026107, @arsenm wrote:

In D79580#2025743, @rampitec wrote:

In D79580#2025725, @arsenm wrote:

In D79580#2025685, @rampitec wrote:

How do we get these address spaces? I'd rather error out.

You can use attribute((address_space)) or hand write IR. We should not error on valid IR. Someday a frontend may wish to track special information with a custom address space number, which we can handle as a global alias similar to how x86 accepts any arbitrary address space

It sounds dangerous. An unknown address space has unknown semantics. How could we assume anything about it?

It doesn't have unknown semantics, it has target defined semantics. We can interpret them however we want. At a minimum any pointer computation on the values should work, even if load and store don't select

Is there any particular motivation to add this support?

[arsenm]

In D79580#2026263, @rampitec wrote:

In D79580#2026107, @arsenm wrote:

In D79580#2025743, @rampitec wrote:

In D79580#2025725, @arsenm wrote:

In D79580#2025685, @rampitec wrote:

How do we get these address spaces? I'd rather error out.

You can use attribute((address_space)) or hand write IR. We should not error on valid IR. Someday a frontend may wish to track special information with a custom address space number, which we can handle as a global alias similar to how x86 accepts any arbitrary address space

It sounds dangerous. An unknown address space has unknown semantics. How could we assume anything about it?

It doesn't have unknown semantics, it has target defined semantics. We can interpret them however we want. At a minimum any pointer computation on the values should work, even if load and store don't select

Is there any particular motivation to add this support?

Just generally making a best effort attempt to handle any IR that passes the verifier. We already try to handle these in a variety of other places the same way, and theoretically someone could want to use it.

[b-sumner]

In D79580#2026301, @arsenm wrote:

In D79580#2026263, @rampitec wrote:

In D79580#2026107, @arsenm wrote:

In D79580#2025743, @rampitec wrote:

In D79580#2025725, @arsenm wrote:

In D79580#2025685, @rampitec wrote:

How do we get these address spaces? I'd rather error out.

You can use attribute((address_space)) or hand write IR. We should not error on valid IR. Someday a frontend may wish to track special information with a custom address space number, which we can handle as a global alias similar to how x86 accepts any arbitrary address space

It sounds dangerous. An unknown address space has unknown semantics. How could we assume anything about it?

It doesn't have unknown semantics, it has target defined semantics. We can interpret them however we want. At a minimum any pointer computation on the values should work, even if load and store don't select

Is there any particular motivation to add this support?

Just generally making a best effort attempt to handle any IR that passes the verifier. We already try to handle these in a variety of other places the same way, and theoretically someone could want to use it.

That sounds reasonable. I can think of a situation where it would enable certain investigations.

[rampitec]

This still sounds very suspicious to me to handle IR which we do not really understand. But since there is a consensus, then OK. There are no questions to the code itself.

[rampitec]

Thinking more about it, why do you want to treat it as global? There is one universal address space, generic.

[arsenm]

In D79580#2026459, @rampitec wrote:

Thinking more about it, why do you want to treat it as global? There is one universal address space, generic

Flat is for addressing, not allocation. The non-global flat addressable address spaces need to be specially managed by the compiler, so a user address space aliasing either wouldn’t really work

[arsenm]

78a43f10c701a9cb353e927e9551f32223fbb37d

llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp

 unsigned GCNTTIImpl::getLoadStoreVecRegBitWidth(unsigned AddrSpace) const {
   if (AddrSpace == AMDGPUAS::GLOBAL_ADDRESS ||
       AddrSpace == AMDGPUAS::CONSTANT_ADDRESS ||
       AddrSpace == AMDGPUAS::CONSTANT_ADDRESS_32BIT ||
       AddrSpace == AMDGPUAS::BUFFER_FAT_POINTER) {
     return 512;
   }
 
-  if (AddrSpace == AMDGPUAS::FLAT_ADDRESS ||
-      AddrSpace == AMDGPUAS::LOCAL_ADDRESS ||
-      AddrSpace == AMDGPUAS::REGION_ADDRESS)
-    return 128;
-
   if (AddrSpace == AMDGPUAS::PRIVATE_ADDRESS)
     return 8 * ST->getMaxPrivateElementSize();
 
-  llvm_unreachable("unhandled address space");
+  // Common to flat, global, local and region. Assume for unknown addrspace.
+  return 128;
 }
 
 bool GCNTTIImpl::isLegalToVectorizeMemChain(unsigned ChainSizeInBytes,
                                                unsigned Alignment,
                                                unsigned AddrSpace) const {
   // We allow vectorization of flat stores, even though we may need to decompose
   // them later if they may access private memory. We don't have enough context
   // here, and legalization can handle it.

llvm/lib/Target/AMDGPU/SIISelLowering.cpp

bool SITargetLowering::isLegalAddressingMode(const DataLayout &DL,
   } else if (AS == AMDGPUAS::FLAT_ADDRESS ||
              AS == AMDGPUAS::UNKNOWN_ADDRESS_SPACE) {
     // For an unknown address space, this usually means that this is for some
     // reason being used for pure arithmetic, and not based on some addressing
     // computation. We don't have instructions that compute pointers with any
     // addressing modes, so treat them as having no offset like flat
     // instructions.
     return isLegalFlatAddressingMode(AM);
-  } else {
-    llvm_unreachable("unhandled address space");
   }
+
+  // Assume a user alias of global for unknown address spaces.
+  return isLegalGlobalAddressingMode(AM);
 }
 
 bool SITargetLowering::canMergeStoresTo(unsigned AS, EVT MemVT,
                                         const SelectionDAG &DAG) const {
   if (AS == AMDGPUAS::GLOBAL_ADDRESS || AS == AMDGPUAS::FLAT_ADDRESS) {
     return (MemVT.getSizeInBits() <= 4 * 32);
   } else if (AS == AMDGPUAS::PRIVATE_ADDRESS) {
     unsigned MaxPrivateBits = 8 * getSubtarget()->getMaxPrivateElementSize();

llvm/test/Transforms/CodeGenPrepare/AMDGPU/addressing-modes.ll

+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -S -codegenprepare -mtriple=amdgcn--amdhsa < %s | FileCheck %s
+
+define amdgpu_kernel void @test_sink_as999_small_max_mubuf_offset(i32 addrspace(999)* %out, i8 addrspace(999)* %in) {
+; CHECK-LABEL: @test_sink_as999_small_max_mubuf_offset(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[OUT_GEP:%.*]] = getelementptr i32, i32 addrspace(999)* [[OUT:%.*]], i32 1024
+; CHECK-NEXT:    [[IN_GEP:%.*]] = getelementptr i8, i8 addrspace(999)* [[IN:%.*]], i64 4095
+; CHECK-NEXT:    [[TID:%.*]] = call i32 @llvm.amdgcn.mbcnt.lo(i32 -1, i32 0) #0
+; CHECK-NEXT:    [[TMP0:%.*]] = icmp eq i32 [[TID]], 0
+; CHECK-NEXT:    br i1 [[TMP0]], label [[ENDIF:%.*]], label [[IF:%.*]]
+; CHECK:       if:
+; CHECK-NEXT:    [[TMP1:%.*]] = load i8, i8 addrspace(999)* [[IN_GEP]]
+; CHECK-NEXT:    [[TMP2:%.*]] = sext i8 [[TMP1]] to i32
+; CHECK-NEXT:    br label [[ENDIF]]
+; CHECK:       endif:
+; CHECK-NEXT:    [[X:%.*]] = phi i32 [ [[TMP2]], [[IF]] ], [ 0, [[ENTRY:%.*]] ]
+; CHECK-NEXT:    store i32 [[X]], i32 addrspace(999)* [[OUT_GEP]]
+; CHECK-NEXT:    br label [[DONE:%.*]]
+; CHECK:       done:
+; CHECK-NEXT:    ret void
+;
+entry:
+  %out.gep = getelementptr i32, i32 addrspace(999)* %out, i32 1024
+  %in.gep = getelementptr i8, i8 addrspace(999)* %in, i64 4095
+  %tid = call i32 @llvm.amdgcn.mbcnt.lo(i32 -1, i32 0) #0
+  %tmp0 = icmp eq i32 %tid, 0
+  br i1 %tmp0, label %endif, label %if
+
+if:
+  %tmp1 = load i8, i8 addrspace(999)* %in.gep
+  %tmp2 = sext i8 %tmp1 to i32
+  br label %endif
+
+endif:
+  %x = phi i32 [ %tmp2, %if ], [ 0, %entry ]
+  store i32 %x, i32 addrspace(999)* %out.gep
+  br label %done
+
+done:
+  ret void
+}
+
+declare i32 @llvm.amdgcn.mbcnt.lo(i32, i32) #0
+
+attributes #0 = { nounwind readnone }
+attributes #1 = { nounwind }
+attributes #2 = { nounwind argmemonly }