Skip to content

Commit

Permalink
Fix target id in decorates when there is forward decl (#2606)
Browse files Browse the repository at this point in the history
When there is forward declaration of a spirv entry, its decorates are
not translated until its definition is seen. Forward id is re-used for
its entry. Id in entry decorates should use forward id as well.

Original commit:
KhronosGroup/SPIRV-LLVM-Translator@305f48884606abf
  • Loading branch information
wenju-he authored and sys-ce-bb committed Jun 27, 2024
1 parent de4b365 commit 6f707a4
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 4 deletions.
10 changes: 10 additions & 0 deletions llvm-spirv/lib/SPIRV/libSPIRV/SPIRVEntry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,16 @@ void SPIRVEntry::takeAnnotations(SPIRVForward *E) {
static_cast<SPIRVFunction *>(this)->takeExecutionModes(E);
}

void SPIRVEntry::replaceTargetIdInDecorates(SPIRVId Id) {
for (auto It = Decorates.begin(), E = Decorates.end(); It != E; ++It)
const_cast<SPIRVDecorate *>(It->second)->setTargetId(Id);
for (auto It = DecorateIds.begin(), E = DecorateIds.end(); It != E; ++It)
const_cast<SPIRVDecorateId *>(It->second)->setTargetId(Id);
for (auto It = MemberDecorates.begin(), E = MemberDecorates.end(); It != E;
++It)
const_cast<SPIRVMemberDecorate *>(It->second)->setTargetId(Id);
}

// Check if an entry has Kind of decoration and get the literal of the
// first decoration of such kind at Index.
bool SPIRVEntry::hasDecorate(Decoration Kind, size_t Index,
Expand Down
1 change: 1 addition & 0 deletions llvm-spirv/lib/SPIRV/libSPIRV/SPIRVEntry.h
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,7 @@ class SPIRVEntry {
void takeDecorates(SPIRVEntry *);
void takeDecorateIds(SPIRVEntry *);
void takeMemberDecorates(SPIRVEntry *);
void replaceTargetIdInDecorates(SPIRVId);

/// After a SPIRV entry is created during reading SPIRV binary by default
/// constructor, this function is called to allow the SPIRV entry to resize
Expand Down
10 changes: 6 additions & 4 deletions llvm-spirv/lib/SPIRV/libSPIRV/SPIRVModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1176,17 +1176,19 @@ SPIRVEntry *SPIRVModuleImpl::replaceForward(SPIRVForward *Forward,
SPIRVEntry *Entry) {
SPIRVId Id = Entry->getId();
SPIRVId ForwardId = Forward->getId();
if (ForwardId == Id)
if (ForwardId == Id) {
IdEntryMap[Id] = Entry;
else {
// Annotations include name, decorations, execution modes
Entry->takeAnnotations(Forward);
} else {
auto Loc = IdEntryMap.find(Id);
assert(Loc != IdEntryMap.end());
IdEntryMap.erase(Loc);
Entry->setId(ForwardId);
IdEntryMap[ForwardId] = Entry;
// Replace current Id with ForwardId in decorates.
Entry->replaceTargetIdInDecorates(ForwardId);
}
// Annotations include name, decorations, execution modes
Entry->takeAnnotations(Forward);
delete Forward;
return Entry;
}
Expand Down
39 changes: 39 additions & 0 deletions llvm-spirv/test/transcoding/decoration-forward-decl.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
; RUN: llvm-as %s -o %t.bc
; RUN: llvm-spirv %t.bc -spirv-text -o - | FileCheck %s --check-prefix=CHECK-SPIRV
; RUN: llvm-spirv %t.bc -o %t.spv
; RUN: spirv-val %t.spv
; RUN: llvm-spirv -r %t.spv -o %t.rev.bc
; RUN: llvm-dis < %t.rev.bc | FileCheck %s --check-prefix=CHECK-LLVM

; Check saturation conversion is translated when there is forward declaration
; of SPIRV entry.

; CHECK-SPIRV: Decorate [[SAT:[0-9]+]] SaturatedConversion
; CHECK-SPIRV: ConvertFToU {{[0-9]+}} [[SAT]]

; CHECK-LLVM: convert_uchar_satf

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

declare spir_func zeroext i8 @_Z30__spirv_ConvertFToU_Ruchar_satf(float)

define spir_func void @forward(float %val, ptr addrspace(1) %dst) {
entry:
br label %for.cond

for.cond: ; preds = %for.body, %entry
%new_val.0 = phi i8 [ undef, %entry ], [ %call1, %for.body ]
%i.0 = phi i32 [ 0, %entry ], [ %inc, %for.body ]
%cmp = icmp ult i32 %i.0, 1
br i1 %cmp, label %for.body, label %for.end

for.body: ; preds = %for.cond
%call1 = call spir_func zeroext i8 @_Z30__spirv_ConvertFToU_Ruchar_satf(float noundef %val)
%inc = add i32 %i.0, 1
br label %for.cond

for.end: ; preds = %for.cond
store i8 %new_val.0, ptr addrspace(1) %dst, align 1
ret void
}

0 comments on commit 6f707a4

Please sign in to comment.