Skip to content

Commit cafbbed

Browse files
Temporarily revert for release (#4848)
Revert "spirv: get field index from SPIR-V type, not AST (#4806)" This reverts commit 8f279ba. This commit isn't passing some of our internal tests and needs to be reverted in preparation for the next Vulkan SDK release.
1 parent 6eedcd2 commit cafbbed

1 file changed

Lines changed: 17 additions & 77 deletions

File tree

tools/clang/lib/SPIRV/SpirvEmitter.cpp

Lines changed: 17 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515

1616
#include "AlignmentSizeCalculator.h"
1717
#include "InitListHandler.h"
18-
#include "LowerTypeVisitor.h"
1918
#include "RawBufferMethods.h"
2019
#include "dxc/DXIL/DxilConstants.h"
2120
#include "dxc/HlslIntrinsicOp.h"
@@ -543,71 +542,6 @@ bool isVkRawBufferLoadIntrinsic(const clang::FunctionDecl *FD) {
543542
return true;
544543
}
545544

546-
// Takes an AST member type, and determines its index in the equivalent SPIR-V
547-
// struct type. This is required as the struct layout might change between the
548-
// AST representation and SPIR-V representation.
549-
uint32_t getFieldIndexInStruct(const SpirvCodeGenOptions &spirvOptions,
550-
LowerTypeVisitor &lowerTypeVisitor,
551-
const MemberExpr *expr) {
552-
// If we are accessing a derived struct, we need to account for the number
553-
// of base structs, since they are placed as fields at the beginning of the
554-
// derived struct.
555-
auto baseType = expr->getBase()->getType();
556-
if (baseType->isPointerType()) {
557-
baseType = baseType->getPointeeType();
558-
}
559-
560-
const auto *fieldDecl =
561-
dynamic_cast<const FieldDecl *>(expr->getMemberDecl());
562-
assert(fieldDecl);
563-
const uint32_t indexAST =
564-
getNumBaseClasses(baseType) + fieldDecl->getFieldIndex();
565-
566-
// The AST type index is not representative of the SPIR-V type index
567-
// because we might squash some fields (bitfields by ex.).
568-
// What we need is to match each AST node with the squashed field and then,
569-
// determine the real index.
570-
const SpirvType *spvType = lowerTypeVisitor.lowerType(
571-
baseType, spirvOptions.sBufferLayoutRule, llvm::None, SourceLocation());
572-
assert(spvType);
573-
574-
const auto st = dynamic_cast<const StructType *>(spvType);
575-
assert(st != nullptr);
576-
const auto &fields = st->getFields();
577-
assert(indexAST <= fields.size());
578-
579-
// Some fields in SPIR-V share the same index (bitfields). Computing the final
580-
// index of the requested field.
581-
uint32_t indexSPV = 0;
582-
for (size_t i = 1; i <= indexAST; i++) {
583-
// Do not remove this condition. This is required to support inheritance:
584-
// 1. SPIR-V composite first element is the parent type:
585-
// by ex "OpTypeStruct %base_struct %float".
586-
// 2. if the parent type is an empty class, it's size it zero, hence
587-
// "%float" offset is also 0.
588-
//
589-
// A way to detect such cases is to check for type difference: fields cannot
590-
// be merged if the type is different.
591-
if (fields[i - 1].type != fields[i].type) {
592-
indexSPV++;
593-
continue;
594-
}
595-
596-
if (fields[i - 1].offset.getValueOr(0) != fields[i].offset.getValueOr(0)) {
597-
indexSPV++;
598-
continue;
599-
}
600-
}
601-
602-
// TODO(issue #4140): remove once bitfields are implemented.
603-
// This is just a safeguard until bitfield support is in. Before bitfields,
604-
// AST indices were always correct, so this function should not change that
605-
// behavior. Once the bitfield support is in, indices will start to diverge,
606-
// and this assert should be removed.
607-
assert(indexSPV == indexAST);
608-
return indexSPV;
609-
}
610-
611545
} // namespace
612546

613547
SpirvEmitter::SpirvEmitter(CompilerInstance &ci)
@@ -7680,17 +7614,23 @@ const Expr *SpirvEmitter::collectArrayStructIndices(
76807614
}
76817615
}
76827616

7683-
{
7684-
LowerTypeVisitor lowerTypeVisitor(astContext, spvContext, spirvOptions);
7685-
const uint32_t fieldIndex =
7686-
getFieldIndexInStruct(spirvOptions, lowerTypeVisitor, indexing);
7687-
7688-
if (rawIndex) {
7689-
rawIndices->push_back(fieldIndex);
7690-
} else {
7691-
indices->push_back(spvBuilder.getConstantInt(
7692-
astContext.IntTy, llvm::APInt(32, fieldIndex, true)));
7693-
}
7617+
// Append the index of the current level
7618+
const auto *fieldDecl = cast<FieldDecl>(indexing->getMemberDecl());
7619+
assert(fieldDecl);
7620+
// If we are accessing a derived struct, we need to account for the number
7621+
// of base structs, since they are placed as fields at the beginning of the
7622+
// derived struct.
7623+
auto baseType = indexing->getBase()->getType();
7624+
if (baseType->isPointerType()) {
7625+
baseType = baseType->getPointeeType();
7626+
}
7627+
const uint32_t index =
7628+
getNumBaseClasses(baseType) + fieldDecl->getFieldIndex();
7629+
if (rawIndex) {
7630+
rawIndices->push_back(index);
7631+
} else {
7632+
indices->push_back(spvBuilder.getConstantInt(
7633+
astContext.IntTy, llvm::APInt(32, index, true)));
76947634
}
76957635

76967636
return base;

0 commit comments

Comments
 (0)