From 9703887a16eff4a0967d8147036d3a440dc4ccb5 Mon Sep 17 00:00:00 2001
From: Rex Xu <rex.xu@amd.com>
Date: Tue, 31 Oct 2023 15:14:59 +0800
Subject: [PATCH] Fix issues of Modf

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)
---
 llpc/translator/lib/SPIRV/SPIRVReader.cpp | 43 ++++++++++++++---------
 1 file changed, 27 insertions(+), 16 deletions(-)

diff --git a/llpc/translator/lib/SPIRV/SPIRVReader.cpp b/llpc/translator/lib/SPIRV/SPIRVReader.cpp
index 2a604639c8..410b8083ba 100644
--- a/llpc/translator/lib/SPIRV/SPIRVReader.cpp
+++ b/llpc/translator/lib/SPIRV/SPIRVReader.cpp
@@ -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
@@ -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);