Skip to content

Commit

Permalink
Fix OpFMod, implement OpSMod and add SPIR-V assembly tests
Browse files Browse the repository at this point in the history 
OpFMod(a, b) was implemented as copysign(frem(a, b), b), which is not a
remainder as described in §2.2.3 Computation of the SPIR-V specification.

For example, OpFMod(-1.0, 100.0) resulted in 1.0, but the correct result
is 99.0.  Likewise, OpFMod(1.0, -100.0) should result in -99.0, not -1.0.

Fix this by adding the divisor ('b') if the signs of the operands differ
and the result of frem(a, b) is non-zero.

As frem(a, b) returns a result of the same sign as the dividend ('a'),
the sign can be compared by taking the result of frem(a, b), copying the
sign of the divisor to the result, and performing a test for equality to
determine whether copying the sign modifies the value.

If the divisor is infinity, then the result of the OpFMod operation will
also be infinity.  If either operand is a NaN, then the result will also
be a NaN.  If the result is infinity, a NaN, or zero, then it will have
the correct sign (i.e. the sign of the dividend).

This also implements OpSMod(a, b) in terms of srem(a, b).  In this case,
the sign is compared by comparing (a ^ b) < 0, which is true if and only
if the sign bits differ.
  • Loading branch information
@StuartDBrady @svenvh
StuartDBrady authored and svenvh committed Sep 27, 2019
1 parent 99e0a01 commit 4de4935
Show file tree
Hide file tree
Showing 8 changed files with 169 additions and 99 deletions.
60 changes: 34 additions & 26 deletions lib/SPIRV/SPIRVReader.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1789,36 +1789,44 @@ Value *SPIRVToLLVM::transValueWithoutDecoration(SPIRVValue *BV, Function *F,
}

case OpFMod: {
// translate OpFMod(a, b) to copysign(frem(a, b), b)
// translate OpFMod(a, b) to:
// r = frem(a, b)
// c = copysign(r, b)
// needs_fixing = islessgreater(r, c)
// result = needs_fixing ? r + b : c
IRBuilder<> Builder(BB);
SPIRVFMod *FMod = static_cast<SPIRVFMod *>(BV);
auto Dividend = transValue(FMod->getDividend(), F, BB);
auto Divisor = transValue(FMod->getDivisor(), F, BB);
auto FRem = BinaryOperator::CreateFRem(Dividend, Divisor, "frem.res", BB);

std::string UnmangledName = OCLExtOpMap::map(OpenCLLIB::Copysign);
std::string MangledName = "copysign";

std::vector<Type *> ArgTypes;
ArgTypes.push_back(FRem->getType());
ArgTypes.push_back(Divisor->getType());
mangleOpenClBuiltin(UnmangledName, ArgTypes, MangledName);

auto FT = FunctionType::get(transType(BV->getType()), ArgTypes, false);
auto Func =
Function::Create(FT, GlobalValue::ExternalLinkage, MangledName, M);
Func->setCallingConv(CallingConv::SPIR_FUNC);
if (isFuncNoUnwind())
Func->addFnAttr(Attribute::NoUnwind);

std::vector<Value *> Args;
Args.push_back(FRem);
Args.push_back(Divisor);

auto Call = CallInst::Create(Func, Args, "copysign", BB);
setCallingConv(Call);
addFnAttr(Call, Attribute::NoUnwind);
return mapValue(BV, Call);
auto FRem = Builder.CreateFRem(Dividend, Divisor, "frem.res");
auto CopySign = Builder.CreateBinaryIntrinsic(
llvm::Intrinsic::copysign, FRem, Divisor, nullptr, "copysign.res");
auto FAdd = Builder.CreateFAdd(FRem, Divisor, "fadd.res");
auto Cmp = Builder.CreateFCmpONE(FRem, CopySign, "cmp.res");
auto Select = Builder.CreateSelect(Cmp, FAdd, CopySign);
return mapValue(BV, Select);
}

case OpSMod: {
// translate OpSMod(a, b) to:
// r = srem(a, b)
// needs_fixing = ((a < 0) != (b < 0) && r != 0)
// result = needs_fixing ? r + b : r
IRBuilder<> Builder(BB);
SPIRVSMod *SMod = static_cast<SPIRVSMod *>(BV);
auto Dividend = transValue(SMod->getDividend(), F, BB);
auto Divisor = transValue(SMod->getDivisor(), F, BB);
auto SRem = Builder.CreateSRem(Dividend, Divisor, "srem.res");
auto Xor = Builder.CreateXor(Dividend, Divisor, "xor.res");
auto Zero = ConstantInt::getNullValue(Dividend->getType());
auto CmpSign = Builder.CreateICmpSLT(Xor, Zero, "cmpsign.res");
auto CmpSRem = Builder.CreateICmpNE(SRem, Zero, "cmpsrem.res");
auto Add = Builder.CreateNSWAdd(SRem, Divisor, "add.res");
auto Cmp = Builder.CreateAnd(CmpSign, CmpSRem, "cmp.res");
auto Select = Builder.CreateSelect(Cmp, Add, SRem);
return mapValue(BV, Select);
}

