[DXIL][Analysis] Implement enough of DXILResourceAnalysis for buffers

[bogner]

This implements the DXILResourceAnalysis pass for dx.TypedBuffer and
dx.RawBuffer types. This should be sufficient to lower
dx.handle.fromBinding for this set of types, but it leaves a number
of TODOs around for other resource types.

This also includes a straightforward print method in ResourceInfo
to make the analysis testable. This is deliberately different than the
printer in lib/Target/DirectX/DXILResource.cpp, which attempts to
print bindings in a format compatible with the comments dxc prints.
We will eventually want to make that functionality driven by this
analysis pass, but it isn't sufficient for testing so we need both.

[llvmbot]

@llvm/pr-subscribers-llvm-ir
@llvm/pr-subscribers-llvm-analysis

@llvm/pr-subscribers-backend-directx

Author: Justin Bogner (bogner)

Changes

This implements the DXILResourceAnalysis pass for dx.TypedBuffer and
dx.RawBuffer types. This should be sufficient to lower
dx.handle.fromBinding for this set of types, but it leaves a number
of TODOs around for other resource types.

This also includes a straightforward print method in ResourceInfo
to make the analysis testable. This is deliberately different than the
printer in lib/Target/DirectX/DXILResource.cpp, which attempts to
print bindings in a format compatible with the comments dxc prints.
We will eventually want to make that functionality driven by this
analysis pass, but it isn't sufficient for testing so we need both.

---

Full diff: https://github.com/llvm/llvm-project/pull/100699.diff

3 Files Affected:

• (modified) llvm/include/llvm/Analysis/DXILResource.h (+3)
• (modified) llvm/lib/Analysis/DXILResource.cpp (+252-2)
• (added) llvm/test/Analysis/DXILResource/buffer-frombinding.ll (+126)

