Skip to content

Commit

Permalink
Fix signed zero issues of FRem
Browse files Browse the repository at this point in the history
We use IR builder CreateFRem to translate OpFRem to LLVM native frem
instruction. The instruction is further lowered by backend following
such formula on our HW:

  frem(x, y) = x - y * trunc(x/y)

There is a latent issue when x=-0.0. Although SPIR-V spec says nothing
about the case when x = 0.0, C fmod function does specify the sign of
the result is the same as that of the dividend even if it is signed zero.
But when we input x=-0.0 to above formula, we finally get the addition of
(-0.0) + 0.0. The result is +0.0 returned by HW. Hence, we have to
manually check this special case when nsz fast math flag is not specified.
  • Loading branch information
amdrexu committed Nov 10, 2023
1 parent 82ef966 commit 5bb8d93
Show file tree
Hide file tree
Showing 7 changed files with 59 additions and 6 deletions.
25 changes: 23 additions & 2 deletions lgc/builder/ArithBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -236,8 +236,7 @@ Value *BuilderImpl::CreateSMod(Value *dividend, Value *divisor, const Twine &ins
}

// =====================================================================================================================
// Create FP modulo operation, where the sign of the result (if not zero) is the same as the sign
// of the divisor.
// Create FP modulo operation, where the sign of the result is the same as the sign of the divisor.
//
// @param dividend : Dividend value
// @param divisor : Divisor value
Expand All @@ -248,6 +247,28 @@ Value *BuilderImpl::CreateFMod(Value *dividend, Value *divisor, const Twine &ins
return CreateFSub(dividend, CreateFMul(divisor, floor), instName);
}

// =====================================================================================================================
// Create FP modulo operation, where the sign of the result is the same as the sign of the dividend.
//
// @param dividend : Dividend value
// @param divisor : Divisor value
// @param instName : Name to give instruction(s)
Value *BuilderImpl::CreateFRem(Value *dividend, Value *divisor, const Twine &instName) {
Value *result = IRBuilder::CreateFRem(dividend, divisor);
if (!getFastMathFlags().noSignedZeros()) {
// NOTE: If the fast math flags might have signed zeros, we should check the special case when dividend is signed
// zero. Although SPIR-V spec says nothing about the case when x = 0.0, C fmod function does specify the sign of
// the result is the same as that of the dividend even if it is signed zero. We lower FRem to this: frem(x, y) =
// x - y * trunc(x/y) on our HW. And when x=-0.0, we get an addition of (-0.0) + 0.0. HW returns +0.0 rather
// than -0.0, which is not consistent with the expectation.
Value *isNegZero = createIsFPClass(dividend, CmpClass::NegativeZero);
result = CreateSelect(isNegZero, dividend, result);
}

result->setName(instName);
return result;
}