case OpFNegate: {
SPIRVUnary *BC = static_cast<SPIRVUnary *>(BV);
return mapValue(
Expand Down
1 change: 0 additions & 1 deletion lib/SPIRV/libSPIRV/SPIRVEntry.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -786,7 +786,6 @@ _SPIRV_OP(ImageDrefGather)
_SPIRV_OP(QuantizeToF16)
_SPIRV_OP(Transpose)
_SPIRV_OP(ArrayLength)
_SPIRV_OP(SMod)
_SPIRV_OP(VectorTimesMatrix)
_SPIRV_OP(MatrixTimesVector)
_SPIRV_OP(MatrixTimesMatrix)
Expand Down
33 changes: 27 additions & 6 deletions lib/SPIRV/libSPIRV/SPIRVInstruction.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1153,22 +1153,21 @@ class SPIRVSwitch : public SPIRVInstruction {
std::vector<SPIRVWord> Pairs;
};

class SPIRVFMod : public SPIRVInstruction {
class SPIRVFSMod : public SPIRVInstruction {
public:
static const Op OC = OpFMod;
static const SPIRVWord FixedWordCount = 4;
// Complete constructor
SPIRVFMod(SPIRVType *TheType, SPIRVId TheId, SPIRVId TheDividend,
SPIRVId TheDivisor, SPIRVBasicBlock *BB)
SPIRVFSMod(Op OC, SPIRVType *TheType, SPIRVId TheId, SPIRVId TheDividend,
SPIRVId TheDivisor, SPIRVBasicBlock *BB)
: SPIRVInstruction(5, OC, TheType, TheId, BB), Dividend(TheDividend),
Divisor(TheDivisor) {
validate();
assert(BB && "Invalid BB");
}
// Incomplete constructor
SPIRVFMod()
SPIRVFSMod(Op OC)
: SPIRVInstruction(OC), Dividend(SPIRVID_INVALID),
Divisor(SPIRVID_INVALID) {}

SPIRVValue *getDividend() const { return getValue(Dividend); }
SPIRVValue *getDivisor() const { return getValue(Divisor); }

Expand All @@ -1195,6 +1194,28 @@ class SPIRVFMod : public SPIRVInstruction {
SPIRVId Divisor;
};

class SPIRVFMod : public SPIRVFSMod {
public:
static const Op OC = OpFMod;
// Complete constructor
SPIRVFMod(SPIRVType *TheType, SPIRVId TheId, SPIRVId TheDividend,
SPIRVId TheDivisor, SPIRVBasicBlock *BB)
: SPIRVFSMod(OC, TheType, TheId, TheDividend, TheDivisor, BB) {}
// Incomplete constructor
SPIRVFMod() : SPIRVFSMod(OC) {}
};

class SPIRVSMod : public SPIRVFSMod {
public:
static const Op OC = OpSMod;
// Complete constructor
SPIRVSMod(SPIRVType *TheType, SPIRVId TheId, SPIRVId TheDividend,
SPIRVId TheDivisor, SPIRVBasicBlock *BB)
: SPIRVFSMod(OC, TheType, TheId, TheDividend, TheDivisor, BB) {}
// Incomplete constructor
SPIRVSMod() : SPIRVFSMod(OC) {}
};

class SPIRVVectorTimesScalar : public SPIRVInstruction {
public:
static const Op OC = OpVectorTimesScalar;
Expand Down
66 changes: 0 additions & 66 deletions test/OpFMod.spt
View file

This file was deleted.

25 changes: 25 additions & 0 deletions test/OpFMod_f32.spvasm
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
; REQUIRES: spirv-as
; RUN: spirv-as --target-env spv1.0 -o %t.spv %s
; RUN: llvm-spirv -r -o - %t.spv | llvm-dis | FileCheck %s
OpCapability Addresses
OpCapability Kernel
OpMemoryModel Physical32 OpenCL
OpEntryPoint Kernel %1 "testFMod_f32"
OpName %a "a"
OpName %b "b"
%void = OpTypeVoid
%float = OpTypeFloat 32
%5 = OpTypeFunction %void %float %float
%1 = OpFunction %void None %5
%a = OpFunctionParameter %float
%b = OpFunctionParameter %float
%6 = OpLabel
%7 = OpFMod %float %a %b
OpReturn
OpFunctionEnd

; CHECK-DAG: %frem.res = frem float %a, %b
; CHECK-DAG: %copysign.res = call float @llvm.copysign.f32(float %frem.res, float %b)
; CHECK-DAG: %fadd.res = fadd float {{%frem\.res, %b|%b, %frem\.res}}
; CHECK-DAG: %cmp.res = fcmp one float {{%frem\.res, %copysign\.res|%copysign\.res, %frem\.res}}
; CHECK: select i1 %cmp.res, float %fadd.res, float %copysign.res
27 changes: 27 additions & 0 deletions test/OpFMod_v2f16.spvasm
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
; REQUIRES: spirv-as
; RUN: spirv-as --target-env spv1.0 -o %t.spv %s
; RUN: llvm-spirv -r -o - %t.spv | llvm-dis | FileCheck %s
OpCapability Addresses
OpCapability Kernel
OpCapability Float16
OpMemoryModel Physical32 OpenCL
OpEntryPoint Kernel %1 "testFMod_v2f16"
OpName %a "a"
OpName %b "b"
%void = OpTypeVoid
%half = OpTypeFloat 16
%half2 = OpTypeVector %half 2
%5 = OpTypeFunction %void %half2 %half2
%1 = OpFunction %void None %5
%a = OpFunctionParameter %half2
%b = OpFunctionParameter %half2
%6 = OpLabel
%7 = OpFMod %half2 %a %b
OpReturn
OpFunctionEnd

; CHECK-DAG: %frem.res = frem <2 x half> %a, %b
; CHECK-DAG: %copysign.res = call <2 x half> @llvm.copysign.v2f16(<2 x half> %frem.res, <2 x half> %b)
; CHECK-DAG: %fadd.res = fadd <2 x half> {{%frem\.res, %b|%b, %frem\.res}}
; CHECK-DAG: %cmp.res = fcmp one <2 x half> {{%frem\.res, %copysign\.res|%copysign\.res, %frem\.res}}
; CHECK: select <2 x i1> %cmp.res, <2 x half> %fadd.res, <2 x half> %copysign.res
27 changes: 27 additions & 0 deletions test/OpSMod_i32.spvasm
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
; REQUIRES: spirv-as
; RUN: spirv-as --target-env spv1.0 -o %t.spv %s
; RUN: llvm-spirv -r -o - %t.spv | llvm-dis | FileCheck %s
OpCapability Addresses
OpCapability Kernel
OpMemoryModel Physical32 OpenCL
OpEntryPoint Kernel %1 "testSMod_i32"
OpName %a "a"
OpName %b "b"
%void = OpTypeVoid
%uint = OpTypeInt 32 0
%5 = OpTypeFunction %void %uint %uint
%1 = OpFunction %void None %5
%a = OpFunctionParameter %uint
%b = OpFunctionParameter %uint
%6 = OpLabel
%7 = OpSMod %uint %a %b
OpReturn
OpFunctionEnd

; CHECK-DAG: %srem.res = srem i32 %a, %b
; CHECK-DAG: %xor.res = xor i32 {{%a, %b|%b, %a}}
; CHECK-DAG: %cmpsign.res = icmp slt i32 %xor.res, 0
; CHECK-DAG: %cmpsrem.res = icmp ne i32 {{%srem\.res, 0|0, %srem\.res}}
; CHECK-DAG: %add.res = add nsw i32 {{%srem\.res, %b|%b, %srem\.res}}
; CHECK-DAG: %cmp.res = and i1 {{%cmpsign\.res, %cmpsrem\.res|%cmpsrem\.res, %cmpsign\.res}}
; CHECK: select i1 %cmp.res, i32 %add.res, i32 %srem.res
29 changes: 29 additions & 0 deletions test/OpSMod_v2i16.spvasm
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
; REQUIRES: spirv-as
; RUN: spirv-as --target-env spv1.0 -o %t.spv %s
; RUN: llvm-spirv -r -o - %t.spv | llvm-dis | FileCheck %s
OpCapability Addresses
OpCapability Kernel
OpCapability Int16
OpMemoryModel Physical32 OpenCL
OpEntryPoint Kernel %1 "testSMod_v2i16"
OpName %a "a"
OpName %b "b"
%void = OpTypeVoid
%ushort = OpTypeInt 16 0
%ushort2 = OpTypeVector %ushort 2
%5 = OpTypeFunction %void %ushort2 %ushort2
%1 = OpFunction %void None %5
%a = OpFunctionParameter %ushort2
%b = OpFunctionParameter %ushort2
%6 = OpLabel
%7 = OpSMod %ushort2 %a %b
OpReturn
OpFunctionEnd

; CHECK-DAG: %srem.res = srem <2 x i16> %a, %b
; CHECK-DAG: %xor.res = xor <2 x i16> {{%a, %b|%b, %a}}
; CHECK-DAG: %cmpsign.res = icmp slt <2 x i16> %xor.res, zeroinitializer
; CHECK-DAG: %cmpsrem.res = icmp ne <2 x i16> {{%srem\.res, zeroinitializer|zeroinitializer, %srem\.res}}
; CHECK-DAG: %add.res = add nsw <2 x i16> {{%srem\.res, %b|%b, %srem\.res}}
; CHECK-DAG: %cmp.res = and <2 x i1> {{%cmpsign\.res, %cmpsrem\.res|%cmpsrem\.res, %cmpsign\.res}}
; CHECK: select <2 x i1> %cmp.res, <2 x i16> %add.res, <2 x i16> %srem.res

0 comments on commit 4de4935

Please sign in to comment.