Skip to content

Commit 4256e4e

Browse files
svenvhdwoodwor-intel
svenvh
authored and
dwoodwor-intel
committed
Use Sign/ZeroExtend image operands for read/write_image
Emit (in OCLToSPIRV) and decode (in SPIRVToOCL) the SignExtend and ZeroExtend Image Operands introduced by SPIR-V version 1.4 to preserve signedness of image read and write operations. This allows distinguishing between e.g. `read_imagei` and `read_imageui` and enables a lossless LLVM -> SPIR-V -> LLVM conversion for those OpenCL builtins. Before SPIR-V 1.4, there was no way to represent the signedness of the image operations and llvm-spirv defaulted to signed. This commit leaves that behaviour unchanged if llvm-spirv's maximum version is set to SPIR-V 1.3 or below. Add the `--spirv-max-version=1.3` flag to some tests that rely on the old behaviour. Original commit: KhronosGroup/SPIRV-LLVM-Translator@bb4ead0
1 parent bf77879 commit 4256e4e

File tree

9 files changed

+131
-32
lines changed

9 files changed

+131
-32
lines changed

‎llvm-spirv/lib/SPIRV/OCLToSPIRV.cpp

Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,20 @@ static Type *getBlockStructType(Value *Parameter) {
8484
return ParamType;
8585
}
8686

