Skip to content

Commit d48d8bc

Browse files
[sycl-post-link] Fix calculation of bool spec constant size (#4819)
Switched to use `DataLayout` instead of `[Scalar/Primitive]TypeSize`, because the latter may not reflect the size of memory allocated for an instance of the type or the number of bytes that are written when an instance of the type is stored to memory. This fixes a few occurrences where booleans considered to be zero bytes long
1 parent 145b9e7 commit d48d8bc

File tree

2 files changed

+32
-37
lines changed

2 files changed

+32
-37
lines changed

llvm/test/tools/sycl-post-link/spec-constants/bool.ll

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,22 @@
11
; RUN: sycl-post-link -spec-const=rt -S %s --ir-output-only -o %t.ll
2-
; RUN: FileCheck %s --input-file=%t.ll --implicit-check-not "call i8 bitcast"
2+
; RUN: FileCheck %s --input-file=%t.ll --implicit-check-not "call i8 bitcast" --check-prefixes=CHECK,CHECK-RT
3+
; RUN: sycl-post-link -spec-const=default -S %s --ir-output-only -o %t.ll
4+
; RUN: FileCheck %s --input-file=%t.ll --check-prefixes=CHECK,CHECK-DEF
35

46
; CHECK-LABEL: void @kernel_A
5-
; CHECK: %[[CALL:.*]] = call i8 @_Z20__spirv_SpecConstantia(i32 [[#]], i8 1)
6-
; CHECK: trunc i8 %[[CALL]] to i1
7+
; CHECK-RT: %[[CALL:.*]] = call i8 @_Z20__spirv_SpecConstantia(i32 [[#]], i8 1)
8+
; CHECK-RT: trunc i8 %[[CALL]] to i1
9+
;
10+
; CHECK-DEF: %[[GEP:gep.*]] = getelementptr i8, i8 addrspace(4)* null, i32 0
11+
; CHECK-DEF: %[[LOAD:load.*]] = load i8, i8 addrspace(4)* %[[GEP]], align 1
12+
; CHECK-DEF: %[[TOBOOL:tobool.*]] = trunc i8 %[[LOAD]] to i1
713
;
814
; CHECK-LABEL: void @kernel_B
9-
; CHECK: call i8 @_Z20__spirv_SpecConstantia(i32 [[#]], i8
15+
; CHECK-RT: call i8 @_Z20__spirv_SpecConstantia(i32 [[#]], i8
16+
;
17+
; CHECK-DEF: %[[GEP:gep.*]] = getelementptr i8, i8 addrspace(4)* null, i32 1
18+
; CHECK-DEF: %[[BC:bc.*]] = bitcast i8 addrspace(4)* %[[GEP]] to %struct.user_type addrspace(4)*
19+
; CHECK-DEF: %[[LOAD:load.*]] = load %struct.user_type, %struct.user_type addrspace(4)* %[[BC]], align 4
1020

1121
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"
1222
target triple = "spir64-unknown-unknown"

llvm/tools/sycl-post-link/SpecConstants.cpp

Lines changed: 18 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -279,10 +279,7 @@ void collectCompositeElementsInfoRecursive(
279279
SpecConstantDescriptor Desc;
280280
Desc.ID = 0; // To be filled later
281281
Desc.Offset = Offset;
282-
// We need to add an additional byte if the type size is not evenly
283-
// divisible by eight, which might be the case for i1, i.e. booleans
284-
Desc.Size = Ty->getPrimitiveSizeInBits() / 8 +
285-
(Ty->getPrimitiveSizeInBits() % 8 != 0);
282+
Desc.Size = M.getDataLayout().getTypeStoreSize(Ty);
286283
Result[Index++] = Desc;
287284
Offset += Desc.Size;
288285
}
@@ -337,8 +334,7 @@ void collectCompositeElementsDefaultValuesRecursive(
337334
// type.
338335
Offset += SL->getSizeInBytes();
339336
} else { // Assume that we encountered some scalar element
340-
int NumBytes = Ty->getScalarSizeInBits() / CHAR_BIT +
341-
(Ty->getScalarSizeInBits() % 8 != 0);
337+
int NumBytes = M.getDataLayout().getTypeStoreSize(Ty);
342338

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

406-
unsigned Size = 0;
407-
if (auto *StructTy = dyn_cast<StructType>(SCTy)) {
408-
const auto *SL = M.getDataLayout().getStructLayout(StructTy);
409-
Size = SL->getSizeInBytes();
410-
} else
411-
Size = SCTy->getScalarSizeInBits() / CHAR_BIT;
402+
unsigned Size = M.getDataLayout().getTypeStoreSize(SCTy);
412403

413404
MDOps.push_back(ConstantAsMetadata::get(
414405
Constant::getIntegerValue(Int32Ty, APInt(32, Size))));
@@ -614,9 +605,7 @@ PreservedAnalyses SpecConstantsPass::run(Module &M,
614605
// to the intrinsic - this should always be possible, as only string
615606
// literals are passed to it in the SYCL RT source code, and application
616607
// code can't use this intrinsic directly.
617-
bool IsComposite =
618-
F.getName().startswith(SYCL_GET_COMPOSITE_SPEC_CONST_VAL) ||
619-
F.getName().startswith(SYCL_GET_COMPOSITE_2020_SPEC_CONST_VAL);
608+
620609
// SYCL 2020 specialization constants provide more functionality so they
621610
// use separate intrinsic with additional arguments.
622611
bool Is2020Intrinsic =
@@ -707,20 +696,7 @@ PreservedAnalyses SpecConstantsPass::run(Module &M,
707696
bool IsNewSpecConstant = Ins.second;
708697
unsigned CurrentOffset = Ins.first->second;
709698
if (IsNewSpecConstant) {
710-
unsigned Size = 0;
711-
if (IsComposite) {
712-
// When handling elements of a structure, we do not use manually
713-
// calculated offsets (which are sum of sizes of all previously
714-
// encountered elements), but instead rely on data provided for us
715-
// by DataLayout, because the structure can be unpacked, i.e.
716-
// padded in order to ensure particular alignment of its elements.
717-
// We rely on the fact that the StructLayout of spec constant RT
718-
// values is the same for the host and the device.
719-
const StructLayout *SL =
720-
M.getDataLayout().getStructLayout(cast<StructType>(SCTy));
721-
Size = SL->getSizeInBytes();
722-
} else
723-
Size = SCTy->getScalarSizeInBits() / CHAR_BIT;
699+
unsigned Size = M.getDataLayout().getTypeStoreSize(SCTy);
724700

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

738-
BitCastInst *BitCast = new BitCastInst(
739-
GEP, PointerType::get(SCTy, GEP->getAddressSpace()), "bc", CI);
740-
741-
Replacement = new LoadInst(SCTy, BitCast, "load", CI);
714+
Instruction *BitCast = nullptr;
715+
if (SCTy->isIntegerTy(1)) // No bitcast to i1 before load
716+
BitCast = GEP;
717+
else
718+
BitCast = new BitCastInst(
719+
GEP, PointerType::get(SCTy, GEP->getAddressSpace()), "bc", CI);
720+
721+
// When we encounter i1 spec constant, we still load the whole byte
722+
Replacement = new LoadInst(SCTy->isIntegerTy(1) ? Int8Ty : SCTy,
723+
BitCast, "load", CI);
724+
if (SCTy->isIntegerTy(1)) // trunc back to i1 if necessary
725+
Replacement = CastInst::CreateIntegerCast(
726+
Replacement, SCTy, /* IsSigned */ false, "tobool", CI);
742727

743728
if (IsNewSpecConstant && DefaultValue)
744729
DefaultsMetadata.push_back(

0 commit comments

Comments
 (0)