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