Skip to content

Commit

Permalink
Avoid duplicate Alignment decorations (#2537)
Browse files Browse the repository at this point in the history
The SPIR-V Validator has recently started checking for duplicate
decorations.  This commit fixes duplicate Alignment decorations that
affected the `test/read_image.cl` test.

Alignment decorations have two potential sources during LLVM to SPIR-V
translation: the instruction's alignment property and
`spirv.Decorations` metadata.  Handle both of these through the
`setAlignment` method, so that duplicates can be avoided.

Calling `setAlignment` with different alignments for the same entity
is probably an error, so add an assert.

Contributes to #2509
  • Loading branch information
svenvh authored Apr 29, 2024
1 parent 0d23d03 commit 926ca2a
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 2 deletions.
15 changes: 13 additions & 2 deletions lib/SPIRV/SPIRVWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1060,7 +1060,7 @@ void LLVMToSPIRVBase::transVectorComputeMetadata(Function *F) {
}
}

static void transMetadataDecorations(Metadata *MD, SPIRVEntry *Target);
static void transMetadataDecorations(Metadata *MD, SPIRVValue *Target);

void LLVMToSPIRVBase::transFPGAFunctionMetadata(SPIRVFunction *BF,
Function *F) {
Expand Down Expand Up @@ -2703,7 +2703,7 @@ void checkIsGlobalVar(SPIRVEntry *E, Decoration Dec) {
ErrStr);
}

static void transMetadataDecorations(Metadata *MD, SPIRVEntry *Target) {
static void transMetadataDecorations(Metadata *MD, SPIRVValue *Target) {
SPIRVErrorLog &ErrLog = Target->getErrorLog();

auto *ArgDecoMD = dyn_cast<MDNode>(MD);
Expand All @@ -2722,6 +2722,17 @@ static void transMetadataDecorations(Metadata *MD, SPIRVEntry *Target) {

const size_t NumOperands = DecoMD->getNumOperands();
switch (static_cast<size_t>(DecoKind)) {
case DecorationAlignment: {
// Handle Alignment via SPIRVValue::setAlignment() to avoid duplicate
// Alignment decorations.
auto *Alignment =
mdconst::dyn_extract<ConstantInt>(DecoMD->getOperand(1));
ErrLog.checkError(Alignment, SPIRVEC_InvalidLlvmModule,
"Alignment operand must be an integer.");
Target->setAlignment(Alignment->getZExtValue());
break;
}

ONE_STRING_DECORATION_CASE(MemoryINTEL, spv)
ONE_STRING_DECORATION_CASE(UserSemantic, spv)
ONE_INT_DECORATION_CASE(AliasScopeINTEL, spv, SPIRVId)
Expand Down
8 changes: 8 additions & 0 deletions lib/SPIRV/libSPIRV/SPIRVValue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,14 @@ void SPIRVValue::setAlignment(SPIRVWord A) {
eraseDecorate(DecorationAlignment);
return;
}
SPIRVWord PrevAlignment;
if (hasAlignment(&PrevAlignment)) {
// Do nothing if the Id already has an Alignment decoration, provided
// it matches the new alignment.
assert(A == PrevAlignment &&
"New alignment does not match existing alignment");
return;
}
addDecorate(new SPIRVDecorate(DecorationAlignment, this, A));
SPIRVDBG(spvdbgs() << "Set alignment " << A << " for obj " << Id << "\n")
}
Expand Down
17 changes: 17 additions & 0 deletions test/align-duplicate.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
; RUN: llvm-as %s -o %t.bc
; RUN: llvm-spirv %t.bc -o %t.spv
; RUN: spirv-val %t.spv

; Test that duplicate align information does not result in SPIR-V validation
; errors due to duplicate Alignment Decorations.

target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64"
target triple = "spir64-unknown-unknown"

define spir_func void @f() {
%res = alloca i16, align 2, !spirv.Decorations !1
ret void
}

!1 = !{!2}
!2 = !{i32 44, i32 2}

0 comments on commit 926ca2a

Please sign in to comment.