// =====================================================================================================================
// Create scalar/vector float/half fused multiply-and-add, to compute a * b + c
//
Expand Down
16 changes: 14 additions & 2 deletions lgc/builder/BuilderRecorder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ StringRef BuilderRecorder::getCallName(BuilderOpcode opcode) {
return "smod";
case BuilderOpcode::FMod:
return "fmod";
case BuilderOpcode::FRem:
return "frem";
case BuilderOpcode::Fma:
return "fma";
case BuilderOpcode::Tan:
Expand Down Expand Up @@ -848,8 +850,7 @@ Value *Builder::CreateSMod(Value *dividend, Value *divisor, const Twine &instNam
}

// =====================================================================================================================
// Create FP modulo operation, where the sign of the result (if not zero) is the same as the sign
// of the divisor.
// Create FP modulo operation, where the sign of the result is the same as the sign of the divisor.
//
// @param dividend : Dividend value
// @param divisor : Divisor value
Expand All @@ -858,6 +859,16 @@ Value *Builder::CreateFMod(Value *dividend, Value *divisor, const Twine &instNam
return record(BuilderOpcode::FMod, dividend->getType(), {dividend, divisor}, instName);
}

// =====================================================================================================================
// Create FP modulo operation, where the sign of the result is the same as the sign of the dividend.
//
// @param dividend : Dividend value
// @param divisor : Divisor value
// @param instName : Name to give instruction(s)
Value *Builder::CreateFRem(Value *dividend, Value *divisor, const Twine &instName) {
return record(BuilderOpcode::FRem, dividend->getType(), {dividend, divisor}, instName);
}

// =====================================================================================================================
// Create scalar/vector float/half fused multiply-and-add, to compute a * b + c
//
Expand Down Expand Up @@ -2025,6 +2036,7 @@ Instruction *Builder::record(BuilderOpcode opcode, Type *resultTy, ArrayRef<Valu
case BuilderOpcode::FMin3:
case BuilderOpcode::FMix:
case BuilderOpcode::FMod:
case BuilderOpcode::FRem:
case BuilderOpcode::FSign:
case BuilderOpcode::FaceForward:
case BuilderOpcode::FindSMsb:
Expand Down
1 change: 1 addition & 0 deletions lgc/builder/BuilderRecorder.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ enum BuilderOpcode : unsigned {
QuantizeToFp16,
SMod,
FMod,
FRem,
Fma,
Tan,
ASin,
Expand Down
4 changes: 4 additions & 0 deletions lgc/builder/BuilderReplayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,10 @@ Value *BuilderReplayer::processCall(unsigned opcode, CallInst *call) {
return m_builder->CreateFMod(args[0], args[1]);
}

case BuilderOpcode::FRem: {
return m_builder->CreateFRem(args[0], args[1]);
}

case BuilderOpcode::Fma: {
return m_builder->CreateFma(args[0], args[1], args[2]);
}
Expand Down
1 change: 1 addition & 0 deletions lgc/include/lgc/builder/BuilderImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ class BuilderImpl : public BuilderDefs {
// Create signed integer or FP modulo operation.
llvm::Value *CreateSMod(llvm::Value *dividend, llvm::Value *divisor, const llvm::Twine &instName = "");
llvm::Value *CreateFMod(llvm::Value *dividend, llvm::Value *divisor, const llvm::Twine &instName = "");
llvm::Value *CreateFRem(llvm::Value *dividend, llvm::Value *divisor, const llvm::Twine &instName = "");

// Create scalar/vector float/half fused multiply-and-add, to compute a * b + c
llvm::Value *CreateFma(llvm::Value *a, llvm::Value *b, llvm::Value *c, const llvm::Twine &instName = "");
Expand Down
12 changes: 10 additions & 2 deletions lgc/interface/lgc/Builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -485,14 +485,22 @@ class Builder : public BuilderDefs {
// @param instName : Name to give instruction(s)
llvm::Value *CreateSMod(llvm::Value *dividend, llvm::Value *divisor, const llvm::Twine &instName = "");

// Create FP modulo operation, where the sign of the result (if not zero) is the same as
// the sign of the divisor. The result is undefined if divisor is zero.
// Create FP modulo operation, where the sign of the result is the same as the sign of the divisor. The result
// is undefined if divisor is zero.
//
// @param dividend : Dividend value
// @param divisor : Divisor value
// @param instName : Name to give instruction(s)
llvm::Value *CreateFMod(llvm::Value *dividend, llvm::Value *divisor, const llvm::Twine &instName = "");

// Create FP modulo operation, where the sign of the result is the same as the sign of the dividend. The result
// is undefined if divisor is zero.
//
// @param dividend : Dividend value
// @param divisor : Divisor value
// @param instName : Name to give instruction(s)
llvm::Value *CreateFRem(llvm::Value *dividend, llvm::Value *divisor, const llvm::Twine &instName = "");

// Create scalar/vector float/half fused multiply-and-add, to compute a * b + c
//
// @param a : One value to multiply
Expand Down
6 changes: 6 additions & 0 deletions llpc/translator/lib/SPIRV/SPIRVReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5367,6 +5367,12 @@ Value *SPIRVToLLVM::transValueWithoutDecoration(SPIRVValue *bv, Function *f, Bas
Value *val1 = transValue(bc->getDivisor(), f, bb);
return mapValue(bc, getBuilder()->CreateFMod(val0, val1));
}
case OpFRem: {
SPIRVBinary *bc = static_cast<SPIRVBinary *>(bv);
Value *val0 = transValue(bc->getOperand(0), f, bb);
Value *val1 = transValue(bc->getOperand(1), f, bb);
return mapValue(bc, getBuilder()->CreateFRem(val0, val1));
}
case OpFNegate: {
SPIRVUnary *bc = static_cast<SPIRVUnary *>(bv);
Value *val0 = transValue(bc->getOperand(0), f, bb);
Expand Down

0 comments on commit 5bb8d93

Please sign in to comment.