[PATCH] D130131: [HLSL] CodeGen hlsl cbuffer/tbuffer.

[Xiang Li via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
python3kgae marked an inline comment as done.
Closed by commit rGebe9c7f3e2da: [HLSL] CodeGen hlsl cbuffer/tbuffer. (authored 
by python3kgae).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130131/new/

https://reviews.llvm.org/D130131

Files:
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGHLSLRuntime.cpp
  clang/lib/CodeGen/CGHLSLRuntime.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGenHLSL/cbuf.hlsl
  clang/test/CodeGenHLSL/cbuf_in_namespace.hlsl
  clang/test/CodeGenHLSL/static_global_and_function_in_cb.hlsl

Index: clang/test/CodeGenHLSL/static_global_and_function_in_cb.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/static_global_and_function_in_cb.hlsl
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -finclude-default-header -x hlsl -triple \
+// RUN:   dxil-pc-shadermodel6.3-library %s \
+// RUN:   -emit-llvm -disable-llvm-passes -o - | FileCheck %s
+
+// CHECK-DAG: @[[CB:.+]] = external constant { float }
+
+cbuffer A {
+float a;
+  // CHECK-DAG:@b = internal global float 3.00e+00, align 4
+  static float b = 3;
+  // CHECK:load float, ptr @[[CB]], align 4
+  // CHECK:load float, ptr @b, align 4
+  float foo() { return a + b; }
+}
+
+float bar() {
+  return foo();
+}
Index: clang/test/CodeGenHLSL/cbuf_in_namespace.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/cbuf_in_namespace.hlsl
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -finclude-default-header -x hlsl -triple \
+// RUN:   dxil-pc-shadermodel6.3-library %s \
+// RUN:   -emit-llvm -disable-llvm-passes -o - | FileCheck %s
+
+// Make sure cbuffer inside namespace works.
+// CHECK: @[[CB:.+]] = external constant { float }
+// CHECK: @[[TB:.+]] = external constant { float }
+namespace n0 {
+namespace n1 {
+  cbuffer A {
+float a;
+  }
+}
+  tbuffer B {
+float b;
+  }
+}
+
+float foo() {
+// CHECK: load float, ptr @[[CB]], align 4
+// CHECK: load float, ptr @[[TB]], align 4
+  return n0::n1::a + n0::b;
+}
Index: clang/test/CodeGenHLSL/cbuf.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/cbuf.hlsl
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -finclude-default-header -x hlsl -triple \
+// RUN:   dxil-pc-shadermodel6.3-library %s \
+// RUN:   -emit-llvm -disable-llvm-passes -o - | FileCheck %s
+
+// CHECK: @[[CB:.+]] = external constant { float, double }
+cbuffer A : register(b0, space1) {
+  float a;
+  double b;
+}
+
+// CHECK: @[[TB:.+]] = external constant { float, double }
+tbuffer A : register(t2, space1) {
+  float c;
+  double d;
+}
+
+float foo() {
+// CHECK: load float, ptr @[[CB]], align 4
+// CHECK: load double, ptr getelementptr inbounds ({ float, double }, ptr @[[CB]], i32 0, i32 1), align 8
+// CHECK: load float, ptr @[[TB]], align 4
+// CHECK: load double, ptr getelementptr inbounds ({ float, double }, ptr @[[TB]], i32 0, i32 1), align 8
+  return a + b + c*d;
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -6451,6 +6451,10 @@
 DI->EmitAndRetainType(getContext().getEnumType(cast(D)));
 break;
 
+  case Decl::HLSLBuffer:
+getHLSLRuntime().addBuffer(cast(D));
+break;
+
   default:
 // Make sure we handled everything we should, every other kind is a
 // non-top-level decl.  FIXME: Would be nice to have an isTopLevelDeclKind
Index: clang/lib/CodeGen/CGHLSLRuntime.h
===
--- clang/lib/CodeGen/CGHLSLRuntime.h
+++ clang/lib/CodeGen/CGHLSLRuntime.h
@@ -19,13 +19,25 @@
 
 #include "clang/Basic/HLSLRuntime.h"
 
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+
+#include 
+
 namespace llvm {
 class GlobalVariable;
 class Function;
+class StructType;
 } // namespace llvm
+
 namespace clang {
 class VarDecl;
 class ParmVarDecl;
+class HLSLBufferDecl;
+class CallExpr;
+class Type;
+class DeclContext;
 
 class FunctionDecl;
 
@@ -34,6 +46,19 @@
 class CodeGenModule;
 
 class CGHLSLRuntime {
+public:
+  struct Buffer {
+Buffer(const HLSLBufferDecl *D);
+llvm::StringRef Name;
+// IsCBuffer - Whether the buffer is a cbuffer (and not a tbuffer).
+bool IsCBuffer;
+llvm::Optional Reg;
+unsigned Space;
+// Global variable and offset for each constant.
+std::vector> Constants;
+llvm::StructType *LayoutStruct = nullptr;
+  };
+
 protected:
   CodeGenModule 
   uint32_t ResourceCounters[static_cast(
@@ -48,11 +73,18 @@
   void annotateHLSLResource(const VarDecl *D, llvm::GlobalVariable *GV);
   void generateGlobalCtorDtorCalls();
 
+  void addBuffer(const 

[PATCH] D130131: [HLSL] CodeGen hlsl cbuffer/tbuffer.

[Eli Friedman via Phabricator via cfe-commits]

efriedma accepted this revision.
efriedma added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:112
+// Replace.
+GV->replaceAllUsesWith(GEP);
+// Erase GV.

python3kgae wrote:
> efriedma wrote:
> > Messing with globals like this feels a little weird, but I guess it's fine 
> > if nothing actually tries to use the erased globals after this code runs.  
> > I'm a little concerned that someone might accidentally rearrange the 
> > relevant code in the future (CodeGenModule has a bunch of maps which aren't 
> > cleared before this code runs).
> These globals should not be in any map except Buf.Constants which is used 
> here.
> If another map has these globals, we should remove them from the map.
> Cannot see any other map has these globals now.
Oh, I see.  Probably not an issue in that case.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130131/new/

https://reviews.llvm.org/D130131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130131: [HLSL] CodeGen hlsl cbuffer/tbuffer.

[Xiang Li via Phabricator via cfe-commits]

python3kgae marked an inline comment as done.
python3kgae added inline comments.



Comment at: clang/lib/CodeGen/CGDecl.cpp:131
   case Decl::RequiresExprBody:
+  case Decl::HLSLBuffer:
 // None of these decls require codegen support.

efriedma wrote:
> I'm a little confused by this.  If it's possible to declare an HLSLBuffer 
> inside a function, why don't you need to handle it?  If it isn't possible to 
> declare an HLSLBuffer this way, can you just move this to use the 
> llvm_unreachable()?
Nice catch.
Fixed.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:112
+// Replace.
+GV->replaceAllUsesWith(GEP);
+// Erase GV.

efriedma wrote:
> Messing with globals like this feels a little weird, but I guess it's fine if 
> nothing actually tries to use the erased globals after this code runs.  I'm a 
> little concerned that someone might accidentally rearrange the relevant code 
> in the future (CodeGenModule has a bunch of maps which aren't cleared before 
> this code runs).
These globals should not be in any map except Buf.Constants which is used here.
If another map has these globals, we should remove them from the map.
Cannot see any other map has these globals now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130131/new/

https://reviews.llvm.org/D130131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130131: [HLSL] CodeGen hlsl cbuffer/tbuffer.

[Xiang Li via Phabricator via cfe-commits]

python3kgae updated this revision to Diff 467318.
python3kgae marked an inline comment as done.
python3kgae added a comment.

Go llvm_unreachable is hit HLSLBuffer inside function.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130131/new/

https://reviews.llvm.org/D130131

Files:
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGHLSLRuntime.cpp
  clang/lib/CodeGen/CGHLSLRuntime.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGenHLSL/cbuf.hlsl
  clang/test/CodeGenHLSL/cbuf_in_namespace.hlsl
  clang/test/CodeGenHLSL/static_global_and_function_in_cb.hlsl

Index: clang/test/CodeGenHLSL/static_global_and_function_in_cb.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/static_global_and_function_in_cb.hlsl
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -finclude-default-header -x hlsl -triple \
+// RUN:   dxil-pc-shadermodel6.3-library %s \
+// RUN:   -emit-llvm -disable-llvm-passes -o - | FileCheck %s
+
+// CHECK-DAG: @[[CB:.+]] = external constant { float }
+
+cbuffer A {
+float a;
+  // CHECK-DAG:@b = internal global float 3.00e+00, align 4
+  static float b = 3;
+  // CHECK:load float, ptr @[[CB]], align 4
+  // CHECK:load float, ptr @b, align 4
+  float foo() { return a + b; }
+}
+
+float bar() {
+  return foo();
+}
Index: clang/test/CodeGenHLSL/cbuf_in_namespace.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/cbuf_in_namespace.hlsl
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -finclude-default-header -x hlsl -triple \
+// RUN:   dxil-pc-shadermodel6.3-library %s \
+// RUN:   -emit-llvm -disable-llvm-passes -o - | FileCheck %s
+
+// Make sure cbuffer inside namespace works.
+// CHECK: @[[CB:.+]] = external constant { float }
+// CHECK: @[[TB:.+]] = external constant { float }
+namespace n0 {
+namespace n1 {
+  cbuffer A {
+float a;
+  }
+}
+  tbuffer B {
+float b;
+  }
+}
+
+float foo() {
+// CHECK: load float, ptr @[[CB]], align 4
+// CHECK: load float, ptr @[[TB]], align 4
+  return n0::n1::a + n0::b;
+}
Index: clang/test/CodeGenHLSL/cbuf.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/cbuf.hlsl
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -finclude-default-header -x hlsl -triple \
+// RUN:   dxil-pc-shadermodel6.3-library %s \
+// RUN:   -emit-llvm -disable-llvm-passes -o - | FileCheck %s
+
+// CHECK: @[[CB:.+]] = external constant { float, double }
+cbuffer A : register(b0, space1) {
+  float a;
+  double b;
+}
+
+// CHECK: @[[TB:.+]] = external constant { float, double }
+tbuffer A : register(t2, space1) {
+  float c;
+  double d;
+}
+
+float foo() {
+// CHECK: load float, ptr @[[CB]], align 4
+// CHECK: load double, ptr getelementptr inbounds ({ float, double }, ptr @[[CB]], i32 0, i32 1), align 8
+// CHECK: load float, ptr @[[TB]], align 4
+// CHECK: load double, ptr getelementptr inbounds ({ float, double }, ptr @[[TB]], i32 0, i32 1), align 8
+  return a + b + c*d;
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -6448,6 +6448,10 @@
 DI->EmitAndRetainType(getContext().getEnumType(cast(D)));
 break;
 
+  case Decl::HLSLBuffer:
+getHLSLRuntime().addBuffer(cast(D));
+break;
+
   default:
 // Make sure we handled everything we should, every other kind is a
 // non-top-level decl.  FIXME: Would be nice to have an isTopLevelDeclKind
Index: clang/lib/CodeGen/CGHLSLRuntime.h
===
--- clang/lib/CodeGen/CGHLSLRuntime.h
+++ clang/lib/CodeGen/CGHLSLRuntime.h
@@ -19,13 +19,25 @@
 
 #include "clang/Basic/HLSLRuntime.h"
 
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+
+#include 
+
 namespace llvm {
 class GlobalVariable;
 class Function;
+class StructType;
 } // namespace llvm
+
 namespace clang {
 class VarDecl;
 class ParmVarDecl;
+class HLSLBufferDecl;
+class CallExpr;
+class Type;
+class DeclContext;
 
 class FunctionDecl;
 
@@ -34,6 +46,19 @@
 class CodeGenModule;
 
 class CGHLSLRuntime {
+public:
+  struct Buffer {
+Buffer(const HLSLBufferDecl *D);
+llvm::StringRef Name;
+// IsCBuffer - Whether the buffer is a cbuffer (and not a tbuffer).
+bool IsCBuffer;
+llvm::Optional Reg;
+unsigned Space;
+// Global variable and offset for each constant.
+std::vector> Constants;
+llvm::StructType *LayoutStruct = nullptr;
+  };
+
 protected:
   CodeGenModule 
   uint32_t ResourceCounters[static_cast(
@@ -48,11 +73,18 @@
   void annotateHLSLResource(const VarDecl *D, llvm::GlobalVariable *GV);
   void generateGlobalCtorDtorCalls();
 
+  void addBuffer(const HLSLBufferDecl *D);
   void finishCodeGen();
 
   void setHLSLEntryAttributes(const FunctionDecl 

[PATCH] D130131: [HLSL] CodeGen hlsl cbuffer/tbuffer.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: clang/lib/CodeGen/CGDecl.cpp:131
   case Decl::RequiresExprBody:
+  case Decl::HLSLBuffer:
 // None of these decls require codegen support.

I'm a little confused by this.  If it's possible to declare an HLSLBuffer 
inside a function, why don't you need to handle it?  If it isn't possible to 
declare an HLSLBuffer this way, can you just move this to use the 
llvm_unreachable()?



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:112
+// Replace.
+GV->replaceAllUsesWith(GEP);
+// Erase GV.

Messing with globals like this feels a little weird, but I guess it's fine if 
nothing actually tries to use the erased globals after this code runs.  I'm a 
little concerned that someone might accidentally rearrange the relevant code in 
the future (CodeGenModule has a bunch of maps which aren't cleared before this 
code runs).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130131/new/

https://reviews.llvm.org/D130131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130131: [HLSL] CodeGen hlsl cbuffer/tbuffer.

[Xiang Li via Phabricator via cfe-commits]

python3kgae added a comment.

gentle ping.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130131/new/

https://reviews.llvm.org/D130131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130131: [HLSL] CodeGen hlsl cbuffer/tbuffer.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a reviewer: asl.
beanz added a subscriber: asl.
beanz added a comment.

+@asl for codegen owner perspective.

This LGTM too. The changes here are well isolated to HLSL, so they should have 
no adverse impact on other language support.

@efriedma, @asl & @rjmccall any feedback?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130131/new/

https://reviews.llvm.org/D130131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130131: [HLSL] CodeGen hlsl cbuffer/tbuffer.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: rjmccall, efriedma.
aaron.ballman added a comment.

No additional comments from me, but I don't feel comfortable signing off on the 
codegen bits; adding some codegen code owners.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130131/new/

https://reviews.llvm.org/D130131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130131: [HLSL] CodeGen hlsl cbuffer/tbuffer.

[Xiang Li via Phabricator via cfe-commits]

python3kgae updated this revision to Diff 462289.
python3kgae marked 2 inline comments as done.
python3kgae edited the summary of this revision.
python3kgae added a comment.

Create issue for FIXME.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130131/new/

https://reviews.llvm.org/D130131

Files:
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGHLSLRuntime.cpp
  clang/lib/CodeGen/CGHLSLRuntime.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGenHLSL/cbuf.hlsl
  clang/test/CodeGenHLSL/cbuf_in_namespace.hlsl
  clang/test/CodeGenHLSL/static_global_and_function_in_cb.hlsl

Index: clang/test/CodeGenHLSL/static_global_and_function_in_cb.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/static_global_and_function_in_cb.hlsl
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -finclude-default-header -x hlsl -triple \
+// RUN:   dxil-pc-shadermodel6.3-library %s \
+// RUN:   -emit-llvm -disable-llvm-passes -o - | FileCheck %s
+
+// CHECK-DAG: @[[CB:.+]] = external constant { float }
+
+cbuffer A {
+float a;
+  // CHECK-DAG:@b = internal global float 3.00e+00, align 4
+  static float b = 3;
+  // CHECK:load float, ptr @[[CB]], align 4
+  // CHECK:load float, ptr @b, align 4
+  float foo() { return a + b; }
+}
+
+float bar() {
+  return foo();
+}
Index: clang/test/CodeGenHLSL/cbuf_in_namespace.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/cbuf_in_namespace.hlsl
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -finclude-default-header -x hlsl -triple \
+// RUN:   dxil-pc-shadermodel6.3-library %s \
+// RUN:   -emit-llvm -disable-llvm-passes -o - | FileCheck %s
+
+// Make sure cbuffer inside namespace works.
+// CHECK: @[[CB:.+]] = external constant { float }
+// CHECK: @[[TB:.+]] = external constant { float }
+namespace n0 {
+namespace n1 {
+  cbuffer A {
+float a;
+  }
+}
+  tbuffer B {
+float b;
+  }
+}
+
+float foo() {
+// CHECK: load float, ptr @[[CB]], align 4
+// CHECK: load float, ptr @[[TB]], align 4
+  return n0::n1::a + n0::b;
+}
Index: clang/test/CodeGenHLSL/cbuf.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/cbuf.hlsl
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -finclude-default-header -x hlsl -triple \
+// RUN:   dxil-pc-shadermodel6.3-library %s \
+// RUN:   -emit-llvm -disable-llvm-passes -o - | FileCheck %s
+
+// CHECK: @[[CB:.+]] = external constant { float, double }
+cbuffer A : register(b0, space1) {
+  float a;
+  double b;
+}
+
+// CHECK: @[[TB:.+]] = external constant { float, double }
+tbuffer A : register(t2, space1) {
+  float c;
+  double d;
+}
+
+float foo() {
+// CHECK: load float, ptr @[[CB]], align 4
+// CHECK: load double, ptr getelementptr inbounds ({ float, double }, ptr @[[CB]], i32 0, i32 1), align 8
+// CHECK: load float, ptr @[[TB]], align 4
+// CHECK: load double, ptr getelementptr inbounds ({ float, double }, ptr @[[TB]], i32 0, i32 1), align 8
+  return a + b + c*d;
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -6448,6 +6448,10 @@
 DI->EmitAndRetainType(getContext().getEnumType(cast(D)));
 break;
 
+  case Decl::HLSLBuffer:
+getHLSLRuntime().addBuffer(cast(D));
+break;
+
   default:
 // Make sure we handled everything we should, every other kind is a
 // non-top-level decl.  FIXME: Would be nice to have an isTopLevelDeclKind
Index: clang/lib/CodeGen/CGHLSLRuntime.h
===
--- clang/lib/CodeGen/CGHLSLRuntime.h
+++ clang/lib/CodeGen/CGHLSLRuntime.h
@@ -19,13 +19,25 @@
 
 #include "clang/Basic/HLSLRuntime.h"
 
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+
+#include 
+
 namespace llvm {
 class GlobalVariable;
 class Function;
+class StructType;
 } // namespace llvm
+
 namespace clang {
 class VarDecl;
 class ParmVarDecl;
+class HLSLBufferDecl;
+class CallExpr;
+class Type;
+class DeclContext;
 
 class FunctionDecl;
 
@@ -34,6 +46,19 @@
 class CodeGenModule;
 
 class CGHLSLRuntime {
+public:
+  struct Buffer {
+Buffer(const HLSLBufferDecl *D);
+llvm::StringRef Name;
+// IsCBuffer - Whether the buffer is a cbuffer (and not a tbuffer).
+bool IsCBuffer;
+llvm::Optional Reg;
+unsigned Space;
+// Global variable and offset for each constant.
+std::vector> Constants;
+llvm::StructType *LayoutStruct = nullptr;
+  };
+
 protected:
   CodeGenModule 
   uint32_t ResourceCounters[static_cast(
@@ -48,11 +73,18 @@
   void annotateHLSLResource(const VarDecl *D, llvm::GlobalVariable *GV);
   void generateGlobalCtorDtorCalls();
 
+  void addBuffer(const HLSLBufferDecl *D);
   void finishCodeGen();
 
   void 

[PATCH] D130131: [HLSL] CodeGen hlsl cbuffer/tbuffer.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

This is looking great. Thank you for all your work iterating on this.

Please update the description to reflect the changes in implementation.




Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:120
+
+void addResourceBinding(GlobalVariable *GV, CGHLSLRuntime::Buffer ) {
+  // FIXME: add resource binding to GV.

Please remove the empty function. We can add it when it does something. Also 
make sure we have an issue for the FIXME.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:141
+
+  // FIXME: support packoffset.
+  uint32_t Offset = 0;

Please file an issue for this or link to the issue if it exists.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130131/new/

https://reviews.llvm.org/D130131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130131: [HLSL] CodeGen hlsl cbuffer/tbuffer.

[Xiang Li via Phabricator via cfe-commits]

python3kgae updated this revision to Diff 462280.
python3kgae marked 8 inline comments as done.
python3kgae added a comment.

Code cleanup to match comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130131/new/

https://reviews.llvm.org/D130131

Files:
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGHLSLRuntime.cpp
  clang/lib/CodeGen/CGHLSLRuntime.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGenHLSL/cbuf.hlsl
  clang/test/CodeGenHLSL/cbuf_in_namespace.hlsl
  clang/test/CodeGenHLSL/static_global_and_function_in_cb.hlsl

Index: clang/test/CodeGenHLSL/static_global_and_function_in_cb.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/static_global_and_function_in_cb.hlsl
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -finclude-default-header -x hlsl -triple \
+// RUN:   dxil-pc-shadermodel6.3-library %s \
+// RUN:   -emit-llvm -disable-llvm-passes -o - | FileCheck %s
+
+// CHECK-DAG: @[[CB:.+]] = external constant { float }
+
+cbuffer A {
+float a;
+  // CHECK-DAG:@b = internal global float 3.00e+00, align 4
+  static float b = 3;
+  // CHECK:load float, ptr @[[CB]], align 4
+  // CHECK:load float, ptr @b, align 4
+  float foo() { return a + b; }
+}
+
+float bar() {
+  return foo();
+}
Index: clang/test/CodeGenHLSL/cbuf_in_namespace.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/cbuf_in_namespace.hlsl
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -finclude-default-header -x hlsl -triple \
+// RUN:   dxil-pc-shadermodel6.3-library %s \
+// RUN:   -emit-llvm -disable-llvm-passes -o - | FileCheck %s
+
+// Make sure cbuffer inside namespace works.
+// CHECK: @[[CB:.+]] = external constant { float }
+// CHECK: @[[TB:.+]] = external constant { float }
+namespace n0 {
+namespace n1 {
+  cbuffer A {
+float a;
+  }
+}
+  tbuffer B {
+float b;
+  }
+}
+
+float foo() {
+// CHECK: load float, ptr @[[CB]], align 4
+// CHECK: load float, ptr @[[TB]], align 4
+  return n0::n1::a + n0::b;
+}
Index: clang/test/CodeGenHLSL/cbuf.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/cbuf.hlsl
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -finclude-default-header -x hlsl -triple \
+// RUN:   dxil-pc-shadermodel6.3-library %s \
+// RUN:   -emit-llvm -disable-llvm-passes -o - | FileCheck %s
+
+// CHECK: @[[CB:.+]] = external constant { float, double }
+cbuffer A : register(b0, space1) {
+  float a;
+  double b;
+}
+
+// CHECK: @[[TB:.+]] = external constant { float, double }
+tbuffer A : register(t2, space1) {
+  float c;
+  double d;
+}
+
+float foo() {
+// CHECK: load float, ptr @[[CB]], align 4
+// CHECK: load double, ptr getelementptr inbounds ({ float, double }, ptr @[[CB]], i32 0, i32 1), align 8
+// CHECK: load float, ptr @[[TB]], align 4
+// CHECK: load double, ptr getelementptr inbounds ({ float, double }, ptr @[[TB]], i32 0, i32 1), align 8
+  return a + b + c*d;
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -6448,6 +6448,10 @@
 DI->EmitAndRetainType(getContext().getEnumType(cast(D)));
 break;
 
+  case Decl::HLSLBuffer:
+getHLSLRuntime().addBuffer(cast(D));
+break;
+
   default:
 // Make sure we handled everything we should, every other kind is a
 // non-top-level decl.  FIXME: Would be nice to have an isTopLevelDeclKind
Index: clang/lib/CodeGen/CGHLSLRuntime.h
===
--- clang/lib/CodeGen/CGHLSLRuntime.h
+++ clang/lib/CodeGen/CGHLSLRuntime.h
@@ -19,13 +19,25 @@
 
 #include "clang/Basic/HLSLRuntime.h"
 
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+
+#include 
+
 namespace llvm {
 class GlobalVariable;
 class Function;
+class StructType;
 } // namespace llvm
+
 namespace clang {
 class VarDecl;
 class ParmVarDecl;
+class HLSLBufferDecl;
+class CallExpr;
+class Type;
+class DeclContext;
 
 class FunctionDecl;
 
@@ -34,6 +46,19 @@
 class CodeGenModule;
 
 class CGHLSLRuntime {
+public:
+  struct Buffer {
+Buffer(const HLSLBufferDecl *D);
+llvm::StringRef Name;
+// IsCBuffer - Whether the buffer is a cbuffer (and not a tbuffer).
+bool IsCBuffer;
+llvm::Optional Reg;
+unsigned Space;
+// Global variable and offset for each constant.
+std::vector> Constants;
+llvm::StructType *LayoutStruct = nullptr;
+  };
+
 protected:
   CodeGenModule 
   uint32_t ResourceCounters[static_cast(
@@ -48,11 +73,18 @@
   void annotateHLSLResource(const VarDecl *D, llvm::GlobalVariable *GV);
   void generateGlobalCtorDtorCalls();
 
+  void addBuffer(const HLSLBufferDecl *D);
   void finishCodeGen();
 
   void setHLSLEntryAttributes(const FunctionDecl *FD, llvm::Function *Fn);
 

[PATCH] D130131: [HLSL] CodeGen hlsl cbuffer/tbuffer.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:81-83
+auto *GV = Const.first;
+Const.second = EltTys.size();
+auto *Ty = GV->getValueType();

Please spell out these types.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:101-102
+  for (auto  : Buf.Constants) {
+auto *EltTy = Buf.LayoutStruct->getElementType(Const.second);
+auto *GV = Const.first;
+unsigned Offset = Const.second;

Please spell out these types.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:108
+
+auto *GVTy = GV->getValueType();
+assert(EltTy == GVTy && "constant type mismatch");

Same here



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:151
+  for (Decl *it : DC->decls()) {
+if (VarDecl *ConstDecl = dyn_cast(it)) {
+  addConstant(ConstDecl, CB);





Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:153-156
+} else if (isa(*it)) {
+  // Nothing to do for this declaration.
+} else if (isa(it)) {
+  // Nothing to do for this declaration.





Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:181
+
+  auto  = M.getDataLayout();
+

Please spell out the type.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.h:51
+  struct Buffer {
+Buffer(HLSLBufferDecl *D);
+llvm::StringRef Name;





Comment at: clang/lib/CodeGen/CGHLSLRuntime.h:76
 
+  void addBuffer(HLSLBufferDecl *D);
   void finishCodeGen();




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130131/new/

https://reviews.llvm.org/D130131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130131: [HLSL] CodeGen hlsl cbuffer/tbuffer.

[Xiang Li via Phabricator via cfe-commits]

python3kgae updated this revision to Diff 462085.
python3kgae added a comment.

Remove nested HLSLBuffer and namespace inside HLSLBuffer.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130131/new/

https://reviews.llvm.org/D130131

Files:
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGHLSLRuntime.cpp
  clang/lib/CodeGen/CGHLSLRuntime.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGenHLSL/cbuf.hlsl
  clang/test/CodeGenHLSL/cbuf_in_namespace.hlsl
  clang/test/CodeGenHLSL/static_global_and_function_in_cb.hlsl

Index: clang/test/CodeGenHLSL/static_global_and_function_in_cb.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/static_global_and_function_in_cb.hlsl
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -finclude-default-header -x hlsl -triple \
+// RUN:   dxil-pc-shadermodel6.3-library %s \
+// RUN:   -emit-llvm -disable-llvm-passes -o - | FileCheck %s
+
+// CHECK-DAG: @[[CB:.+]] = external constant { float }
+
+cbuffer A {
+float a;
+  // CHECK-DAG:@b = internal global float 3.00e+00, align 4
+  static float b = 3;
+  // CHECK:load float, ptr @[[CB]], align 4
+  // CHECK:load float, ptr @b, align 4
+  float foo() { return a + b; }
+}
+
+float bar() {
+  return foo();
+}
Index: clang/test/CodeGenHLSL/cbuf_in_namespace.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/cbuf_in_namespace.hlsl
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -finclude-default-header -x hlsl -triple \
+// RUN:   dxil-pc-shadermodel6.3-library %s \
+// RUN:   -emit-llvm -disable-llvm-passes -o - | FileCheck %s
+
+// Make sure namespace inside cbuffer  works.
+// CHECK: @[[CB:.+]] = external constant { float }
+// CHECK: @[[TB:.+]] = external constant { float }
+namespace n0 {
+namespace n1 {
+  cbuffer A {
+float a;
+  }
+}
+  tbuffer B {
+float b;
+  }
+}
+
+float foo() {
+// CHECK: load float, ptr @[[CB]], align 4
+// CHECK: load float, ptr @[[TB]], align 4
+  return n0::n1::a + n0::b;
+}
Index: clang/test/CodeGenHLSL/cbuf.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/cbuf.hlsl
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -finclude-default-header -x hlsl -triple \
+// RUN:   dxil-pc-shadermodel6.3-library %s \
+// RUN:   -emit-llvm -disable-llvm-passes -o - | FileCheck %s
+
+// CHECK: @[[CB:.+]] = external constant { float, double }
+cbuffer A : register(b0, space1) {
+  float a;
+  double b;
+}
+
+// CHECK: @[[TB:.+]] = external constant { float, double }
+tbuffer A : register(t2, space1) {
+  float c;
+  double d;
+}
+
+float foo() {
+// CHECK: load float, ptr @[[CB]], align 4
+// CHECK: load double, ptr getelementptr inbounds ({ float, double }, ptr @[[CB]], i32 0, i32 1), align 8
+// CHECK: load float, ptr @[[TB]], align 4
+// CHECK: load double, ptr getelementptr inbounds ({ float, double }, ptr @[[TB]], i32 0, i32 1), align 8
+  return a + b + c*d;
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -6448,6 +6448,10 @@
 DI->EmitAndRetainType(getContext().getEnumType(cast(D)));
 break;
 
+  case Decl::HLSLBuffer:
+getHLSLRuntime().addBuffer(cast(D));
+break;
+
   default:
 // Make sure we handled everything we should, every other kind is a
 // non-top-level decl.  FIXME: Would be nice to have an isTopLevelDeclKind
Index: clang/lib/CodeGen/CGHLSLRuntime.h
===
--- clang/lib/CodeGen/CGHLSLRuntime.h
+++ clang/lib/CodeGen/CGHLSLRuntime.h
@@ -19,13 +19,25 @@
 
 #include "clang/Basic/HLSLRuntime.h"
 
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+
+#include 
+
 namespace llvm {
 class GlobalVariable;
 class Function;
+class StructType;
 } // namespace llvm
+
 namespace clang {
 class VarDecl;
 class ParmVarDecl;
+class HLSLBufferDecl;
+class CallExpr;
+class Type;
+class DeclContext;
 
 class FunctionDecl;
 
@@ -34,6 +46,19 @@
 class CodeGenModule;
 
 class CGHLSLRuntime {
+public:
+  struct Buffer {
+Buffer(HLSLBufferDecl *D);
+llvm::StringRef Name;
+// IsCBuffer - Whether the buffer is a cbuffer (and not a tbuffer).
+bool IsCBuffer;
+llvm::Optional Reg;
+unsigned Space;
+// Global variable and offset for each constant.
+std::vector> Constants;
+llvm::StructType *LayoutStruct = nullptr;
+  };
+
 protected:
   CodeGenModule 
   uint32_t ResourceCounters[static_cast(
@@ -48,11 +73,18 @@
   void annotateHLSLResource(const VarDecl *D, llvm::GlobalVariable *GV);
   void generateGlobalCtorDtorCalls();
 
+  void addBuffer(HLSLBufferDecl *D);
   void finishCodeGen();
 
   void setHLSLEntryAttributes(const FunctionDecl *FD, llvm::Function *Fn);
 
   void 

[PATCH] D130131: [HLSL] CodeGen hlsl cbuffer/tbuffer.

[Xiang Li via Phabricator via cfe-commits]

python3kgae marked an inline comment as done.
python3kgae added a comment.

In D130131#3720552 , @beanz wrote:

> Now that I'm seeing the code in D131370 , I 
> don't know that this is the right way to do things.
>
> I think using address spaces like this is odd. Address spaces aren't really 
> intended for use differentiating high level access types, but rather memory 
> regions and properties of that memory. I feel like the use of address spaces 
> in this change and in D131370  just makes 
> it more complicated and gets in the way.
>
> In DXIL, since we don't really have raw memory load and stores, none of these 
> address spaces mean anything. It is probably just cleaner to not use address 
> spaces in the CodeGen here either.

Removed address space.
Also removed nested HLSLBuffer and namespace inside HLSLBuffer.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130131/new/

https://reviews.llvm.org/D130131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130131: [HLSL] CodeGen hlsl cbuffer/tbuffer.

[Xiang Li via Phabricator via cfe-commits]

python3kgae updated this revision to Diff 462084.
python3kgae added a comment.

Remove address space and manual align.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130131/new/

https://reviews.llvm.org/D130131

Files:
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGHLSLRuntime.cpp
  clang/lib/CodeGen/CGHLSLRuntime.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGenHLSL/cbuf.hlsl
  clang/test/CodeGenHLSL/cbuf_in_namespace.hlsl
  clang/test/CodeGenHLSL/static_global_and_function_in_cb.hlsl

Index: clang/test/CodeGenHLSL/static_global_and_function_in_cb.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/static_global_and_function_in_cb.hlsl
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -finclude-default-header -x hlsl -triple \
+// RUN:   dxil-pc-shadermodel6.3-library %s \
+// RUN:   -emit-llvm -disable-llvm-passes -o - | FileCheck %s
+
+// CHECK-DAG: @[[CB:.+]] = external constant { float }
+
+cbuffer A {
+float a;
+  // CHECK-DAG:@b = internal global float 3.00e+00, align 4
+  static float b = 3;
+  // CHECK:load float, ptr @[[CB]], align 4
+  // CHECK:load float, ptr @b, align 4
+  float foo() { return a + b; }
+}
+
+float bar() {
+  return foo();
+}
Index: clang/test/CodeGenHLSL/cbuf_in_namespace.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/cbuf_in_namespace.hlsl
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -finclude-default-header -x hlsl -triple \
+// RUN:   dxil-pc-shadermodel6.3-library %s \
+// RUN:   -emit-llvm -disable-llvm-passes -o - | FileCheck %s
+
+// Make sure namespace inside cbuffer  works.
+// CHECK: @[[CB:.+]] = external constant { float }
+// CHECK: @[[TB:.+]] = external constant { float }
+namespace n0 {
+namespace n1 {
+  cbuffer A {
+float a;
+  }
+}
+  tbuffer B {
+float b;
+  }
+}
+
+float foo() {
+// CHECK: load float, ptr @[[CB]], align 4
+// CHECK: load float, ptr @[[TB]], align 4
+  return n0::n1::a + n0::b;
+}
Index: clang/test/CodeGenHLSL/cbuf.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/cbuf.hlsl
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -finclude-default-header -x hlsl -triple \
+// RUN:   dxil-pc-shadermodel6.3-library %s \
+// RUN:   -emit-llvm -disable-llvm-passes -o - | FileCheck %s
+
+// CHECK: @[[CB:.+]] = external constant { float, double }
+cbuffer A : register(b0, space1) {
+  float a;
+  double b;
+}
+
+// CHECK: @[[TB:.+]] = external constant { float, double }
+tbuffer A : register(t2, space1) {
+  float c;
+  double d;
+}
+
+float foo() {
+// CHECK: load float, ptr @[[CB]], align 4
+// CHECK: load double, ptr getelementptr inbounds ({ float, double }, ptr @[[CB]], i32 0, i32 1), align 8
+// CHECK: load float, ptr @[[TB]], align 4
+// CHECK: load double, ptr getelementptr inbounds ({ float, double }, ptr @[[TB]], i32 0, i32 1), align 8
+  return a + b + c*d;
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -6448,6 +6448,10 @@
 DI->EmitAndRetainType(getContext().getEnumType(cast(D)));
 break;
 
+  case Decl::HLSLBuffer:
+getHLSLRuntime().addBuffer(cast(D));
+break;
+
   default:
 // Make sure we handled everything we should, every other kind is a
 // non-top-level decl.  FIXME: Would be nice to have an isTopLevelDeclKind
Index: clang/lib/CodeGen/CGHLSLRuntime.h
===
--- clang/lib/CodeGen/CGHLSLRuntime.h
+++ clang/lib/CodeGen/CGHLSLRuntime.h
@@ -19,13 +19,25 @@
 
 #include "clang/Basic/HLSLRuntime.h"
 
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+
+#include 
+
 namespace llvm {
 class GlobalVariable;
 class Function;
+class StructType;
 } // namespace llvm
+
 namespace clang {
 class VarDecl;
 class ParmVarDecl;
+class HLSLBufferDecl;
+class CallExpr;
+class Type;
+class DeclContext;
 
 class FunctionDecl;
 
@@ -34,6 +46,19 @@
 class CodeGenModule;
 
 class CGHLSLRuntime {
+public:
+  struct Buffer {
+Buffer(HLSLBufferDecl *D);
+llvm::StringRef Name;
+// IsCBuffer - Whether the buffer is a cbuffer (and not a tbuffer).
+bool IsCBuffer;
+llvm::Optional Reg;
+unsigned Space;
+// Global variable and offset for each constant.
+std::vector> Constants;
+llvm::StructType *LayoutStruct = nullptr;
+  };
+
 protected:
   CodeGenModule 
   uint32_t ResourceCounters[static_cast(
@@ -48,11 +73,18 @@
   void annotateHLSLResource(const VarDecl *D, llvm::GlobalVariable *GV);
   void generateGlobalCtorDtorCalls();
 
+  void addBuffer(HLSLBufferDecl *D);
   void finishCodeGen();
 
   void setHLSLEntryAttributes(const FunctionDecl *FD, llvm::Function *Fn);
 
   void emitEntryFunction(const FunctionDecl 

[PATCH] D130131: [HLSL] CodeGen hlsl cbuffer/tbuffer.

[Xiang Li via Phabricator via cfe-commits]

python3kgae added a comment.

In D130131#3720552 , @beanz wrote:

> Now that I'm seeing the code in D131370 , I 
> don't know that this is the right way to do things.
>
> I think using address spaces like this is odd. Address spaces aren't really 
> intended for use differentiating high level access types, but rather memory 
> regions and properties of that memory. I feel like the use of address spaces 
> in this change and in D131370  just makes 
> it more complicated and gets in the way.
>
> In DXIL, since we don't really have raw memory load and stores, none of these 
> address spaces mean anything. It is probably just cleaner to not use address 
> spaces in the CodeGen here either.

If not use address space, how do llvm passes know a cbuffer ptr is different 
from a tbuffer ptr or ptr to a static global variable? Or llvm passes don't 
need to know the difference?




Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:96
+  }
+  Buf.LayoutStruct = llvm::StructType::get(EltTys[0]->getContext(), EltTys);
+}

beanz wrote:
> Why are you manually inserting padding?
> 
> IR level accesses don't require explicit layout, and we don't do this in DXC 
> either.
Does DL take care of the alignment


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130131/new/

https://reviews.llvm.org/D130131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130131: [HLSL] CodeGen hlsl cbuffer/tbuffer.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

Now that I'm seeing the code in D131370 , I 
don't know that this is the right way to do things.

I think using address spaces like this is odd. Address spaces aren't really 
intended for use differentiating high level access types, but rather memory 
regions and properties of that memory. I feel like the use of address spaces in 
this change and in D131370  just makes it 
more complicated and gets in the way.

In DXIL, since we don't really have raw memory load and stores, none of these 
address spaces mean anything. It is probably just cleaner to not use address 
spaces in the CodeGen here either.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130131/new/

https://reviews.llvm.org/D130131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130131: [HLSL] CodeGen hlsl cbuffer/tbuffer.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:96
+  }
+  Buf.LayoutStruct = llvm::StructType::get(EltTys[0]->getContext(), EltTys);
+}

Why are you manually inserting padding?

IR level accesses don't require explicit layout, and we don't do this in DXC 
either.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130131/new/

https://reviews.llvm.org/D130131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130131: [HLSL] CodeGen hlsl cbuffer/tbuffer.

[Xiang Li via Phabricator via cfe-commits]

python3kgae marked 3 inline comments as done.
python3kgae added inline comments.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:61
+
+GlobalVariable *replaceCBuffer(CGHLSLRuntime::CBuffer ) {
+  const unsigned CBufferAddressSpace = 4;

Anastasia wrote:
> python3kgae wrote:
> > Anastasia wrote:
> > > I don't think I understand the intent of this function along with 
> > > `CGHLSLRuntime::addConstant` that populates this collection.
> > It is translating
> > 
> > ```
> >  cbuffer A {
> >float a;
> >float b;
> > }
> > float foo() {
> >   return a + b;
> > }
> > ```
> > into
> > 
> > ```
> > struct cb.A.Ty {
> >   float a;
> >   float b;
> > };
> > 
> > cbuffer A {
> >   cb.A.Ty CBA;
> > }
> > float foo() {
> >   return CBA.a + CBA.b;
> > }
> > ```
> > 
> > Both a and b are in the global scope and will get a GlobalVariable in clang 
> > codeGen.
> > By doing the translation, we can ensure each buffer map to one 
> > GlobalVariable and save cbuffer layout in the value type of that 
> > GlobalVariable.
> Ok, I see, so if we are to translate it to C it would be something similar to:
> 
> 
> ```
> struct A {
>float a;
>float b;
> } cbuffer_A __attribute__((address_space(256)));
> 
> float foo() {
>   return cbuffer_A.a + cbuffer_A.b;
> }
> ```
> Maybe you can add some comments to explain the intent of this code at a 
> higher level... not sure if the generation can reuse or be made a bit close 
> to the regular C structs + address spaces...
> 
Added comments.
Will check how union is supported in clang codeGen. The behavior feels similar, 
hope could share some code.



Comment at: clang/test/CodeGenHLSL/nest_cbuf.hlsl:8
+  // CHECK: @[[TB:.+]] = external addrspace(5) constant { float, i32, double }
+  tbuffer A : register(t2, space1) {
+float c;

Anastasia wrote:
> is this generated as nested structs?
No. This will generate as two separate structs.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130131/new/

https://reviews.llvm.org/D130131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130131: [HLSL] CodeGen hlsl cbuffer/tbuffer.

[Xiang Li via Phabricator via cfe-commits]

python3kgae updated this revision to Diff 450978.
python3kgae added a comment.

Add more test and comment.
Also use getTargetAddressSpace.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130131/new/

https://reviews.llvm.org/D130131

Files:
  clang/include/clang/Basic/AddressSpaces.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/Basic/Targets/DirectX.h
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/Basic/Targets/SPIR.h
  clang/lib/Basic/Targets/TCE.h
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGHLSLRuntime.cpp
  clang/lib/CodeGen/CGHLSLRuntime.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGenHLSL/cbuf.hlsl
  clang/test/CodeGenHLSL/cbuf_in_namespace.hlsl
  clang/test/CodeGenHLSL/nest_cbuf.hlsl
  clang/test/CodeGenHLSL/static_global_and_function_in_cb.hlsl

Index: clang/test/CodeGenHLSL/static_global_and_function_in_cb.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/static_global_and_function_in_cb.hlsl
@@ -0,0 +1,18 @@
+// RUN: %clang_dxc -Tlib_6_7 -fcgl  -Fo - %s | FileCheck %s
+
+// CHECK-DAG: @[[CB:.+]] = external addrspace(4) constant { float }
+
+cbuffer A {
+  namespace n0 {
+float a;
+  }
+  // CHECK-DAG:@b = internal global float 3.00e+00, align 4
+  static float b = 3;
+  // CHECK:load float, ptr addrspacecast (ptr addrspace(4) @[[CB]] to ptr), align 4
+  // CHECK:load float, ptr @b, align 4
+  float foo() { return n0::a + b; }
+}
+
+float bar() {
+  return foo();
+}
Index: clang/test/CodeGenHLSL/nest_cbuf.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/nest_cbuf.hlsl
@@ -0,0 +1,20 @@
+// RUN: %clang_dxc -Tlib_6_7 -fcgl  -Fo - %s | FileCheck %s
+
+// CHECK: @[[CB:.+]] = external addrspace(4) constant { float, i32, double }
+cbuffer A {
+  float a;
+  double b;
+  // CHECK: @[[TB:.+]] = external addrspace(5) constant { float, i32, double }
+  tbuffer A : register(t2, space1) {
+float c;
+double d;
+  }
+}
+
+float foo() {
+// CHECK: load float, ptr addrspacecast (ptr addrspace(4) @[[CB]] to ptr), align 4
+// CHECK: load double, ptr addrspacecast (ptr addrspace(4) getelementptr inbounds ({ float, i32, double }, ptr addrspace(4) @[[CB]], i32 0, i32 2) to ptr), align 8
+// CHECK: load float, ptr addrspacecast (ptr addrspace(5) @[[TB]] to ptr), align 4
+// CHECK: load double, ptr addrspacecast (ptr addrspace(5) getelementptr inbounds ({ float, i32, double }, ptr addrspace(5) @[[TB]], i32 0, i32 2) to ptr), align 8
+  return a + b + c*d;
+}
Index: clang/test/CodeGenHLSL/cbuf_in_namespace.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/cbuf_in_namespace.hlsl
@@ -0,0 +1,18 @@
+// RUN: %clang_dxc -Tlib_6_7 -fcgl  -Fo - %s | FileCheck %s
+
+// Make sure namespace inside cbuffer  works.
+// CHECK: @[[CB:.+]] = external addrspace(4) constant { float, float }
+cbuffer A {
+  namespace n0 {
+float a;
+  }
+  namespace n1 {
+float b;
+  }
+}
+
+float foo() {
+// CHECK: load float, ptr addrspacecast (ptr addrspace(4) @[[CB]] to ptr), align 4
+// CHECK: load float, ptr addrspacecast (ptr addrspace(4) getelementptr inbounds ({ float, float }, ptr addrspace(4) @[[CB]], i32 0, i32 1) to ptr), align 4
+  return n0::a + n1::b;
+}
Index: clang/test/CodeGenHLSL/cbuf.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/cbuf.hlsl
@@ -0,0 +1,21 @@
+// RUN: %clang_dxc -Tlib_6_7 -fcgl  -Fo - %s | FileCheck %s
+
+// CHECK: @[[CB:.+]] = external addrspace(4) constant { float, i32, double }
+cbuffer A : register(b0, space1) {
+  float a;
+  double b;
+}
+
+// CHECK: @[[TB:.+]] = external addrspace(5) constant { float, i32, double }
+tbuffer A : register(t2, space1) {
+  float c;
+  double d;
+}
+
+float foo() {
+// CHECK: load float, ptr addrspacecast (ptr addrspace(4) @[[CB]] to ptr), align 4
+// CHECK: load double, ptr addrspacecast (ptr addrspace(4) getelementptr inbounds ({ float, i32, double }, ptr addrspace(4) @[[CB]], i32 0, i32 2) to ptr), align 8
+// CHECK: load float, ptr addrspacecast (ptr addrspace(5) @[[TB]] to ptr), align 4
+// CHECK: load double, ptr addrspacecast (ptr addrspace(5) getelementptr inbounds ({ float, i32, double }, ptr addrspace(5) @[[TB]], i32 0, i32 2) to ptr), align 8
+  return a + b + c*d;
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -6370,6 +6370,10 @@
 DI->EmitAndRetainType(getContext().getEnumType(cast(D)));
 break;
 
+  case Decl::HLSLBuffer:
+getHLSLRuntime().addBuffer(cast(D));
+break;
+
   default:
 // Make sure we handled everything we should, every other kind is a
 // non-top-level decl.  FIXME: Would be 

[PATCH] D130131: [HLSL] CodeGen hlsl cbuffer/tbuffer.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:61
+
+GlobalVariable *replaceCBuffer(CGHLSLRuntime::CBuffer ) {
+  const unsigned CBufferAddressSpace = 4;

python3kgae wrote:
> Anastasia wrote:
> > I don't think I understand the intent of this function along with 
> > `CGHLSLRuntime::addConstant` that populates this collection.
> It is translating
> 
> ```
>  cbuffer A {
>float a;
>float b;
> }
> float foo() {
>   return a + b;
> }
> ```
> into
> 
> ```
> struct cb.A.Ty {
>   float a;
>   float b;
> };
> 
> cbuffer A {
>   cb.A.Ty CBA;
> }
> float foo() {
>   return CBA.a + CBA.b;
> }
> ```
> 
> Both a and b are in the global scope and will get a GlobalVariable in clang 
> codeGen.
> By doing the translation, we can ensure each buffer map to one GlobalVariable 
> and save cbuffer layout in the value type of that GlobalVariable.
Ok, I see, so if we are to translate it to C it would be something similar to:


```
struct A {
   float a;
   float b;
} cbuffer_A __attribute__((address_space(256)));

float foo() {
  return cbuffer_A.a + cbuffer_A.b;
}
```
Maybe you can add some comments to explain the intent of this code at a higher 
level... not sure if the generation can reuse or be made a bit close to the 
regular C structs + address spaces...




Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:119
+void CGHLSLRuntime::addConstant(VarDecl *D, Buffer ) {
+  if (D->getStorageClass() == SC_Static) {
+// For static inside cbuffer, take as global static.

btw is this covered in the test?



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:152
+  // as it only refers to globally scoped declarations.
+  CGM.EmitTopLevelDecl(it);
+} else if (NamespaceDecl *ND = dyn_cast(it)) {

Ok I think you don't have this in the tests too and the one below?




Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:183
+  const unsigned CBufferAddressSpace =
+  ASMap[(unsigned)clang::LangAS::hlsl_cbuffer];
+  const unsigned TBufferAddressSpace =

I think it's better to use `getTargetAddressSpace` from `ASTContext`.



Comment at: clang/test/CodeGenHLSL/nest_cbuf.hlsl:8
+  // CHECK: @[[TB:.+]] = external addrspace(5) constant { float, i32, double }
+  tbuffer A : register(t2, space1) {
+float c;

is this generated as nested structs?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130131/new/

https://reviews.llvm.org/D130131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130131: [HLSL] CodeGen hlsl cbuffer/tbuffer.

[Xiang Li via Phabricator via cfe-commits]

python3kgae updated this revision to Diff 448435.
python3kgae marked 2 inline comments as done.
python3kgae added a comment.

Remove useless comment.
Use llvm::Optional.
Use real type instead of auto.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130131/new/

https://reviews.llvm.org/D130131

Files:
  clang/include/clang/Basic/AddressSpaces.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/Basic/Targets/DirectX.h
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/Basic/Targets/SPIR.h
  clang/lib/Basic/Targets/TCE.h
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGHLSLRuntime.cpp
  clang/lib/CodeGen/CGHLSLRuntime.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGenHLSL/cbuf.hlsl
  clang/test/CodeGenHLSL/nest_cbuf.hlsl

Index: clang/test/CodeGenHLSL/nest_cbuf.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/nest_cbuf.hlsl
@@ -0,0 +1,20 @@
+// RUN: %clang_dxc -Tlib_6_7 -fcgl  -Fo - %s | FileCheck %s
+
+// CHECK: @[[CB:.+]] = external addrspace(4) constant { float, i32, double }
+cbuffer A {
+  float a;
+  double b;
+  // CHECK: @[[TB:.+]] = external addrspace(5) constant { float, i32, double }
+  tbuffer A : register(t2, space1) {
+float c;
+double d;
+  }
+}
+
+float foo() {
+// CHECK: load float, ptr addrspacecast (ptr addrspace(4) @[[CB]] to ptr), align 4
+// CHECK: load double, ptr addrspacecast (ptr addrspace(4) getelementptr inbounds ({ float, i32, double }, ptr addrspace(4) @[[CB]], i32 0, i32 2) to ptr), align 8
+// CHECK: load float, ptr addrspacecast (ptr addrspace(5) @[[TB]] to ptr), align 4
+// CHECK: load double, ptr addrspacecast (ptr addrspace(5) getelementptr inbounds ({ float, i32, double }, ptr addrspace(5) @[[TB]], i32 0, i32 2) to ptr), align 8
+  return a + b + c*d;
+}
Index: clang/test/CodeGenHLSL/cbuf.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/cbuf.hlsl
@@ -0,0 +1,21 @@
+// RUN: %clang_dxc -Tlib_6_7 -fcgl  -Fo - %s | FileCheck %s
+
+// CHECK: @[[CB:.+]] = external addrspace(4) constant { float, i32, double }
+cbuffer A : register(b0, space1) {
+  float a;
+  double b;
+}
+
+// CHECK: @[[TB:.+]] = external addrspace(5) constant { float, i32, double }
+tbuffer A : register(t2, space1) {
+  float c;
+  double d;
+}
+
+float foo() {
+// CHECK: load float, ptr addrspacecast (ptr addrspace(4) @[[CB]] to ptr), align 4
+// CHECK: load double, ptr addrspacecast (ptr addrspace(4) getelementptr inbounds ({ float, i32, double }, ptr addrspace(4) @[[CB]], i32 0, i32 2) to ptr), align 8
+// CHECK: load float, ptr addrspacecast (ptr addrspace(5) @[[TB]] to ptr), align 4
+// CHECK: load double, ptr addrspacecast (ptr addrspace(5) getelementptr inbounds ({ float, i32, double }, ptr addrspace(5) @[[TB]], i32 0, i32 2) to ptr), align 8
+  return a + b + c*d;
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -6281,6 +6281,10 @@
 DI->EmitAndRetainType(getContext().getEnumType(cast(D)));
 break;
 
+  case Decl::HLSLBuffer:
+getHLSLRuntime().addBuffer(cast(D));
+break;
+
   default:
 // Make sure we handled everything we should, every other kind is a
 // non-top-level decl.  FIXME: Would be nice to have an isTopLevelDeclKind
Index: clang/lib/CodeGen/CGHLSLRuntime.h
===
--- clang/lib/CodeGen/CGHLSLRuntime.h
+++ clang/lib/CodeGen/CGHLSLRuntime.h
@@ -15,21 +15,52 @@
 #ifndef LLVM_CLANG_LIB_CODEGEN_CGHLSLRUNTIME_H
 #define LLVM_CLANG_LIB_CODEGEN_CGHLSLRUNTIME_H
 
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+#include 
+
+namespace llvm {
+class GlobalVariable;
+class StructType;
+} // namespace llvm
+
 namespace clang {
+class HLSLBufferDecl;
+class VarDecl;
+class DeclContext;
 
 namespace CodeGen {
 
 class CodeGenModule;
 
 class CGHLSLRuntime {
+public:
+  struct Buffer {
+Buffer(HLSLBufferDecl *D);
+llvm::StringRef Name;
+// IsCBuffer - Whether the buffer is a cbuffer (and not a tbuffer).
+bool IsCBuffer;
+llvm::Optional Reg;
+unsigned Space;
+// Global variable and offset for each constant.
+std::vector> Constants;
+llvm::StructType *LayoutStruct = nullptr;
+  };
+
 protected:
   CodeGenModule 
 
 public:
   CGHLSLRuntime(CodeGenModule ) : CGM(CGM) {}
   virtual ~CGHLSLRuntime() {}
-
+  void addBuffer(HLSLBufferDecl *D);
   void finishCodeGen();
+
+private:
+  void addConstant(VarDecl *D, Buffer );
+  void addBufferDecls(DeclContext *DC, Buffer );
+  llvm::SmallVector Buffers;
 };
 
 } // namespace CodeGen
Index: clang/lib/CodeGen/CGHLSLRuntime.cpp
===
--- 

[PATCH] D130131: [HLSL] CodeGen hlsl cbuffer/tbuffer.

[Xiang Li via Phabricator via cfe-commits]

python3kgae updated this revision to Diff 448414.
python3kgae marked an inline comment as done.
python3kgae added a comment.

Add test for nested cbuffer.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130131/new/

https://reviews.llvm.org/D130131

Files:
  clang/include/clang/Basic/AddressSpaces.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/Basic/Targets/DirectX.h
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/Basic/Targets/SPIR.h
  clang/lib/Basic/Targets/TCE.h
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGHLSLRuntime.cpp
  clang/lib/CodeGen/CGHLSLRuntime.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGenHLSL/cbuf.hlsl
  clang/test/CodeGenHLSL/nest_cbuf.hlsl

Index: clang/test/CodeGenHLSL/nest_cbuf.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/nest_cbuf.hlsl
@@ -0,0 +1,20 @@
+// RUN: %clang_dxc -Tlib_6_7 -fcgl  -Fo - %s | FileCheck %s
+
+// CHECK: @[[CB:.+]] = external addrspace(4) constant { float, i32, double }
+cbuffer A : register(b0, space1) {
+  float a;
+  double b;
+  // CHECK: @[[TB:.+]] = external addrspace(5) constant { float, i32, double }
+  tbuffer A : register(b2, space1) {
+float c;
+double d;
+  }
+}
+
+float foo() {
+// CHECK: load float, ptr addrspacecast (ptr addrspace(4) @[[CB]] to ptr), align 4
+// CHECK: load double, ptr addrspacecast (ptr addrspace(4) getelementptr inbounds ({ float, i32, double }, ptr addrspace(4) @[[CB]], i32 0, i32 2) to ptr), align 8
+// CHECK: load float, ptr addrspacecast (ptr addrspace(5) @[[TB]] to ptr), align 4
+// CHECK: load double, ptr addrspacecast (ptr addrspace(5) getelementptr inbounds ({ float, i32, double }, ptr addrspace(5) @[[TB]], i32 0, i32 2) to ptr), align 8
+  return a + b + c*d;
+}
Index: clang/test/CodeGenHLSL/cbuf.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/cbuf.hlsl
@@ -0,0 +1,21 @@
+// RUN: %clang_dxc -Tlib_6_7 -fcgl  -Fo - %s | FileCheck %s
+
+// CHECK: @[[CB:.+]] = external addrspace(4) constant { float, i32, double }
+cbuffer A : register(b0, space1) {
+  float a;
+  double b;
+}
+
+// CHECK: @[[TB:.+]] = external addrspace(5) constant { float, i32, double }
+tbuffer A : register(b2, space1) {
+  float c;
+  double d;
+}
+
+float foo() {
+// CHECK: load float, ptr addrspacecast (ptr addrspace(4) @[[CB]] to ptr), align 4
+// CHECK: load double, ptr addrspacecast (ptr addrspace(4) getelementptr inbounds ({ float, i32, double }, ptr addrspace(4) @[[CB]], i32 0, i32 2) to ptr), align 8
+// CHECK: load float, ptr addrspacecast (ptr addrspace(5) @[[TB]] to ptr), align 4
+// CHECK: load double, ptr addrspacecast (ptr addrspace(5) getelementptr inbounds ({ float, i32, double }, ptr addrspace(5) @[[TB]], i32 0, i32 2) to ptr), align 8
+  return a + b + c*d;
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -6281,6 +6281,10 @@
 DI->EmitAndRetainType(getContext().getEnumType(cast(D)));
 break;
 
+  case Decl::HLSLBuffer:
+getHLSLRuntime().addBuffer(cast(D));
+break;
+
   default:
 // Make sure we handled everything we should, every other kind is a
 // non-top-level decl.  FIXME: Would be nice to have an isTopLevelDeclKind
Index: clang/lib/CodeGen/CGHLSLRuntime.h
===
--- clang/lib/CodeGen/CGHLSLRuntime.h
+++ clang/lib/CodeGen/CGHLSLRuntime.h
@@ -15,21 +15,51 @@
 #ifndef LLVM_CLANG_LIB_CODEGEN_CGHLSLRUNTIME_H
 #define LLVM_CLANG_LIB_CODEGEN_CGHLSLRUNTIME_H
 
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+#include 
+
+namespace llvm {
+class GlobalVariable;
+class StructType;
+} // namespace llvm
+
 namespace clang {
+class HLSLBufferDecl;
+class VarDecl;
+class DeclContext;
 
 namespace CodeGen {
 
 class CodeGenModule;
 
 class CGHLSLRuntime {
+public:
+  struct Buffer {
+Buffer(HLSLBufferDecl *D);
+llvm::StringRef Name;
+// IsCBuffer - Whether the buffer is a cbuffer (and not a tbuffer).
+bool IsCBuffer;
+unsigned Reg;
+unsigned Space;
+// Global variable and offset for each constant.
+std::vector> Constants;
+llvm::StructType *LayoutStruct = nullptr;
+  };
+
 protected:
   CodeGenModule 
 
 public:
   CGHLSLRuntime(CodeGenModule ) : CGM(CGM) {}
   virtual ~CGHLSLRuntime() {}
-
+  void addBuffer(HLSLBufferDecl *D);
   void finishCodeGen();
+
+private:
+  void addConstant(VarDecl *D, Buffer );
+  void addBufferDecls(DeclContext *DC, Buffer );
+  llvm::SmallVector Buffers;
 };
 
 } // namespace CodeGen
Index: clang/lib/CodeGen/CGHLSLRuntime.cpp
===
--- clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ 

[PATCH] D130131: [HLSL] CodeGen hlsl cbuffer/tbuffer.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/lib/Basic/Targets/SPIR.h:46
+0, // ptr64
+// HLSL address space values for this map are dummy and they can't be used
+0, // hlsl_cbuffer

I don’t think you need this comment. Most of those adds space values aren’t 
applicable to these targets, that’s why they are all 0.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:185
+  const unsigned TBufferAddressSpace =
+  ASMap[(unsigned)clang::LangAS::hlsl_tbuffer];
+

Why look both of these values up when you’re only going to use one?



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:189
+layoutBuffer(Buf, DL);
+auto AddressSapce =
+Buf.IsCBuffer ? CBufferAddressSpace : TBufferAddressSpace;

nit: Don’t use `auto` in places where the type isn’t obvious from the 
expression.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:204
+  } else {
+Reg = UINT_MAX;
+Space = 0;

nit: Rather than using int max as a sentinel here, why not use an 
llvm::Optional?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130131/new/

https://reviews.llvm.org/D130131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130131: [HLSL] CodeGen hlsl cbuffer/tbuffer.

[Xiang Li via Phabricator via cfe-commits]

python3kgae marked an inline comment as done.
python3kgae added inline comments.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:142
+  if (!Inner) {
+DiagnosticsEngine  = CGM.getDiags();
+unsigned DiagID = Diags.getCustomDiagID(DiagnosticsEngine::Error,

beanz wrote:
> python3kgae wrote:
> > python3kgae wrote:
> > > Anastasia wrote:
> > > > Is this case covered by the test?
> > > Nice catch.
> > > I'll add a test for this.
> > Cannot add a test before other HLSL resource type like Texture2D is 
> > supported :(
> `Texture2D` will be a `CXXRecordDecl`, not a `HLSLBufferDecl`, we only need 
> buffer decls for the legacy buffers. So this should be testable by nesting a 
> cbuffer inside a cbuffer right?
I see.
I was thinking about test the error path.
I'll add a test for nested cbuffer.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130131/new/

https://reviews.llvm.org/D130131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130131: [HLSL] CodeGen hlsl cbuffer/tbuffer.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:142
+  if (!Inner) {
+DiagnosticsEngine  = CGM.getDiags();
+unsigned DiagID = Diags.getCustomDiagID(DiagnosticsEngine::Error,

python3kgae wrote:
> python3kgae wrote:
> > Anastasia wrote:
> > > Is this case covered by the test?
> > Nice catch.
> > I'll add a test for this.
> Cannot add a test before other HLSL resource type like Texture2D is supported 
> :(
`Texture2D` will be a `CXXRecordDecl`, not a `HLSLBufferDecl`, we only need 
buffer decls for the legacy buffers. So this should be testable by nesting a 
cbuffer inside a cbuffer right?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130131/new/

https://reviews.llvm.org/D130131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130131: [HLSL] CodeGen hlsl cbuffer/tbuffer.

[Xiang Li via Phabricator via cfe-commits]

python3kgae marked an inline comment as done.
python3kgae added inline comments.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:142
+  if (!Inner) {
+DiagnosticsEngine  = CGM.getDiags();
+unsigned DiagID = Diags.getCustomDiagID(DiagnosticsEngine::Error,

python3kgae wrote:
> Anastasia wrote:
> > Is this case covered by the test?
> Nice catch.
> I'll add a test for this.
Cannot add a test before other HLSL resource type like Texture2D is supported :(



Comment at: clang/lib/CodeGen/CGHLSLRuntime.h:39
+llvm::StringRef Name;
+bool IsCBuffer;
+unsigned Reg;

python3kgae wrote:
> Anastasia wrote:
> > what does this field represent?
> The field is for marking the CBuffer as a cbuffer or tbuffer.
> The name is confusing.
> How about adding an enum BufferKind { CBuffer, TBuffer };?
Change struct CBuffer into struct Buffer to match HLSLBufferDecl.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130131/new/

https://reviews.llvm.org/D130131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130131: [HLSL] CodeGen hlsl cbuffer/tbuffer.

[Xiang Li via Phabricator via cfe-commits]

python3kgae updated this revision to Diff 448407.
python3kgae added a comment.

Add more comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130131/new/

https://reviews.llvm.org/D130131

Files:
  clang/include/clang/Basic/AddressSpaces.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/Basic/Targets/DirectX.h
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/Basic/Targets/SPIR.h
  clang/lib/Basic/Targets/TCE.h
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGHLSLRuntime.cpp
  clang/lib/CodeGen/CGHLSLRuntime.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGenHLSL/cbuf.hlsl

Index: clang/test/CodeGenHLSL/cbuf.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/cbuf.hlsl
@@ -0,0 +1,21 @@
+// RUN: %clang_dxc -Tlib_6_7 -fcgl  -Fo - %s | FileCheck %s
+
+// CHECK: @[[CB:.+]] = external addrspace(4) constant { float, i32, double }
+cbuffer A : register(b0, space1) {
+  float a;
+  double b;
+}
+
+// CHECK: @[[TB:.+]] = external addrspace(5) constant { float, i32, double }
+tbuffer A : register(b2, space1) {
+  float c;
+  double d;
+}
+
+float foo() {
+// CHECK: load float, ptr addrspacecast (ptr addrspace(4) @[[CB]] to ptr), align 4
+// CHECK: load double, ptr addrspacecast (ptr addrspace(4) getelementptr inbounds ({ float, i32, double }, ptr addrspace(4) @[[CB]], i32 0, i32 2) to ptr), align 8
+// CHECK: load float, ptr addrspacecast (ptr addrspace(5) @[[TB]] to ptr), align 4
+// CHECK: load double, ptr addrspacecast (ptr addrspace(5) getelementptr inbounds ({ float, i32, double }, ptr addrspace(5) @[[TB]], i32 0, i32 2) to ptr), align 8
+  return a + b + c*d;
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -6281,6 +6281,10 @@
 DI->EmitAndRetainType(getContext().getEnumType(cast(D)));
 break;
 
+  case Decl::HLSLBuffer:
+getHLSLRuntime().addBuffer(cast(D));
+break;
+
   default:
 // Make sure we handled everything we should, every other kind is a
 // non-top-level decl.  FIXME: Would be nice to have an isTopLevelDeclKind
Index: clang/lib/CodeGen/CGHLSLRuntime.h
===
--- clang/lib/CodeGen/CGHLSLRuntime.h
+++ clang/lib/CodeGen/CGHLSLRuntime.h
@@ -15,21 +15,51 @@
 #ifndef LLVM_CLANG_LIB_CODEGEN_CGHLSLRUNTIME_H
 #define LLVM_CLANG_LIB_CODEGEN_CGHLSLRUNTIME_H
 
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+#include 
+
+namespace llvm {
+class GlobalVariable;
+class StructType;
+} // namespace llvm
+
 namespace clang {
+class HLSLBufferDecl;
+class VarDecl;
+class DeclContext;
 
 namespace CodeGen {
 
 class CodeGenModule;
 
 class CGHLSLRuntime {
+public:
+  struct Buffer {
+Buffer(HLSLBufferDecl *D);
+llvm::StringRef Name;
+// IsCBuffer - Whether the buffer is a cbuffer (and not a tbuffer).
+bool IsCBuffer;
+unsigned Reg;
+unsigned Space;
+// Global variable and offset for each constant.
+std::vector> Constants;
+llvm::StructType *LayoutStruct = nullptr;
+  };
+
 protected:
   CodeGenModule 
 
 public:
   CGHLSLRuntime(CodeGenModule ) : CGM(CGM) {}
   virtual ~CGHLSLRuntime() {}
-
+  void addBuffer(HLSLBufferDecl *D);
   void finishCodeGen();
+
+private:
+  void addConstant(VarDecl *D, Buffer );
+  void addBufferDecls(DeclContext *DC, Buffer );
+  llvm::SmallVector Buffers;
 };
 
 } // namespace CodeGen
Index: clang/lib/CodeGen/CGHLSLRuntime.cpp
===
--- clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -13,7 +13,9 @@
 //===--===//
 
 #include "CGHLSLRuntime.h"
+#include "CGDebugInfo.h"
 #include "CodeGenModule.h"
+
 #include "clang/Basic/TargetOptions.h"
 #include "llvm/IR/Metadata.h"
 #include "llvm/IR/Module.h"
@@ -23,6 +25,7 @@
 using namespace llvm;
 
 namespace {
+
 void addDxilValVersion(StringRef ValVersionStr, llvm::Module ) {
   // The validation of ValVersionStr is done at HLSLToolChain::TranslateArgs.
   // Assume ValVersionStr is legal here.
@@ -42,11 +45,163 @@
   StringRef DxilValKey = "dx.valver";
   M.addModuleFlag(llvm::Module::ModFlagBehavior::AppendUnique, DxilValKey, Val);
 }
+
+void layoutBuffer(CGHLSLRuntime::Buffer , const DataLayout ) {
+  if (Buf.Constants.empty())
+return;
+
+  // FIXME: support legacy cbuffer layout.
+  auto  = Buf.Constants[0].first->getContext();
+  std::vector EltTys;
+  uint64_t Offset = 0;
+  for (auto  : Buf.Constants) {
+auto *GV = Const.first;
+Const.second = EltTys.size();
+auto *Ty = GV->getValueType();
+auto Align = DL.getPrefTypeAlign(Ty);
+if (uint64_t PaddingSize = (Offset % Align.value())) {
+  

[PATCH] D130131: [HLSL] CodeGen hlsl cbuffer/tbuffer.

[Xiang Li via Phabricator via cfe-commits]

python3kgae updated this revision to Diff 448403.
python3kgae added a comment.
Herald added subscribers: kosarev, mattd, gchakrabarti, asavonic, kerbowa, 
jvesely, jholewinski.

Take care alignment when layout CBuffer.
Add hlsl_cbuffer/tbuffer to clang::LangAS.
Change CGHLSLRuntime::CBuffer to CGHLSLRuntime::Buffer to match HLSLBufferDecl.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130131/new/

https://reviews.llvm.org/D130131

Files:
  clang/include/clang/Basic/AddressSpaces.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/Basic/Targets/DirectX.h
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/Basic/Targets/SPIR.h
  clang/lib/Basic/Targets/TCE.h
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGHLSLRuntime.cpp
  clang/lib/CodeGen/CGHLSLRuntime.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGenHLSL/cbuf.hlsl

Index: clang/test/CodeGenHLSL/cbuf.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/cbuf.hlsl
@@ -0,0 +1,21 @@
+// RUN: %clang_dxc -Tlib_6_7 -fcgl  -Fo - %s | FileCheck %s
+
+// CHECK: @[[CB:.+]] = external addrspace(4) constant { float, i32, double }
+cbuffer A : register(b0, space1) {
+  float a;
+  double b;
+}
+
+// CHECK: @[[TB:.+]] = external addrspace(5) constant { float, i32, double }
+tbuffer A : register(b2, space1) {
+  float c;
+  double d;
+}
+
+float foo() {
+// CHECK: load float, ptr addrspacecast (ptr addrspace(4) @[[CB]] to ptr), align 4
+// CHECK: load double, ptr addrspacecast (ptr addrspace(4) getelementptr inbounds ({ float, i32, double }, ptr addrspace(4) @[[CB]], i32 0, i32 2) to ptr), align 8
+// CHECK: load float, ptr addrspacecast (ptr addrspace(5) @[[TB]] to ptr), align 4
+// CHECK: load double, ptr addrspacecast (ptr addrspace(5) getelementptr inbounds ({ float, i32, double }, ptr addrspace(5) @[[TB]], i32 0, i32 2) to ptr), align 8
+  return a + b + c*d;
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -6281,6 +6281,10 @@
 DI->EmitAndRetainType(getContext().getEnumType(cast(D)));
 break;
 
+  case Decl::HLSLBuffer:
+getHLSLRuntime().addBuffer(cast(D));
+break;
+
   default:
 // Make sure we handled everything we should, every other kind is a
 // non-top-level decl.  FIXME: Would be nice to have an isTopLevelDeclKind
Index: clang/lib/CodeGen/CGHLSLRuntime.h
===
--- clang/lib/CodeGen/CGHLSLRuntime.h
+++ clang/lib/CodeGen/CGHLSLRuntime.h
@@ -15,21 +15,50 @@
 #ifndef LLVM_CLANG_LIB_CODEGEN_CGHLSLRUNTIME_H
 #define LLVM_CLANG_LIB_CODEGEN_CGHLSLRUNTIME_H
 
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+#include 
+
+namespace llvm {
+class GlobalVariable;
+class StructType;
+} // namespace llvm
+
 namespace clang {
+class HLSLBufferDecl;
+class VarDecl;
+class DeclContext;
 
 namespace CodeGen {
 
 class CodeGenModule;
 
 class CGHLSLRuntime {
+public:
+  struct Buffer {
+Buffer(HLSLBufferDecl *D);
+llvm::StringRef Name;
+bool IsCBuffer;
+unsigned Reg;
+unsigned Space;
+// Global variable and offset for each constant.
+std::vector> Constants;
+llvm::StructType *LayoutStruct = nullptr;
+  };
+
 protected:
   CodeGenModule 
 
 public:
   CGHLSLRuntime(CodeGenModule ) : CGM(CGM) {}
   virtual ~CGHLSLRuntime() {}
-
+  void addBuffer(HLSLBufferDecl *D);
   void finishCodeGen();
+
+private:
+  void addConstant(VarDecl *D, Buffer );
+  void addBufferDecls(DeclContext *DC, Buffer );
+  llvm::SmallVector Buffers;
 };
 
 } // namespace CodeGen
Index: clang/lib/CodeGen/CGHLSLRuntime.cpp
===
--- clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -13,7 +13,9 @@
 //===--===//
 
 #include "CGHLSLRuntime.h"
+#include "CGDebugInfo.h"
 #include "CodeGenModule.h"
+
 #include "clang/Basic/TargetOptions.h"
 #include "llvm/IR/Metadata.h"
 #include "llvm/IR/Module.h"
@@ -23,6 +25,7 @@
 using namespace llvm;
 
 namespace {
+
 void addDxilValVersion(StringRef ValVersionStr, llvm::Module ) {
   // The validation of ValVersionStr is done at HLSLToolChain::TranslateArgs.
   // Assume ValVersionStr is legal here.
@@ -42,11 +45,162 @@
   StringRef DxilValKey = "dx.valver";
   M.addModuleFlag(llvm::Module::ModFlagBehavior::AppendUnique, DxilValKey, Val);
 }
+
+void layoutBuffer(CGHLSLRuntime::Buffer , const DataLayout ) {
+  if (Buf.Constants.empty())
+return;
+
+  // FIXME: support legacy cbuffer layout.
+  auto  = Buf.Constants[0].first->getContext();
+  std::vector EltTys;
+  uint64_t Offset = 0;
+  for (auto  : Buf.Constants) {
+auto *GV = Const.first;
+

[PATCH] D130131: [HLSL] CodeGen hlsl cbuffer/tbuffer.

[Xiang Li via Phabricator via cfe-commits]

python3kgae added inline comments.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:61
+
+GlobalVariable *replaceCBuffer(CGHLSLRuntime::CBuffer ) {
+  const unsigned CBufferAddressSpace = 4;

Anastasia wrote:
> I don't think I understand the intent of this function along with 
> `CGHLSLRuntime::addConstant` that populates this collection.
It is translating

```
 cbuffer A {
   float a;
   float b;
}
float foo() {
  return a + b;
}
```
into

```
struct cb.A.Ty {
  float a;
  float b;
};

cbuffer A {
  cb.A.Ty CBA;
}
float foo() {
  return CBA.a + CBA.b;
}
```

Both a and b are in the global scope and will get a GlobalVariable in clang 
codeGen.
By doing the translation, we can ensure each buffer map to one GlobalVariable 
and save cbuffer layout in the value type of that GlobalVariable.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:62
+GlobalVariable *replaceCBuffer(CGHLSLRuntime::CBuffer ) {
+  const unsigned CBufferAddressSpace = 4;
+  const unsigned TBufferAddressSpace = 5;

Anastasia wrote:
> It might be better to avoid using hard-coded constants. Are you adding new 
> entires in clang's `AddressSpace` enum to represent different logical memory 
> segment of the language?
Thanks for pointing it out. I'll add the new address space to LangAS.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:142
+  if (!Inner) {
+DiagnosticsEngine  = CGM.getDiags();
+unsigned DiagID = Diags.getCustomDiagID(DiagnosticsEngine::Error,

Anastasia wrote:
> Is this case covered by the test?
Nice catch.
I'll add a test for this.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.h:39
+llvm::StringRef Name;
+bool IsCBuffer;
+unsigned Reg;

Anastasia wrote:
> what does this field represent?
The field is for marking the CBuffer as a cbuffer or tbuffer.
The name is confusing.
How about adding an enum BufferKind { CBuffer, TBuffer };?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130131/new/

https://reviews.llvm.org/D130131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130131: [HLSL] CodeGen hlsl cbuffer/tbuffer.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:61
+
+GlobalVariable *replaceCBuffer(CGHLSLRuntime::CBuffer ) {
+  const unsigned CBufferAddressSpace = 4;

I don't think I understand the intent of this function along with 
`CGHLSLRuntime::addConstant` that populates this collection.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:62
+GlobalVariable *replaceCBuffer(CGHLSLRuntime::CBuffer ) {
+  const unsigned CBufferAddressSpace = 4;
+  const unsigned TBufferAddressSpace = 5;

It might be better to avoid using hard-coded constants. Are you adding new 
entires in clang's `AddressSpace` enum to represent different logical memory 
segment of the language?



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:142
+  if (!Inner) {
+DiagnosticsEngine  = CGM.getDiags();
+unsigned DiagID = Diags.getCustomDiagID(DiagnosticsEngine::Error,

Is this case covered by the test?



Comment at: clang/lib/CodeGen/CGHLSLRuntime.h:39
+llvm::StringRef Name;
+bool IsCBuffer;
+unsigned Reg;

what does this field represent?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130131/new/

https://reviews.llvm.org/D130131

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130131: [HLSL] CodeGen hlsl cbuffer/tbuffer.

[Xiang Li via Phabricator via cfe-commits]

python3kgae created this revision.
python3kgae added reviewers: aaron.ballman, Anastasia, kuhar, bogner, beanz, 
pow2clk.
Herald added a project: All.
python3kgae requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

cbuffer A {

  float a;
  float b;

}

will be translated to a global variable with special address space.

Something like

struct CB_Ty {

  float a;
  float b;

};

__attribute__((address_space(3)))
CB_Ty A;

And all use of a and b will be replaced with A.a and A.b.

Only support none-legacy cbuffer layout now.
CodeGen for Resource binding will be in separate patch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130131

Files:
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGHLSLRuntime.cpp
  clang/lib/CodeGen/CGHLSLRuntime.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGenHLSL/cbuf.hlsl

Index: clang/test/CodeGenHLSL/cbuf.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/cbuf.hlsl
@@ -0,0 +1,21 @@
+// RUN: %clang_dxc -Tlib_6_7 -fcgl  -Fo - %s | FileCheck %s
+
+// CHECK: @[[CB:.+]] = external addrspace(4) constant { float, float }
+cbuffer A : register(b0, space1) {
+  float a;
+  float b;
+}
+
+// CHECK: @[[TB:.+]] = external addrspace(5) constant { float, float }
+tbuffer A : register(b2, space1) {
+  float c;
+  float d;
+}
+
+float foo() {
+// CHECK: load float, ptr addrspacecast (ptr addrspace(4) @[[CB]] to ptr), align 4
+// CHECK: load float, ptr addrspacecast (ptr addrspace(4) getelementptr inbounds ({ float, float }, ptr addrspace(4) @[[CB]], i32 0, i32 1) to ptr), align 4
+// CHECK: load float, ptr addrspacecast (ptr addrspace(5) @[[TB]] to ptr), align 4
+// CHECK: load float, ptr addrspacecast (ptr addrspace(5) getelementptr inbounds ({ float, float }, ptr addrspace(5) @[[TB]], i32 0, i32 1) to ptr), align 4
+  return a + b + c*d;
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -6281,6 +6281,10 @@
 DI->EmitAndRetainType(getContext().getEnumType(cast(D)));
 break;
 
+  case Decl::HLSLBuffer:
+getHLSLRuntime().addCBuffer(cast(D));
+break;
+
   default:
 // Make sure we handled everything we should, every other kind is a
 // non-top-level decl.  FIXME: Would be nice to have an isTopLevelDeclKind
Index: clang/lib/CodeGen/CGHLSLRuntime.h
===
--- clang/lib/CodeGen/CGHLSLRuntime.h
+++ clang/lib/CodeGen/CGHLSLRuntime.h
@@ -15,21 +15,48 @@
 #ifndef LLVM_CLANG_LIB_CODEGEN_CGHLSLRUNTIME_H
 #define LLVM_CLANG_LIB_CODEGEN_CGHLSLRUNTIME_H
 
+#include "llvm/ADT/StringRef.h"
+
+namespace llvm {
+class GlobalVariable;
+class StructType;
+} // namespace llvm
+
 namespace clang {
+class HLSLBufferDecl;
+class VarDecl;
+class DeclContext;
 
 namespace CodeGen {
 
 class CodeGenModule;
 
 class CGHLSLRuntime {
+public:
+  struct CBuffer {
+CBuffer(HLSLBufferDecl *D);
+llvm::StringRef Name;
+bool IsCBuffer;
+unsigned Reg;
+unsigned Space;
+// Global variable and offset for each constant.
+std::vector> Constants;
+llvm::StructType *LayoutStruct = nullptr;
+  };
+
 protected:
   CodeGenModule 
 
 public:
   CGHLSLRuntime(CodeGenModule ) : CGM(CGM) {}
   virtual ~CGHLSLRuntime() {}
-
+  void addCBuffer(HLSLBufferDecl *D);
   void finishCodeGen();
+
+private:
+  void addConstant(VarDecl *D, CBuffer );
+  void addCBufferDecls(DeclContext *DC, CBuffer );
+  SmallVector CBuffers;
 };
 
 } // namespace CodeGen
Index: clang/lib/CodeGen/CGHLSLRuntime.cpp
===
--- clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -13,7 +13,9 @@
 //===--===//
 
 #include "CGHLSLRuntime.h"
+#include "CGDebugInfo.h"
 #include "CodeGenModule.h"
+
 #include "clang/Basic/TargetOptions.h"
 #include "llvm/IR/Metadata.h"
 #include "llvm/IR/Module.h"
@@ -23,6 +25,7 @@
 using namespace llvm;
 
 namespace {
+
 void addDxilValVersion(StringRef ValVersionStr, llvm::Module ) {
   // The validation of ValVersionStr is done at HLSLToolChain::TranslateArgs.
   // Assume ValVersionStr is legal here.
@@ -42,11 +45,138 @@
   StringRef DxilValKey = "dx.valver";
   M.addModuleFlag(llvm::Module::ModFlagBehavior::AppendUnique, DxilValKey, Val);
 }
+
+void layoutCBuffer(CGHLSLRuntime::CBuffer , const DataLayout ) {
+  // FIXME: support legacy cbuffer layout.
+  std::vector EltTys;
+  for (auto  : CB.Constants) {
+auto *GV = Const.first;
+Const.second = EltTys.size();
+auto *Ty = GV->getValueType();
+EltTys.emplace_back(Ty);
+  }
+  CB.LayoutStruct = llvm::StructType::get(EltTys[0]->getContext(), EltTys);
+}
+
+GlobalVariable *replaceCBuffer(CGHLSLRuntime::CBuffer ) {