87+
/// Return one of the SPIR-V 1.4 SignExtend or ZeroExtend image operands
88+
/// for a demangled function name, or 0 if the function does not return an
89+
/// integer type (e.g. read_imagef).
90+
static unsigned getImageSignZeroExt(StringRef DemangledName) {
91+
bool IsSigned = !DemangledName.endswith("ui") && DemangledName.back() == 'i';
92+
bool IsUnsigned = DemangledName.endswith("ui");
93+
94+
if (IsSigned)
95+
return ImageOperandsMask::ImageOperandsSignExtendMask;
96+
if (IsUnsigned)
97+
return ImageOperandsMask::ImageOperandsZeroExtendMask;
98+
return 0;
99+
}
100+
87101
bool OCLToSPIRVLegacy::runOnModule(Module &M) {
88102
setOCLTypeToSPIRV(&getAnalysis<OCLTypeToSPIRVLegacy>());
89103
return runOCLToSPIRV(M);
@@ -266,7 +280,7 @@ void OCLToSPIRVBase::visitCallInst(CallInst &CI) {
266280
}
267281
if (DemangledName.find(kOCLBuiltinName::ReadImage) == 0) {
268282
if (MangledName.find(kMangledName::Sampler) != StringRef::npos) {
269-
visitCallReadImageWithSampler(&CI, MangledName);
283+
visitCallReadImageWithSampler(&CI, MangledName, DemangledName);
270284
return;
271285
}
272286
if (MangledName.find("msaa") != StringRef::npos) {
@@ -973,7 +987,8 @@ void OCLToSPIRVBase::visitCallReadImageMSAA(CallInst *CI,
973987
}
974988

975989
void OCLToSPIRVBase::visitCallReadImageWithSampler(CallInst *CI,
976-
StringRef MangledName) {
990+
StringRef MangledName,
991+
StringRef DemangledName) {
977992
assert(MangledName.find(kMangledName::Sampler) != StringRef::npos);
978993
assert(CI->getCalledFunction() && "Unexpected indirect call");
979994
Function *Func = CI->getCalledFunction();
@@ -999,22 +1014,26 @@ void OCLToSPIRVBase::visitCallReadImageWithSampler(CallInst *CI,
9991014
Args[0] = SampledImg;
10001015
Args.erase(Args.begin() + 1, Args.begin() + 2);
10011016

1017+
unsigned ImgOpMask = getImageSignZeroExt(DemangledName);
1018+
unsigned ImgOpMaskInsIndex = Args.size();
10021019
switch (Args.size()) {
10031020
case 2: // no lod
1004-
Args.push_back(getInt32(M, ImageOperandsMask::ImageOperandsLodMask));
1021+
ImgOpMask |= ImageOperandsMask::ImageOperandsLodMask;
1022+
ImgOpMaskInsIndex = Args.size();
10051023
Args.push_back(getFloat32(M, 0.f));
10061024
break;
10071025
case 3: // explicit lod
1008-
Args.insert(Args.begin() + 2,
1009-
getInt32(M, ImageOperandsMask::ImageOperandsLodMask));
1026+
ImgOpMask |= ImageOperandsMask::ImageOperandsLodMask;
1027+
ImgOpMaskInsIndex = 2;
10101028
break;
10111029
case 4: // gradient
1012-
Args.insert(Args.begin() + 2,
1013-
getInt32(M, ImageOperandsMask::ImageOperandsGradMask));
1030+
ImgOpMask |= ImageOperandsMask::ImageOperandsGradMask;
1031+
ImgOpMaskInsIndex = 2;
10141032
break;
10151033
default:
10161034
assert(0 && "read_image* with unhandled number of args!");
10171035
}
1036+
Args.insert(Args.begin() + ImgOpMaskInsIndex, getInt32(M, ImgOpMask));
10181037

10191038
// SPIR-V instruction always returns 4-element vector
10201039
if (IsRetScalar)
@@ -1130,19 +1149,32 @@ void OCLToSPIRVBase::visitCallBuiltinSimple(CallInst *CI, StringRef MangledName,
11301149
void OCLToSPIRVBase::visitCallReadWriteImage(CallInst *CI,
11311150
StringRef DemangledName) {
11321151
OCLBuiltinTransInfo Info;
1133-
if (DemangledName.find(kOCLBuiltinName::ReadImage) == 0)
1152+
if (DemangledName.find(kOCLBuiltinName::ReadImage) == 0) {
11341153
Info.UniqName = kOCLBuiltinName::ReadImage;
1154+
unsigned ImgOpMask = getImageSignZeroExt(DemangledName);
1155+
if (ImgOpMask) {
1156+
Info.PostProc = [&](std::vector<Value *> &Args) {
1157+
Args.push_back(getInt32(M, ImgOpMask));
1158+
};
1159+
}
1160+
}
11351161

11361162
if (DemangledName.find(kOCLBuiltinName::WriteImage) == 0) {
11371163
Info.UniqName = kOCLBuiltinName::WriteImage;
11381164
Info.PostProc = [&](std::vector<Value *> &Args) {
1165+
unsigned ImgOpMask = getImageSignZeroExt(DemangledName);
1166+
unsigned ImgOpMaskInsIndex = Args.size();
11391167
if (Args.size() == 4) // write with lod
11401168
{
11411169
auto Lod = Args[2];
11421170
Args.erase(Args.begin() + 2);
1143-
Args.push_back(getInt32(M, ImageOperandsMask::ImageOperandsLodMask));
1171+
ImgOpMask |= ImageOperandsMask::ImageOperandsLodMask;
1172+
ImgOpMaskInsIndex = Args.size();
11441173
Args.push_back(Lod);
11451174
}
1175+
if (ImgOpMask) {
1176+
Args.insert(Args.begin() + ImgOpMaskInsIndex, getInt32(M, ImgOpMask));
1177+
}
11461178
};
11471179
}
11481180

‎llvm-spirv/lib/SPIRV/OCLToSPIRV.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,8 @@ class OCLToSPIRVBase : public InstVisitor<OCLToSPIRVBase> {
169169
/// read_image(image, sampler, ...) =>
170170
/// sampled_image = __spirv_SampledImage(image, sampler);
171171
/// return __spirv_ImageSampleExplicitLod_R{ReturnType}(sampled_image, ...);
172-
void visitCallReadImageWithSampler(CallInst *CI, StringRef MangledName);
172+
void visitCallReadImageWithSampler(CallInst *CI, StringRef MangledName,
173+
StringRef DemangledName);
173174

174175
/// Transform read_image with msaa image arguments.
175176
/// Sample argument must be acoded as Image Operand.

‎llvm-spirv/lib/SPIRV/SPIRVToOCL.cpp

Lines changed: 34 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -751,27 +751,43 @@ void SPIRVToOCLBase::visitCallGroupWaitEvents(CallInst *CI, Op OC) {
751751
&Attrs);
752752
}
753753

754-
static char getTypeSuffix(Type *T) {
755-
char Suffix;
754+
static std::string getTypeSuffix(Type *T, bool IsSigned) {
755+
std::string Suffix;
756756

757757
Type *ST = T->getScalarType();
758758
if (ST->isHalfTy())
759-
Suffix = 'h';
759+
Suffix = "h";
760760
else if (ST->isFloatTy())
761-
Suffix = 'f';
761+
Suffix = "f";
762+
else if (IsSigned)
763+
Suffix = "i";
762764
else
763-
Suffix = 'i';
765+
Suffix = "ui";
764766

765767
return Suffix;
766768
}
767769

768770
void SPIRVToOCLBase::mutateArgsForImageOperands(std::vector<Value *> &Args,
769-
unsigned ImOpArgIndex) {
771+
unsigned ImOpArgIndex,
772+
bool &IsSigned) {
773+
// Default to signed.
774+
IsSigned = true;
770775
if (Args.size() > ImOpArgIndex) {
771776
ConstantInt *ImOp = dyn_cast<ConstantInt>(Args[ImOpArgIndex]);
772777
uint64_t ImOpValue = 0;
773778
if (ImOp)
774779
ImOpValue = ImOp->getZExtValue();
780+
unsigned SignZeroExtMasks = ImageOperandsMask::ImageOperandsSignExtendMask |
781+
ImageOperandsMask::ImageOperandsZeroExtendMask;
782+
// If one of the SPIR-V 1.4 SignExtend/ZeroExtend operands is present, take
783+
// it into account and drop the mask.
784+
if (ImOpValue & SignZeroExtMasks) {
785+
if (ImOpValue & ImageOperandsMask::ImageOperandsZeroExtendMask)
786+
IsSigned = false;
787+
ImOpValue &= ~SignZeroExtMasks;
788+
Args[3] = getInt32(M, ImOpValue);
789+
ImOp = cast<ConstantInt>(Args[3]);
790+
}
775791
// Drop "Image Operands" argument.
776792
Args.erase(Args.begin() + ImOpArgIndex);
777793

@@ -785,7 +801,6 @@ void SPIRVToOCLBase::mutateArgsForImageOperands(std::vector<Value *> &Args,
785801
}
786802
}
787803

788-
// TODO: Handle unsigned integer return type. May need spec change.
789804
void SPIRVToOCLBase::visitCallSPIRVImageSampleExplicitLodBuiltIn(CallInst *CI,
790805
Op OC) {
791806
assert(CI->getCalledFunction() && "Unexpected indirect call");
@@ -807,7 +822,8 @@ void SPIRVToOCLBase::visitCallSPIRVImageSampleExplicitLodBuiltIn(CallInst *CI,
807822
auto Sampler = CallSampledImg->getArgOperand(1);
808823
Args[0] = Img;
809824
Args.insert(Args.begin() + 1, Sampler);
810-
mutateArgsForImageOperands(Args, 3);
825+
bool IsSigned;
826+
mutateArgsForImageOperands(Args, 3, IsSigned);
811827
if (CallSampledImg->hasOneUse()) {
812828
CallSampledImg->replaceAllUsesWith(
813829
UndefValue::get(CallSampledImg->getType()));
@@ -818,7 +834,8 @@ void SPIRVToOCLBase::visitCallSPIRVImageSampleExplicitLodBuiltIn(CallInst *CI,
818834
if (auto VT = dyn_cast<VectorType>(T))
819835
T = VT->getElementType();
820836
RetTy = IsDepthImage ? T : CI->getType();
821-
return std::string(kOCLBuiltinName::SampledReadImage) + getTypeSuffix(T);
837+
return std::string(kOCLBuiltinName::SampledReadImage) +
838+
getTypeSuffix(T, IsSigned);
822839
};
823840

824841
auto ModifyRetTy = [=](CallInst *NewCI) -> Instruction * {
@@ -842,11 +859,13 @@ void SPIRVToOCLBase::visitCallSPIRVImageWriteBuiltIn(CallInst *CI, Op OC) {
842859
M, CI,
843860
[=](CallInst *, std::vector<Value *> &Args) {
844861
llvm::Type *T = Args[2]->getType();
845-
mutateArgsForImageOperands(Args, 3);
862+
bool IsSigned;
863+
mutateArgsForImageOperands(Args, 3, IsSigned);
846864
if (Args.size() > 3) {
847865
std::swap(Args[2], Args[3]);
848866
}
849-
return std::string(kOCLBuiltinName::WriteImage) + getTypeSuffix(T);
867+
return std::string(kOCLBuiltinName::WriteImage) +
868+
getTypeSuffix(T, IsSigned);
850869
},
851870
&Attrs);
852871
}
@@ -857,9 +876,11 @@ void SPIRVToOCLBase::visitCallSPIRVImageReadBuiltIn(CallInst *CI, Op OC) {
857876
mutateCallInstOCL(
858877
M, CI,
859878
[=](CallInst *, std::vector<Value *> &Args) {
860-
mutateArgsForImageOperands(Args, 2);
879+
bool IsSigned;
880+
mutateArgsForImageOperands(Args, 2, IsSigned);
861881
llvm::Type *T = CI->getType();
862-
return std::string(kOCLBuiltinName::ReadImage) + getTypeSuffix(T);
882+
return std::string(kOCLBuiltinName::ReadImage) +
883+
getTypeSuffix(T, IsSigned);
863884
},
864885
&Attrs);
865886
}

‎llvm-spirv/lib/SPIRV/SPIRVToOCL.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -273,9 +273,10 @@ class SPIRVToOCLBase : public InstVisitor<SPIRVToOCLBase> {
273273
std::string translateOpaqueType(StringRef STName);
274274

275275
/// Mutate the argument list based on (optional) image operands at position
276-
/// ImOpArgIndex.
276+
/// ImOpArgIndex. Set IsSigned according to any SignExtend/ZeroExtend Image
277+
/// Operands present in Args, or default to signed if there are none.
277278
void mutateArgsForImageOperands(std::vector<Value *> &Args,
278-
unsigned ImOpArgIndex);
279+
unsigned ImOpArgIndex, bool &IsSigned);
279280

280281
protected:
281282
Module *M;

‎llvm-spirv/lib/SPIRV/libSPIRV/SPIRVInstruction.cpp

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,47 @@ std::vector<SPIRVType *> SPIRVInstruction::getOperandTypes() {
157157
return getOperandTypes(getOperands());
158158
}
159159

160+
size_t SPIRVImageInstBase::getImageOperandsIndex() const {
161+
switch (OpCode) {
162+
case OpImageRead:
163+
case OpImageSampleExplicitLod:
164+
return 2;
165+
case OpImageWrite:
166+
return 3;
167+
default:
168+
return ~0U;
169+
}
170+
}
171+
172+
void SPIRVImageInstBase::setOpWords(const std::vector<SPIRVWord> &OpsArg) {
173+
std::vector<SPIRVWord> Ops = OpsArg;
174+
175+
// If the Image Operands field has the SignExtend or ZeroExtend bit set,
176+
// either raise the minimum SPIR-V version to 1.4, or drop the operand
177+
// if SPIR-V 1.4 cannot be emitted.
178+
size_t ImgOpsIndex = getImageOperandsIndex();
179+
if (ImgOpsIndex != ~0U && ImgOpsIndex < Ops.size()) {
180+
SPIRVWord ImgOps = Ops[ImgOpsIndex];
181+
unsigned SignZeroExtMasks = ImageOperandsMask::ImageOperandsSignExtendMask |
182+
ImageOperandsMask::ImageOperandsZeroExtendMask;
183+
if (ImgOps & SignZeroExtMasks) {
184+
SPIRVModule *M = getModule();
185+
if (M->isAllowedToUseVersion(VersionNumber::SPIRV_1_4)) {
186+
M->setMinSPIRVVersion(static_cast<SPIRVWord>(VersionNumber::SPIRV_1_4));
187+
} else {
188+
// Drop SignExtend/ZeroExtend if we cannot use SPIR-V 1.4.
189+
Ops[ImgOpsIndex] &= ~SignZeroExtMasks;
190+
if (Ops[ImgOpsIndex] == 0) {
191+
// Drop the Image Operands if SignExtend/ZeroExtend was the only
192+
// bit set.
193+
Ops.pop_back();
194+
}
195+
}
196+
}
197+
}
198+
SPIRVInstTemplateBase::setOpWords(Ops);
199+
}
200+
160201
bool isSpecConstantOpAllowedOp(Op OC) {
161202
static SPIRVWord Table[] = {
162203
OpSConvert,

‎llvm-spirv/lib/SPIRV/libSPIRV/SPIRVInstruction.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2763,6 +2763,12 @@ class SPIRVImageInstBase : public SPIRVInstTemplateBase {
27632763
SPIRVCapVec getRequiredCapability() const override {
27642764
return getVec(CapabilityImageBasic);
27652765
}
2766+
2767+
protected:
2768+
void setOpWords(const std::vector<SPIRVWord> &OpsArg) override;
2769+
2770+
private:
2771+
size_t getImageOperandsIndex() const;
27662772
};
27672773

27682774
#define _SPIRV_OP(x, ...) \

‎llvm-spirv/test/read_image.cl

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
// RUN: %clang_cc1 -triple spir64 -fdeclare-opencl-builtins -finclude-default-header -O0 -cl-std=CL2.0 -emit-llvm-bc %s -o %t.bc -no-opaque-pointers
2-
// RUN: llvm-spirv %t.bc -o %t.spv
2+
// RUN: llvm-spirv --spirv-max-version=1.3 %t.bc -o %t.spv
33
// RUN: spirv-val %t.spv
44
// RUN: llvm-spirv %t.spv -to-text -o - | FileCheck %s --check-prefix=CHECK-SPIRV
55
// RUN: llvm-spirv -r %t.spv -o %t.rev.bc --spirv-target-env=SPV-IR
66
// RUN: llvm-dis < %t.rev.bc | FileCheck %s --check-prefix=CHECK-SPV-LLVM
7-
// RUN: llvm-spirv %t.rev.bc -o %t.rev.spv
7+
// RUN: llvm-spirv --spirv-max-version=1.3 %t.rev.bc -o %t.rev.spv
88
// RUN: spirv-val %t.rev.spv
9-
// RUN: llvm-spirv %t.rev.spv -to-text -o - | FileCheck %s --check-prefix=CHECK-SPIRV
9+
// RUN: llvm-spirv --spirv-max-version=1.3 %t.rev.spv -to-text -o - | FileCheck %s --check-prefix=CHECK-SPIRV
1010

1111
// CHECK-SPIRV: TypeInt [[IntTy:[0-9]+]]
1212
// CHECK-SPIRV: TypeVector [[IVecTy:[0-9]+]] [[IntTy]]

‎llvm-spirv/test/transcoding/SampledImage.cl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// RUN: %clang_cc1 -triple spir -cl-std=CL2.0 %s -fdeclare-opencl-builtins -finclude-default-header -emit-llvm-bc -o %t.bc -no-opaque-pointers
22
// RUN: llvm-spirv %t.bc -spirv-text -o %t.txt
33
// RUN: FileCheck < %t.txt %s --check-prefix=CHECK-SPIRV
4-
// RUN: llvm-spirv %t.bc -o %t.spv
4+
// RUN: llvm-spirv --spirv-max-version=1.3 %t.bc -o %t.spv
55
// RUN: spirv-val %t.spv
66
// RUN: llvm-spirv -r %t.spv -o %t.rev.bc
77
// RUN: llvm-dis < %t.rev.bc | FileCheck %s --check-prefix=CHECK-LLVM

‎llvm-spirv/test/transcoding/image_signedness.ll

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,5 @@
11
; Test that signedness of calls to read_image(u)i/write_image(u)i is preserved.
22

3-
; TODO: Translator does not handle signedness for read_image/write_image yet.
4-
; XFAIL: *
5-
63
; RUN: llvm-as %s -o %t.bc
74
; RUN: llvm-spirv %t.bc -o %t.spv
85
; RUN: spirv-val %t.spv

0 commit comments

Comments
 (0)