Skip to content

[sycl-post-link] Fix calculation of bool spec constant size #4819

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 27, 2021
Merged

[sycl-post-link] Fix calculation of bool spec constant size #4819

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
297ca9e
[sycl-post-link] Fix calculation of bool spec constant size
AlexeySachkov Oct 26, 2021
6973242
Apply comments
AlexeySachkov Oct 26, 2021
39ac7f9
Use DataLayout to compute type sizes
AlexeySachkov Oct 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions llvm/test/tools/sycl-post-link/spec-constants/bool.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,22 @@
; RUN: sycl-post-link -spec-const=rt -S %s --ir-output-only -o %t.ll
; RUN: FileCheck %s --input-file=%t.ll --implicit-check-not "call i8 bitcast"
; RUN: FileCheck %s --input-file=%t.ll --implicit-check-not "call i8 bitcast" --check-prefixes=CHECK,CHECK-RT
; RUN: sycl-post-link -spec-const=default -S %s --ir-output-only -o %t.ll
; RUN: FileCheck %s --input-file=%t.ll --check-prefixes=CHECK,CHECK-DEF

; CHECK-LABEL: void @kernel_A
; CHECK: %[[CALL:.*]] = call i8 @_Z20__spirv_SpecConstantia(i32 [[#]], i8 1)
; CHECK: trunc i8 %[[CALL]] to i1
; CHECK-RT: %[[CALL:.*]] = call i8 @_Z20__spirv_SpecConstantia(i32 [[#]], i8 1)
; CHECK-RT: trunc i8 %[[CALL]] to i1
;
; CHECK-DEF: %[[GEP:gep.*]] = getelementptr i8, i8 addrspace(4)* null, i32 0
; CHECK-DEF: %[[LOAD:load.*]] = load i8, i8 addrspace(4)* %[[GEP]], align 1
; CHECK-DEF: %[[TOBOOL:tobool.*]] = trunc i8 %[[LOAD]] to i1
;
; CHECK-LABEL: void @kernel_B
; CHECK: call i8 @_Z20__spirv_SpecConstantia(i32 [[#]], i8
; CHECK-RT: call i8 @_Z20__spirv_SpecConstantia(i32 [[#]], i8
;
; CHECK-DEF: %[[GEP:gep.*]] = getelementptr i8, i8 addrspace(4)* null, i32 1
; CHECK-DEF: %[[BC:bc.*]] = bitcast i8 addrspace(4)* %[[GEP]] to %struct.user_type addrspace(4)*
; CHECK-DEF: %[[LOAD:load.*]] = load %struct.user_type, %struct.user_type addrspace(4)* %[[BC]], align 4

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"
Expand Down
51 changes: 18 additions & 33 deletions llvm/tools/sycl-post-link/SpecConstants.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,10 +279,7 @@ void collectCompositeElementsInfoRecursive(
SpecConstantDescriptor Desc;
Desc.ID = 0; // To be filled later
Desc.Offset = Offset;
// We need to add an additional byte if the type size is not evenly
// divisible by eight, which might be the case for i1, i.e. booleans
Desc.Size = Ty->getPrimitiveSizeInBits() / 8 +
(Ty->getPrimitiveSizeInBits() % 8 != 0);
Desc.Size = M.getDataLayout().getTypeStoreSize(Ty);
Result[Index++] = Desc;
Offset += Desc.Size;
}
Expand Down Expand Up @@ -337,8 +334,7 @@ void collectCompositeElementsDefaultValuesRecursive(
// type.
Offset += SL->getSizeInBytes();
} else { // Assume that we encountered some scalar element
int NumBytes = Ty->getScalarSizeInBits() / CHAR_BIT +
(Ty->getScalarSizeInBits() % 8 != 0);
int NumBytes = M.getDataLayout().getTypeStoreSize(Ty);

if (auto IntConst = dyn_cast<ConstantInt>(C)) {
auto Val = IntConst->getValue().getZExtValue();
Expand Down Expand Up @@ -403,12 +399,7 @@ MDNode *generateSpecConstantMetadata(const Module &M, StringRef SymbolicID,
MDOps.push_back(ConstantAsMetadata::get(
Constant::getIntegerValue(Int32Ty, APInt(32, 0))));

unsigned Size = 0;
if (auto *StructTy = dyn_cast<StructType>(SCTy)) {
const auto *SL = M.getDataLayout().getStructLayout(StructTy);
Size = SL->getSizeInBytes();
} else
Size = SCTy->getScalarSizeInBits() / CHAR_BIT;
unsigned Size = M.getDataLayout().getTypeStoreSize(SCTy);

MDOps.push_back(ConstantAsMetadata::get(
Constant::getIntegerValue(Int32Ty, APInt(32, Size))));
Expand Down Expand Up @@ -614,9 +605,7 @@ PreservedAnalyses SpecConstantsPass::run(Module &M,
// to the intrinsic - this should always be possible, as only string
// literals are passed to it in the SYCL RT source code, and application
// code can't use this intrinsic directly.
bool IsComposite =
F.getName().startswith(SYCL_GET_COMPOSITE_SPEC_CONST_VAL) ||
F.getName().startswith(SYCL_GET_COMPOSITE_2020_SPEC_CONST_VAL);

// SYCL 2020 specialization constants provide more functionality so they
// use separate intrinsic with additional arguments.
bool Is2020Intrinsic =
Expand Down Expand Up @@ -707,20 +696,7 @@ PreservedAnalyses SpecConstantsPass::run(Module &M,
bool IsNewSpecConstant = Ins.second;
unsigned CurrentOffset = Ins.first->second;
if (IsNewSpecConstant) {
unsigned Size = 0;
if (IsComposite) {
// When handling elements of a structure, we do not use manually
// calculated offsets (which are sum of sizes of all previously
// encountered elements), but instead rely on data provided for us
// by DataLayout, because the structure can be unpacked, i.e.
// padded in order to ensure particular alignment of its elements.
// We rely on the fact that the StructLayout of spec constant RT
// values is the same for the host and the device.
const StructLayout *SL =
M.getDataLayout().getStructLayout(cast<StructType>(SCTy));
Size = SL->getSizeInBytes();
} else
Size = SCTy->getScalarSizeInBits() / CHAR_BIT;
unsigned Size = M.getDataLayout().getTypeStoreSize(SCTy);

SCMetadata[SymID] = generateSpecConstantMetadata(
M, SymID, SCTy, NextID, /* is native spec constant */ false);
Expand All @@ -735,10 +711,19 @@ PreservedAnalyses SpecConstantsPass::run(Module &M,
Int8Ty, RTBuffer,
{ConstantInt::get(Int32Ty, CurrentOffset, false)}, "gep", CI);

BitCastInst *BitCast = new BitCastInst(
GEP, PointerType::get(SCTy, GEP->getAddressSpace()), "bc", CI);

Replacement = new LoadInst(SCTy, BitCast, "load", CI);
Instruction *BitCast = nullptr;
if (SCTy->isIntegerTy(1)) // No bitcast to i1 before load
BitCast = GEP;
else
BitCast = new BitCastInst(
GEP, PointerType::get(SCTy, GEP->getAddressSpace()), "bc", CI);

// When we encounter i1 spec constant, we still load the whole byte
Replacement = new LoadInst(SCTy->isIntegerTy(1) ? Int8Ty : SCTy,
BitCast, "load", CI);
if (SCTy->isIntegerTy(1)) // trunc back to i1 if necessary
Replacement = CastInst::CreateIntegerCast(
Replacement, SCTy, /* IsSigned */ false, "tobool", CI);

if (IsNewSpecConstant && DefaultValue)
DefaultsMetadata.push_back(
Expand Down