Anastasia added inline comments.
================ Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:61 + +GlobalVariable *replaceCBuffer(CGHLSLRuntime::CBuffer &CB) { + 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 &CB) { + 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<NamespaceDecl>(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