diff --git a/llvm/include/llvm/Analysis/DXILResource.h b/llvm/include/llvm/Analysis/DXILResource.h
index eef526b548f07..88a9d19b83d2d 100644
--- a/llvm/include/llvm/Analysis/DXILResource.h
+++ b/llvm/include/llvm/Analysis/DXILResource.h
@@ -18,6 +18,7 @@
 namespace llvm {
 class CallInst;
 class MDTuple;
+class TargetExtType;
 
 namespace dxil {
 
@@ -213,6 +214,8 @@ class ResourceInfo {
 
   ResourceBinding getBinding() const { return Binding; }
   std::pair<uint32_t, uint32_t> getAnnotateProps() const;
+
+  void print(raw_ostream &OS) const;
 };
 
 } // namespace dxil
diff --git a/llvm/lib/Analysis/DXILResource.cpp b/llvm/lib/Analysis/DXILResource.cpp
index 4efcb0c15d2ff..b6c4ef6550279 100644
--- a/llvm/lib/Analysis/DXILResource.cpp
+++ b/llvm/lib/Analysis/DXILResource.cpp
@@ -8,9 +8,14 @@
 
 #include "llvm/Analysis/DXILResource.h"
 #include "llvm/ADT/APInt.h"
+#include "llvm/IR/Constants.h"
 #include "llvm/IR/DerivedTypes.h"
+#include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/Instructions.h"
+#include "llvm/IR/Intrinsics.h"
+#include "llvm/IR/IntrinsicsDirectX.h"
 #include "llvm/IR/Metadata.h"
+#include "llvm/IR/Module.h"
 #include "llvm/InitializePasses.h"
 
 #define DEBUG_TYPE "dxil-resource"
@@ -331,6 +336,249 @@ std::pair<uint32_t, uint32_t> ResourceInfo::getAnnotateProps() const {
   return {Word0, Word1};
 }
 
+void ResourceInfo::print(raw_ostream &OS) const {
+  OS << "  Symbol: ";
+  Symbol->printAsOperand(OS);
+  OS << "\n";
+
+  OS << "  Name: \"" << Name << "\"\n"
+     << "  Binding:\n"
+     << "    Unique ID: " << Binding.UniqueID << "\n"
+     << "    Space: " << Binding.Space << "\n"
+     << "    Lower Bound: " << Binding.LowerBound << "\n"
+     << "    Size: " << Binding.Size << "\n"
+     << "  Class: " << static_cast<unsigned>(RC) << "\n"
+     << "  Kind: " << static_cast<unsigned>(Kind) << "\n";
+
+  if (isCBuffer()) {
+    OS << "  CBuffer size: " << CBufferSize << "\n";
+  } else if (isSampler()) {
+    OS << "  Sampler Type: " << static_cast<unsigned>(SamplerTy) << "\n";
+  } else {
+    if (isUAV()) {
+      OS << "  Globally Coherent: " << UAVFlags.GloballyCoherent << "\n"
+         << "  HasCounter: " << UAVFlags.HasCounter << "\n"
+         << "  IsROV: " << UAVFlags.IsROV << "\n";
+    }
+    if (isMultiSample())
+      OS << "  Sample Count: " << MultiSample.Count << "\n";
+
+    if (isStruct()) {
+      OS << "  Buffer Stride: " << Struct.Stride << "\n";
+      uint32_t AlignLog2 = Struct.Alignment ? Log2(*Struct.Alignment) : 0;
+      OS << "  Alignment: " << AlignLog2 << "\n";
+    } else if (isTyped()) {
+      OS << "  Element Type: " << static_cast<unsigned>(Typed.ElementTy) << "\n"
+         << "  Element Count: " << static_cast<unsigned>(Typed.ElementCount)
+         << "\n";
+    } else if (isFeedback())
+      OS << "  Feedback Type: " << static_cast<unsigned>(Feedback.Type) << "\n";
+  }
+}
+
+//===----------------------------------------------------------------------===//
+// ResourceMapper
+
+static dxil::ElementType toDXILElementType(Type *Ty, bool IsSigned) {
+  // TODO: Handle unorm, snorm, and packed.
+  Ty = Ty->getScalarType();
+
+  if (Ty->isIntegerTy()) {
+    switch (Ty->getIntegerBitWidth()) {
+    case 16:
+      return IsSigned ? ElementType::I16 : ElementType::U16;
+    case 32:
+      return IsSigned ? ElementType::I32 : ElementType::U32;
+    case 64:
+      return IsSigned ? ElementType::I64 : ElementType::U64;
+    case 1:
+    default:
+      return ElementType::Invalid;
+    }
+  } else if (Ty->isFloatTy()) {
+    return ElementType::F32;
+  } else if (Ty->isDoubleTy()) {
+    return ElementType::F64;
+  } else if (Ty->isHalfTy()) {
+    return ElementType::F16;
+  }
+
+  return ElementType::Invalid;
+}
+
+namespace {
+
+class ResourceMapper {
+  Module &M;
+  LLVMContext &Context;
+  DXILResourceMap &Resources;
+
+  // Unique ID is per resource type to match DXC.
+  uint32_t NextUAV = 0;
+  uint32_t NextSRV = 0;
+  uint32_t NextCBuf = 0;
+  uint32_t NextSmp = 0;
+
+public:
+  ResourceMapper(Module &M,
+                 MapVector<CallInst *, dxil::ResourceInfo> &Resources)
+      : M(M), Context(M.getContext()), Resources(Resources) {}
+
+  void diagnoseHandle(CallInst *CI, const Twine &Msg,
+                      DiagnosticSeverity Severity = DS_Error) {
+    std::string S;
+    raw_string_ostream SS(S);
+    CI->printAsOperand(SS);
+    DiagnosticInfoUnsupported Diag(*CI->getFunction(), Msg + ": " + SS.str(),
+                                   CI->getDebugLoc(), Severity);
+    Context.diagnose(Diag);
+  }
+
+  ResourceInfo *mapBufferType(CallInst *CI, TargetExtType *HandleTy,
+                              bool IsTyped) {
+    if (HandleTy->getNumTypeParameters() != 1 ||
+        HandleTy->getNumIntParameters() != (IsTyped ? 3 : 2)) {
+      diagnoseHandle(CI, Twine("Invalid buffer target type"));
+      return nullptr;
+    }
+
+    Type *ElTy = HandleTy->getTypeParameter(0);
+    unsigned IsWriteable = HandleTy->getIntParameter(0);
+    unsigned IsROV = HandleTy->getIntParameter(1);
+    bool IsSigned = IsTyped && HandleTy->getIntParameter(2);
+
+    ResourceClass RC = IsWriteable ? ResourceClass::UAV : ResourceClass::SRV;
+    ResourceKind Kind;
+    if (IsTyped)
+      Kind = ResourceKind::TypedBuffer;
+    else if (ElTy->isIntegerTy(8))
+      Kind = ResourceKind::RawBuffer;
+    else
+      Kind = ResourceKind::StructuredBuffer;
+
+    // TODO: We need to lower to a typed pointer, can we smuggle the type
+    // through?
+    Value *Symbol = UndefValue::get(PointerType::getUnqual(Context));
+    // TODO: We don't actually keep track of the name right now...
+    StringRef Name = "";
+
+    auto [It, Success] = Resources.try_emplace(CI, RC, Kind, Symbol, Name);
+    assert(Success && "Mapping the same CallInst again?");
+    (void)Success;
+    // We grab a pointer into the map's storage, which isn't generally safe.
+    // Since we're just using this to fill in the info the map won't mutate and
+    // the pointer stays valid for as long as we need it to.
+    ResourceInfo *RI = &(It->second);
+
+    if (RI->isUAV())
+      // TODO: We need analysis for GloballyCoherent and HasCounter
+      RI->setUAV(false, false, IsROV);
+
+    if (RI->isTyped()) {
+      dxil::ElementType ET = toDXILElementType(ElTy, IsSigned);
+      uint32_t Count = 1;
+      if (auto *VTy = dyn_cast<FixedVectorType>(ElTy))
+        Count = VTy->getNumElements();
+      RI->setTyped(ET, Count);
+    } else if (RI->isStruct()) {
+      const DataLayout &DL = M.getDataLayout();
+
+      // This mimics what DXC does. Notably, we only ever set the alignment if
+      // the type is actually a struct type.
+      uint32_t Stride = DL.getTypeAllocSize(ElTy);
+      MaybeAlign Alignment;
+      if (auto *STy = dyn_cast<StructType>(ElTy))
+        Alignment = DL.getStructLayout(STy)->getAlignment();
+      RI->setStruct(Stride, Alignment);
+    }
+
+    return RI;
+  }
+
+  ResourceInfo *mapHandleIntrin(CallInst *CI) {
+    FunctionType *FTy = CI->getFunctionType();
+    Type *RetTy = FTy->getReturnType();
+    auto *HandleTy = dyn_cast<TargetExtType>(RetTy);
+    if (!HandleTy) {
+      diagnoseHandle(CI, "dx.handle.fromBinding requires target type");
+      return nullptr;
+    }
+
+    StringRef TypeName = HandleTy->getName();
+    if (TypeName == "dx.TypedBuffer") {
+      return mapBufferType(CI, HandleTy, /*IsTyped=*/true);
+    } else if (TypeName == "dx.RawBuffer") {
+      return mapBufferType(CI, HandleTy, /*IsTyped=*/false);
+    } else if (TypeName == "dx.CBuffer") {
+      // TODO: implement
+      diagnoseHandle(CI, "dx.CBuffer handles are not implemented yet");
+      return nullptr;
+    } else if (TypeName == "dx.Sampler") {
+      // TODO: implement
+      diagnoseHandle(CI, "dx.Sampler handles are not implemented yet");
+      return nullptr;
+    } else if (TypeName == "dx.Texture") {
+      // TODO: implement
+      diagnoseHandle(CI, "dx.Texture handles are not implemented yet");
+      return nullptr;
+    }
+
+    diagnoseHandle(CI, "Invalid target(dx) type");
+    return nullptr;
+  }
+
+  ResourceInfo *mapHandleFromBinding(CallInst *CI) {
+    assert(CI->getIntrinsicID() == Intrinsic::dx_handle_fromBinding &&
+           "Must be dx.handle.fromBinding intrinsic");
+
+    ResourceInfo *RI = mapHandleIntrin(CI);
+    if (!RI)
+      return nullptr;
+
+    uint32_t NextID;
+    if (RI->isCBuffer())
+      NextID = NextCBuf++;
+    else if (RI->isSampler())
+      NextID = NextSmp++;
+    else if (RI->isUAV())
+      NextID = NextUAV++;
+    else
+      NextID = NextSRV++;
+
+    uint32_t Space = cast<ConstantInt>(CI->getArgOperand(0))->getZExtValue();
+    uint32_t LowerBound =
+        cast<ConstantInt>(CI->getArgOperand(1))->getZExtValue();
+    uint32_t Size = cast<ConstantInt>(CI->getArgOperand(2))->getZExtValue();
+
+    RI->bind(NextID, Space, LowerBound, Size);
+
+    return RI;
+  }
+
+  void mapResources() {
+    for (Function &F : M.functions()) {
+      if (!F.isDeclaration())
+        continue;
+      LLVM_DEBUG(dbgs() << "Function: " << F.getName() << "\n");
+      Intrinsic::ID ID = F.getIntrinsicID();
+      switch (ID) {
+      default:
+        // TODO: handle `dx.op` functions.
+        continue;
+      case Intrinsic::dx_handle_fromBinding:
+        for (User *U : F.users()) {
+          LLVM_DEBUG(dbgs() << "  Visiting: " << *U << "\n");
+          if (CallInst *CI = dyn_cast<CallInst>(U))
+            mapHandleFromBinding(CI);
+        }
+        break;
+      }
+    }
+  }
+};
+
+} // namespace
+
 //===----------------------------------------------------------------------===//
 // DXILResourceAnalysis and DXILResourcePrinterPass
 
@@ -340,6 +588,7 @@ AnalysisKey DXILResourceAnalysis::Key;
 DXILResourceMap DXILResourceAnalysis::run(Module &M,
                                           ModuleAnalysisManager &AM) {
   DXILResourceMap Data;
+  ResourceMapper(M, Data).mapResources();
   return Data;
 }
 
@@ -352,7 +601,7 @@ PreservedAnalyses DXILResourcePrinterPass::run(Module &M,
     OS << "Binding for ";
     Handle->print(OS);
     OS << "\n";
-    // TODO: Info.print(OS);
+    Info.print(OS);
     OS << "\n";
   }
 
@@ -374,6 +623,7 @@ void DXILResourceWrapperPass::getAnalysisUsage(AnalysisUsage &AU) const {
 
 bool DXILResourceWrapperPass::runOnModule(Module &M) {
   ResourceMap.reset(new DXILResourceMap());
+  ResourceMapper(M, *ResourceMap).mapResources();
   return false;
 }
 
@@ -388,7 +638,7 @@ void DXILResourceWrapperPass::print(raw_ostream &OS, const Module *) const {
     OS << "Binding for ";
     Handle->print(OS);
     OS << "\n";
-    // TODO: Info.print(OS);
+    Info.print(OS);
     OS << "\n";
   }
 }
diff --git a/llvm/test/Analysis/DXILResource/buffer-frombinding.ll b/llvm/test/Analysis/DXILResource/buffer-frombinding.ll
new file mode 100644
index 0000000000000..a54b2188fd077
--- /dev/null
+++ b/llvm/test/Analysis/DXILResource/buffer-frombinding.ll
@@ -0,0 +1,126 @@
+; RUN: opt -S -disable-output -passes="print<dxil-resource>" < %s 2>&1 | FileCheck %s
+
+@G = external constant <4 x float>, align 4
+
+define void @test_typedbuffer() {
+  ; RWBuffer<float4> Buf : register(u5, space3)
+  %typed0 = call target("dx.TypedBuffer", <4 x float>, 1, 0, 0)
+              @llvm.dx.handle.fromBinding.tdx.TypedBuffer_f32_1_0(
+                  i32 3, i32 5, i32 1, i32 0, i1 false)
+  ; CHECK: Binding for %typed0
+  ; CHECK:   Symbol: ptr undef
+  ; CHECK:   Name: ""
+  ; CHECK:   Binding:
+  ; CHECK:     Unique ID: 0
+  ; CHECK:     Space: 3
+  ; CHECK:     Lower Bound: 5
+  ; CHECK:     Size: 1
+  ; CHECK:   Class: 1
+  ; CHECK:   Kind: 10
+  ; CHECK:   Globally Coherent: 0
+  ; CHECK:   HasCounter: 0
+  ; CHECK:   IsROV: 0
+  ; CHECK:   Element Type: 9
+  ; CHECK:   Element Count: 4
+
+  ; RWBuffer<int> Buf : register(u7, space2)
+  %typed1 = call target("dx.TypedBuffer", i32, 1, 0, 1)
+      @llvm.dx.handle.fromBinding.tdx.TypedBuffer_i32_1_0t(
+          i32 2, i32 7, i32 1, i32 0, i1 false)
+  ; CHECK: Binding for %typed1
+  ; CHECK:   Symbol: ptr undef
+  ; CHECK:   Name: ""
+  ; CHECK:   Binding:
+  ; CHECK:     Unique ID: 1
+  ; CHECK:     Space: 2
+  ; CHECK:     Lower Bound: 7
+  ; CHECK:     Size: 1
+  ; CHECK:   Class: 1
+  ; CHECK:   Kind: 10
+  ; CHECK:   Globally Coherent: 0
+  ; CHECK:   HasCounter: 0
+  ; CHECK:   IsROV: 0
+  ; CHECK:   Element Type: 4
+  ; CHECK:   Element Count: 1
+
+  ; Buffer<uint4> Buf[24] : register(t3, space5)
+  %typed2 = call target("dx.TypedBuffer", <4 x i32>, 0, 0, 0)
+      @llvm.dx.handle.fromBinding.tdx.TypedBuffer_i32_0_0t(
+          i32 2, i32 7, i32 24, i32 0, i1 false)
+  ; CHECK: Binding for %typed2
+  ; CHECK:   Symbol: ptr undef
+  ; CHECK:   Name: ""
+  ; CHECK:   Binding:
+  ; CHECK:     Unique ID: 0
+  ; CHECK:     Space: 2
+  ; CHECK:     Lower Bound: 7
+  ; CHECK:     Size: 24
+  ; CHECK:   Class: 0
+  ; CHECK:   Kind: 10
+  ; CHECK:   Element Type: 5
+  ; CHECK:   Element Count: 4
+
+  ret void
+}
+
+define void @test_structbuffer() {
+  ; struct S { float4 a; uint4 b; };
+  ; StructuredBuffer<S> Buf : register(t2, space4)
+  %struct0 = call target("dx.RawBuffer", {<4 x float>, <4 x i32>}, 0, 0)
+      @llvm.dx.handle.fromBinding.tdx.RawBuffer_sl_v4f32v4i32s_0_0t(
+          i32 4, i32 2, i32 1, i32 0, i1 false)
+  ; CHECK: Binding for %struct0
+  ; CHECK:   Symbol: ptr undef
+  ; CHECK:   Name: ""
+  ; CHECK:   Binding:
+  ; CHECK:     Unique ID: 1
+  ; CHECK:     Space: 4
+  ; CHECK:     Lower Bound: 2
+  ; CHECK:     Size: 1
+  ; CHECK:   Class: 0
+  ; CHECK:   Kind: 12
+  ; CHECK:   Buffer Stride: 32
+  ; CHECK:   Alignment: 4
+
+  ret void
+}
+
+define void @test_bytebuffer() {
+  ; ByteAddressBuffer Buf : register(t8, space1)
+  %byteaddr0 = call target("dx.RawBuffer", i8, 0, 0)
+      @llvm.dx.handle.fromBinding.tdx.RawBuffer_i8_0_0t(
+          i32 1, i32 8, i32 1, i32 0, i1 false)
+  ; CHECK: Binding for %byteaddr0
+  ; CHECK:   Symbol: ptr undef
+  ; CHECK:   Name: ""
+  ; CHECK:   Binding:
+  ; CHECK:     Unique ID: 2
+  ; CHECK:     Space: 1
+  ; CHECK:     Lower Bound: 8
+  ; CHECK:     Size: 1
+  ; CHECK:   Class: 0
+  ; CHECK:   Kind: 11
+
+  ret void
+}
+
+; Note: We need declarations for each handle.fromBinding in the same
+; order as they appear in source to ensure that we can put our CHECK
+; lines along side the thing they're checking.
+declare target("dx.TypedBuffer", <4 x float>, 1, 0, 0)
+        @llvm.dx.handle.fromBinding.tdx.TypedBuffer_v4f32_1_0_0t(
+        i32, i32, i32, i32, i1) #0
+declare target("dx.TypedBuffer", i32, 1, 0, 1)
+        @llvm.dx.handle.fromBinding.tdx.TypedBuffer_i32_1_0_1t(
+            i32, i32, i32, i32, i1) #0
+declare target("dx.TypedBuffer", <4 x i32>, 0, 0, 0)
+        @llvm.dx.handle.fromBinding.tdx.TypedBuffer_v4i32_0_0_0t(
+            i32, i32, i32, i32, i1) #0
+declare target("dx.RawBuffer", { <4 x float>, <4 x i32> }, 0, 0)
+        @llvm.dx.handle.fromBinding.tdx.RawBuffer_sl_v4f32v4i32s_0_0t(
+            i32, i32, i32, i32, i1) #0
+declare target("dx.RawBuffer", i8, 0, 0)
+        @llvm.dx.handle.fromBinding.tdx.RawBuffer_i8_0_0t(
+            i32, i32, i32, i32, i1) #0
+
+attributes #0 = { nocallback nofree nosync nounwind willreturn memory(none) }

[python3kgae]

Could we use ptr with special address space for RawBuffer and CBuffer instead of handles for clang codeGen output?
Then dx.RawBuffer could be dx.StructuredBuffer, since it doesn't share between RawBuffer and StructuredBuffer anymore.

[bogner]

We could potentially use that approach for RawBuffer, but that's something we'd need to design and it seems awkward to have a separate code path for RawBuffer vs the other buffer and texture types. I'm not sure if it would be worth it or not.

CBuffer probably shouldn't be done like that - it's much more akin to TypedBuffer in that it can only be accessed by 16 byte loads and it has all of the special layout stuff to deal with.

[python3kgae]

For hlsl

struct A {
  float a;
};
ConstantBuffer<A> cb;
ByteAddressBuffer bab;
Buffer<float> tb;

dxc -spirv will generate

%type_ConstantBuffer_A                     = OpTypeStruct %float
%_ptr_Uniform_type_ConstantBuffer_A        = OpTypePointer Uniform %type_ConstantBuffer_A
 %cb                                       = OpVariable %_ptr_Uniform_type_ConstantBuffer_A Uniform

%_runtimearr_uint                          = OpTypeRuntimeArray %uint
%type_ByteAddressBuffer                    = OpTypeStruct %_runtimearr_uint
%_ptr_Uniform_type_ByteAddressBuffer       = OpTypePointer Uniform %type_ByteAddressBuffer
 %bab                                      = OpVariable %_ptr_Uniform_type_ByteAddressBuffer Uniform

%type_buffer_image                      = OpTypeImage %float Buffer 2 0 0 1 R32f
%_ptr_UniformConstant_type_buffer_image = OpTypePointer UniformConstant %type_buffer_image
 %tb                                    = OpVariable %_ptr_UniformConstant_type_buffer_image UniformConstant

Both RawBuffer and ConstantBuffer are pointers of struct while TypedBuffer is pointer of OpTypeImage.
So CBuffer is closer to RawBuffer instead of TypedBuffer.

[bogner]

I don't know about SPIR-V, but just using a struct for a cbuffer in LLVM IR opens up a bunch of complexity about the layout. Consider:

cbuffer FunnyLayout { 
   float2 first;
   float arr[2];
   float2 last;
};

This is layed out like so in a cbuffer, in 3 rows (excuse the ascii art):

 0        4        8       12
 +--------+--------+--------+--------+
 |      first      |        |        |
 +--------+--------+--------+--------+
16       20       24       28
 +--------+--------+--------+--------+
 | arr[0] |        |        |        |
 +--------+--------+--------+--------+
32       36       40       44
 +--------+--------+--------+--------+
 | arr[1] |      last       |        |
 +--------+--------+--------+--------+

To access "last", we do a cbuffer load with index 2 and take the second and third elements.

There are two problems with using a pointer to an llvm struct type for this. First, the layout of {<2 x float>, [2 x float], <2 x float>} is as follows:

0        4        8       12
 +--------+--------+--------+--------+
 |      first      | arr[0] | arr[1] |
 +--------+--------+--------+--------+
16       20
 +--------+--------+
 |      last       |
 +--------+--------+

Second, even if you were to construct a type with a bunch of padding so that it's layed out like a cbuffer, we'd have to enforce that all loads and stores are of exactly 16 bytes on a 16 byte boundary. This is trivial if we have an intrinsic for it, but requires extra validation and logic in various places and special knowledge in optimizations if we just use plain loads and stores.

[python3kgae]

We can use https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/DirectX/CBufferDataLayout.h to calculate the layout or size for cbuffer.
If we create the 16bytes intrinsic at clang codeGen, SPIRV will need more work to make it back to struct.
Or do you mean clang codeGen generate struct pointer, the 16bytes intrinsic just generated at DirectX backend?

[bogner]

FWIW clang codegen for SPIR-V doesn't necessarily need to generate the same thing for SPIR-V and DirectX here. The path that lowers to target intrinsics has hooks per target - see CGHLSLRuntime::convertHLSLSpecificType in #97362 and TargetCodeGenInfo::getOpenCLType to see what I mean. If it ends up making more sense to do things differently for SPIR-V or other targets that aren't DXIL, we have the tools to deal with that.

Even so, there are still issues with "just" making cbuffer a pointer to a struct in a specific address space. We have to do analysis to figure out the type, since the pointer would be opaque. We need to account for the fact that a load of last in my example above must also load arr[1], so optimizations that may want to split those up may need to be undone. These are the things the non-legacy cbuffer layout was trying to avoid, but we're stuck with what we have in DXIL for now.

In any case, we would still need the intrinsic to represent where the cbuffer comes from, so pretty well the only thing that would change here is that the if (TypeName == "dx.CBuffer") check would be some kind of pointer walk and address space check instead, so I'm not really sure that this PR is the best forum for discussing how we implement something that this doesn't yet implement anyway. I'm starting to work on a document describing what I think we should do about cbuffers in general that we will then be able to discuss this further on.

[damyanp]

What's the Unique ID for?

[bogner]

This is the "Unique resource record ID" in the DXIL resource metadata. I'll go ahead and rename it to "RecordID" to make this a bit clearer.

[hekota]

LGTM!

[hekota]

This should probably be i32 5, i32 3, ..?

[amy-kwan]

I think this patch is causing issues on three PPC bots:
https://lab.llvm.org/buildbot/#/builders/95/builds/2574/steps/5/logs/stdio
https://lab.llvm.org/buildbot/#/builders/76/builds/1939/steps/4/logs/stdio
https://lab.llvm.org/buildbot/#/builders/168/builds/2190/steps/6/logs/stdio

Could you please investigate/revert this if a quick fix is not possible?

[joker-eph]

This fails with GCC 7:

/vol/worker/mlir-nvidia/mlir-nvidia-gcc7/llvm.src/llvm/lib/Analysis/DXILResource.cpp: In function ‘constexpr llvm::StringRef getElementTypeName(llvm::dxil::ElementType)’:
/vol/worker/mlir-nvidia/mlir-nvidia-gcc7/llvm.src/llvm/include/llvm/Support/ErrorHandling.h:144:36: error: call to non-constexpr function ‘void llvm::llvm_unreachable_internal(const char*, const char*, unsigned int)’
   ::llvm::llvm_unreachable_internal(msg, __FILE__, __LINE__)
   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~
/vol/worker/mlir-nvidia/mlir-nvidia-gcc7/llvm.src/llvm/lib/Analysis/DXILResource.cpp:126:3: note: in expansion of macro ‘llvm_unreachable’
   llvm_unreachable("Unhandled ElementType");
   ^~~~~~~~~~~~~~~~

[joker-eph]

Sorry, I had to revert to unbreak the bots for now, but feel free to reland with the fix!

[bogner]

Sorry, I had to revert to unbreak the bots for now, but feel free to reland with the fix!

No need to apologize, thanks for getting trunk back to green. Oddly I don't seem to have gotten any emails from bots for these particular failures...