Skip to content

Commit

Permalink
Fix issues of Modf
Browse files Browse the repository at this point in the history
When x is -0.0, modf will return 0.0 according to this formula:

  modf(x) = x - trunc(x)

This leads to an addition of (-0.0) + 0.0 and our HW returns 0.0. According
to SPIR-V spec, the result has the same sign as that of the input. We must
check this special case by doing this:

  modf(x) = copysign(modf(x), x)

Also, according to C modf function, when x=+INF/-INF, the result is 0.0 but
we have such computation (-INF) + INF or INF - INF. Our HW returns NaN so we
must check this special case as well by do this:

  modf(x) = (x == INF) ? copysign(0.0, x) : modf(x)
  • Loading branch information
amdrexu committed Nov 3, 2023
1 parent feb8bd5 commit 9703887
Showing 1 changed file with 27 additions and 16 deletions.
43 changes: 27 additions & 16 deletions llpc/translator/lib/SPIRV/SPIRVReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8767,8 +8767,8 @@ Constant *SPIRVToLLVM::buildShaderBlockMetadata(SPIRVType *bt, ShaderBlockDecora
Value *SPIRVToLLVM::transGLSLExtInst(SPIRVExtInst *extInst, BasicBlock *bb) {
auto bArgs = extInst->getArguments();
auto args = transValue(extInst->getValues(bArgs), bb->getParent(), bb);
switch (static_cast<GLSLExtOpKind>(extInst->getExtOp())) {

auto extOp = static_cast<GLSLExtOpKind>(extInst->getExtOp());
switch (extOp) {
case GLSLstd450Round:
case GLSLstd450RoundEven:
// Round to whole number
Expand Down Expand Up @@ -8903,25 +8903,36 @@ Value *SPIRVToLLVM::transGLSLExtInst(SPIRVExtInst *extInst, BasicBlock *bb) {
// Inverse of square matrix
return getBuilder()->CreateMatrixInverse(args[0]);

case GLSLstd450Modf: {
// Split input into fractional and whole number parts.
case GLSLstd450Modf:
case GLSLstd450ModfStruct: {
// Split input into fractional and whole number parts:
// i = trunc(x), y = modf(x) = x - trunc(x) = x - i
Value *wholeNum = getBuilder()->CreateUnaryIntrinsic(Intrinsic::trunc, args[0]);
Value *fract = getBuilder()->CreateFSub(args[0], wholeNum);
// Vectors are represented as arrays in memory, so we need to cast the pointer of array to pointer of vector before
// storing.
if (wholeNum->getType()->isVectorTy()) {

if (!getBuilder()->getFastMathFlags().noInfs()) {
// NOTE: There is an issue when x=+INF/-INF. According to C modf function, the result of Modf is
// 0.0 when x=+INF/-INF. But when we input x=+INF/-INF to above formula, we finally get the computation
// of (-INF) + INF or INF - INF. The result is NaN returned by HW. Hence, we have to manually check this
// special case when NoInfs fast math flag is not specified.
Value *isInf = getBuilder()->CreateIsInf(args[0]);
fract = getBuilder()->CreateSelect(isInf, ConstantFP::getNullValue(args[0]->getType()), fract);
}

if (!getBuilder()->getFastMathFlags().noSignedZeros() || !getBuilder()->getFastMathFlags().noInfs()) {
// NOTE: There is an issue when x=-0.0. According to SPIR-V spec, the sign of the result of Modf is the
// same as the sign of the input. 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.
fract = getBuilder()->CreateCopySign(fract, args[0]);
}

if (extOp == GLSLstd450Modf) {
assert(args[1]->getType()->isPointerTy());
Type *const castType = wholeNum->getType()->getPointerTo(args[1]->getType()->getPointerAddressSpace());
args[1] = getBuilder()->CreateBitCast(args[1], castType);
getBuilder()->CreateStore(wholeNum, args[1]);
return fract;
}
getBuilder()->CreateStore(wholeNum, args[1]);
return fract;
}

case GLSLstd450ModfStruct: {
// Split input into fractional and whole number parts.
Value *wholeNum = getBuilder()->CreateUnaryIntrinsic(Intrinsic::trunc, args[0]);
Value *fract = getBuilder()->CreateFSub(args[0], wholeNum);
Value *result = PoisonValue::get(transType(extInst->getType()));
result = getBuilder()->CreateInsertValue(result, fract, 0);
result = getBuilder()->CreateInsertValue(result, wholeNum, 1);
Expand Down

0 comments on commit 9703887

Please sign in to comment.