Skip to content

Commit cc0a77e

Browse files
svenvhsys-ce-bb
authored andcommitted
Avoid duplicate Alignment decorations (#2537)
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 KhronosGroup/SPIRV-LLVM-Translator#2509 Original commit: KhronosGroup/SPIRV-LLVM-Translator@926ca2ae8497166
1 parent f69ec32 commit cc0a77e

File tree

3 files changed

+38
-2
lines changed

3 files changed

+38
-2
lines changed

llvm-spirv/lib/SPIRV/SPIRVWriter.cpp

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1076,7 +1076,7 @@ void LLVMToSPIRVBase::transVectorComputeMetadata(Function *F) {
10761076
}
10771077
}
10781078

1079-
static void transMetadataDecorations(Metadata *MD, SPIRVEntry *Target);
1079+
static void transMetadataDecorations(Metadata *MD, SPIRVValue *Target);
10801080

10811081
void LLVMToSPIRVBase::transFPGAFunctionMetadata(SPIRVFunction *BF,
10821082
Function *F) {
@@ -2763,7 +2763,7 @@ void checkIsGlobalVar(SPIRVEntry *E, Decoration Dec) {
27632763
ErrStr);
27642764
}
27652765

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

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

27832783
const size_t NumOperands = DecoMD->getNumOperands();
27842784
switch (static_cast<size_t>(DecoKind)) {
2785+
case DecorationAlignment: {
2786+
// Handle Alignment via SPIRVValue::setAlignment() to avoid duplicate
2787+
// Alignment decorations.
2788+
auto *Alignment =
2789+
mdconst::dyn_extract<ConstantInt>(DecoMD->getOperand(1));
2790+
ErrLog.checkError(Alignment, SPIRVEC_InvalidLlvmModule,
2791+
"Alignment operand must be an integer.");
2792+
Target->setAlignment(Alignment->getZExtValue());
2793+
break;
2794+
}
2795+
27852796
ONE_STRING_DECORATION_CASE(MemoryINTEL, spv)
27862797
ONE_STRING_DECORATION_CASE(UserSemantic, spv)
27872798
ONE_INT_DECORATION_CASE(AliasScopeINTEL, spv, SPIRVId)

llvm-spirv/lib/SPIRV/libSPIRV/SPIRVValue.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,14 @@ void SPIRVValue::setAlignment(SPIRVWord A) {
5151
eraseDecorate(DecorationAlignment);
5252
return;
5353
}
54+
SPIRVWord PrevAlignment;
55+
if (hasAlignment(&PrevAlignment)) {
56+
// Do nothing if the Id already has an Alignment decoration, provided
57+
// it matches the new alignment.
58+
assert(A == PrevAlignment &&
59+
"New alignment does not match existing alignment");
60+
return;
61+
}
5462
addDecorate(new SPIRVDecorate(DecorationAlignment, this, A));
5563
SPIRVDBG(spvdbgs() << "Set alignment " << A << " for obj " << Id << "\n")
5664
}

llvm-spirv/test/align-duplicate.ll

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
; RUN: llvm-as %s -o %t.bc
2+
; RUN: llvm-spirv %t.bc -o %t.spv
3+
; RUN: spirv-val %t.spv
4+
5+
; Test that duplicate align information does not result in SPIR-V validation
6+
; errors due to duplicate Alignment Decorations.
7+
8+
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"
9+
target triple = "spir64-unknown-unknown"
10+
11+
define spir_func void @f() {
12+
%res = alloca i16, align 2, !spirv.Decorations !1
13+
ret void
14+
}
15+
16+
!1 = !{!2}
17+
!2 = !{i32 44, i32 2}

0 commit comments

Comments
 (0)