Skip to content
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.

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

Fix signed zero issues of FRem #2782

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
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. According to SPIR-V spec, the result of FRem must have the same sign of the dividend but we lower FRem to
// this: frem(x, y) = x - y * trunc(x/y). When x=-0.0, we get an addition of (-0.0) + 0.0. HW returns +0.0 rather
// than -0.0, which violates the spec expectation.
Value *zero = Constant::getNullValue(divisor->getType());
Value *isZero = CreateFCmpOEQ(dividend, zero);
result = canonicalize(CreateSelect(isZero, dividend, result)); // Ensure we flush denorms as expected
}

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 @@ -846,8 +848,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 @@ -856,6 +857,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 @@ -2012,6 +2023,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 @@ -155,6 +155,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 @@ -5361,6 +5361,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
Loading