llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-hlsl Author: Justin Bogner (bogner) <details> <summary>Changes</summary> We weren't accounting for skipped fields correctly when emitting struct member exprs, which could lead to us reading padding instead of the member itself when a struct followed an array. Fixes #<!-- -->179716 --- Full diff: https://github.com/llvm/llvm-project/pull/179768.diff 2 Files Affected: - (modified) clang/lib/CodeGen/CGHLSLRuntime.cpp (+16-3) - (modified) clang/test/CodeGenHLSL/resources/cbuffer_geps.hlsl (+3-3) ``````````diff diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp index cff2824d29dc5..985bc8640118e 100644 --- a/clang/lib/CodeGen/CGHLSLRuntime.cpp +++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp @@ -1556,10 +1556,7 @@ LValue CGHLSLRuntime::emitBufferMemberExpr(CodeGenFunction &CGF, auto *Field = dyn_cast<FieldDecl>(E->getMemberDecl()); assert(Field && "Unexpected access into HLSL buffer"); - // Get the field index for the struct. const RecordDecl *Rec = Field->getParent(); - unsigned FieldIdx = - CGM.getTypes().getCGRecordLayout(Rec).getLLVMFieldNo(Field); // Work out the buffer layout type to index into. QualType RecType = CGM.getContext().getCanonicalTagType(Rec); @@ -1571,6 +1568,22 @@ LValue CGHLSLRuntime::emitBufferMemberExpr(CodeGenFunction &CGF, llvm::StructType *LayoutTy = HLSLBufferLayoutBuilder(CGM).layOutStruct( RecType->getAsCanonical<RecordType>(), EmptyOffsets); + // Get the field index for the layout struct, accounting for padding. + unsigned FieldIdx = + CGM.getTypes().getCGRecordLayout(Rec).getLLVMFieldNo(Field); + assert(FieldIdx < LayoutTy->getNumElements() && + "Layout struct is smaller than member struct"); + unsigned Skipped = 0; + for (unsigned I = 0; I <= FieldIdx;) { + llvm::Type *ElementTy = LayoutTy->getElementType(I + Skipped); + if (CGF.CGM.getTargetCodeGenInfo().isHLSLPadding(ElementTy)) + ++Skipped; + else + ++I; + } + FieldIdx += Skipped; + assert(FieldIdx < LayoutTy->getNumElements() && "Access out of bounds"); + // Now index into the struct, making sure that the type we return is the // buffer layout type rather than the original type in the AST. QualType FieldType = Field->getType(); diff --git a/clang/test/CodeGenHLSL/resources/cbuffer_geps.hlsl b/clang/test/CodeGenHLSL/resources/cbuffer_geps.hlsl index 7a0b45875faf9..657694aa90fd7 100644 --- a/clang/test/CodeGenHLSL/resources/cbuffer_geps.hlsl +++ b/clang/test/CodeGenHLSL/resources/cbuffer_geps.hlsl @@ -73,7 +73,7 @@ void cbstructs() { use(s1.a1); // CHECK: load <3 x i16>, ptr addrspace(2) getelementptr inbounds nuw (%B, ptr addrspace(2) @s2, i32 0, i32 1), align 2 use(s2.b1); - // CHECK: load <2 x float>, ptr addrspace(2) getelementptr inbounds nuw (%C, ptr addrspace(2) @s3, i32 0, i32 1), align 8 + // CHECK: load <2 x float>, ptr addrspace(2) getelementptr inbounds nuw (%C, ptr addrspace(2) @s3, i32 0, i32 2), align 8 use(s3.c2.a1); // CHECK: load <2 x float>, ptr addrspace(2) getelementptr (<{ %A, target("dx.Padding", 8) }>, ptr addrspace(2) @s4, i32 2, i32 0), align 8 use(s4[2].a1); @@ -108,9 +108,9 @@ void cbmix() { use(m3.y); // CHECK: load <2 x float>, ptr addrspace(2) getelementptr (<{ <2 x float>, target("dx.Padding", 8) }>, ptr addrspace(2) getelementptr (<{ <{ [3 x <{ <2 x float>, target("dx.Padding", 8) }>], <2 x float> }>, target("dx.Padding", 8) }>, ptr addrspace(2) @m4, i32 2, i32 0), i32 3, i32 0), align 16 use(m4[2][3]); - // CHECK: load <4 x i32>, ptr addrspace(2) getelementptr inbounds nuw ([[ANON_1]], ptr addrspace(2) @m5, i32 0, i32 1), align 16 + // CHECK: load <4 x i32>, ptr addrspace(2) getelementptr inbounds nuw ([[ANON_1]], ptr addrspace(2) @m5, i32 0, i32 2), align 16 use(m5.d); - // CHECK: load <4 x i32>, ptr addrspace(2) getelementptr inbounds ([5 x <4 x i32>], ptr addrspace(2) getelementptr (<{ %ArrayAndScalar, target("dx.Padding", 12) }>, ptr addrspace(2) getelementptr inbounds nuw ([[ANON_2]], ptr addrspace(2) @m6, i32 0, i32 1), i32 2, i32 0), i32 0, i32 2), align 16 + // CHECK: load <4 x i32>, ptr addrspace(2) getelementptr inbounds ([5 x <4 x i32>], ptr addrspace(2) getelementptr (<{ %ArrayAndScalar, target("dx.Padding", 12) }>, ptr addrspace(2) getelementptr inbounds nuw ([[ANON_2]], ptr addrspace(2) @m6, i32 0, i32 2), i32 2, i32 0), i32 0, i32 2), align 16 use(m6.j[2].x[2]); // CHECK: load <1 x double>, ptr addrspace(2) @m7, align 8 use(m7); `````````` </details> https://github.com/llvm/llvm-project/pull/179768 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
