Allowing to combine canonicalization and saturation

Considering mix mode operations for flushing denorms to zero

Author: krystian-andrzejewski

IGC/Compiler/CISACodeGen/EmitVISAPass.cpp

@@ -8836,10 +8836,6 @@ void EmitPass::EmitIntrinsicMessage(llvm::IntrinsicInst* inst)
         emitSqrt(inst);
         break;
 
-    case Intrinsic::canonicalize:
-        emitCanonicalize(inst);
-        break;
-
     default:
         inst->print(IGC::Debug::ods());
         IGC_ASSERT_MESSAGE(0, "unknown intrinsic");
@@ -16849,7 +16845,7 @@ void EmitPass::emitFrc(llvm::GenIntrinsicInst* inst)
     m_encoder->Frc(m_destination, src0);
 }
 
-void IGC::EmitPass::emitCanonicalize(llvm::Instruction* inst)
+void IGC::EmitPass::emitCanonicalize(llvm::Instruction* inst, const DstModifier& modifier)
 {
     // Force to flush denormal fp value to zero. Select one of two possible solutions:
     // 1. add inputVal, -0.0
@@ -16861,10 +16857,11 @@ void IGC::EmitPass::emitCanonicalize(llvm::Instruction* inst)
     bool flushVal = pCodeGenContext->m_floatDenormMode16 == ::IGC::FLOAT_DENORM_FLUSH_TO_ZERO && inst->getType()->isHalfTy();
     flushVal = flushVal || (pCodeGenContext->m_floatDenormMode32 == ::IGC::FLOAT_DENORM_FLUSH_TO_ZERO && inst->getType()->isFloatTy());
     flushVal = flushVal || (pCodeGenContext->m_floatDenormMode64 == ::IGC::FLOAT_DENORM_FLUSH_TO_ZERO && inst->getType()->isDoubleTy());
-    if (flushVal)
+    if (flushVal || modifier.sat)
     {
         CVariable* inputVal = GetSymbol(inst->getOperand(0));
         CVariable* negativeZero = m_currShader->GetScalarConstant(llvm::ConstantFP::get(inst->getType(), -0.0));
+        m_encoder->SetDstModifier(modifier);
         m_encoder->Add(m_destination, inputVal, negativeZero);
     }
 }

IGC/Compiler/CISACodeGen/EmitVISAPass.hpp

@@ -416,7 +416,7 @@ class EmitPass : public llvm::FunctionPass
     void emitAddPair(CVariable* Dst, CVariable* Src0, CVariable* Src1);
 
     void emitSqrt(llvm::Instruction* inst);
-    void emitCanonicalize(llvm::Instruction* inst);
+    void emitCanonicalize(llvm::Instruction* inst, const DstModifier& modifier);
     void emitRsq(llvm::Instruction* inst);
     void emitFrc(llvm::GenIntrinsicInst* inst);
 

IGC/Compiler/CISACodeGen/PatternMatchPass.cpp

@@ -954,7 +954,7 @@ namespace IGC
         switch (I.getOpcode())
         {
         case Instruction::FSub:
-            match = MatchFloor(I) || 
+            match = MatchFloor(I) ||
                 MatchFrc(I) ||
                 MatchLrp(I) ||
                 MatchPredAdd(I) ||
@@ -999,7 +999,7 @@ namespace IGC
                 MatchModifier(I);
             break;
         case Instruction::FAdd:
-            match = 
+            match =
                 MatchLrp(I) ||
                 MatchPredAdd(I) ||
                 MatchMad(I) ||
@@ -1220,6 +1220,9 @@ namespace IGC
         case Intrinsic::fshr:
             match = MatchFunnelShiftRotate(I);
             break;
+        case Intrinsic::canonicalize:
+            match = MatchCanonicalizeInstruction(I);
+            break;
         default:
             match = MatchSingleInstruction(I);
             // no pattern for the rest of the intrinsics
@@ -1626,9 +1629,9 @@ namespace IGC
         }
         return found;
     }
-    
+
     /*
-    below pass handles x - frac(x) = floor(x) pattern. Refer below : 
+    below pass handles x - frac(x) = floor(x) pattern. Refer below :
 
     frc (8|M0) r20.0<1>:f r19.0<8;8,1>:f {Compacted, @1}
     add (8|M0) (ge)f0.1 r19.0<1>:f r19.0<8;8,1>:f -r20.0<8;8,1>:f {@1}
@@ -3101,43 +3104,120 @@ namespace IGC
     {
         struct CanonicalizeInstPattern : Pattern
         {
-            CanonicalizeInstPattern(llvm::Instruction* pInst, bool isNeeded) : m_pInst(pInst), m_IsNeeded(isNeeded) {}
+            CanonicalizeInstPattern(llvm::Instruction* pInst) : m_pInst(pInst) {}
 
             llvm::Instruction* m_pInst;
-            bool m_IsNeeded;
+            Pattern* m_pPattern = nullptr;
 
             virtual void Emit(EmitPass* pass, const DstModifier& modifier)
             {
-                IGC_ASSERT(modifier.sat == false && modifier.flag == nullptr);
-                if (m_IsNeeded)
+                if (m_pPattern)
                 {
-                    pass->emitCanonicalize(m_pInst);
+                    m_pPattern->Emit(pass, modifier);
+                }
+                else
+                {
+                    pass->emitCanonicalize(m_pInst, modifier);
                 }
             }
         };
 
-        IGC_ASSERT(I.getNumOperands() == 1);
-        bool isNeeded = true;
 
         // FAdd, FSub, FMul, FDiv instructions flush subnormals to zero.
         // However, mix mode and math instructions preserve subnormals.
         // Other instructions also preserve subnormals.
-        if (llvm::BinaryOperator * pBianaryOperator = llvm::dyn_cast<llvm::BinaryOperator>(I.getOperand(0)))
+        // FSat intrinsic instruction can be emitted i.e. as FAdd so such an
+        // instruction should be inspected recursively.
+        std::function<bool(llvm::Value*)> DetermineIfMixMode;
+        DetermineIfMixMode = [&DetermineIfMixMode, this](llvm::Value* operand) -> bool
         {
-            switch (pBianaryOperator->getOpcode())
+            bool isMixModePossible = false;
+            if (m_Platform.supportMixMode())
             {
-            case llvm::BinaryOperator::BinaryOps::FAdd:
-            case llvm::BinaryOperator::BinaryOps::FMul:
-            case llvm::BinaryOperator::BinaryOps::FSub:
-            case llvm::BinaryOperator::BinaryOps::FDiv:
-                isNeeded = false;
-            default:
-                break;
+                if (llvm::BinaryOperator* pBianaryOperator = llvm::dyn_cast<llvm::BinaryOperator>(operand))
+                {
+                    // the switch instruction is executed to break the recursion if it is unneeded.
+                    // The cause for this recursion is a possibility of constructing mad instructions.
+                    switch (pBianaryOperator->getOpcode())
+                    {
+                    case llvm::BinaryOperator::BinaryOps::FAdd:
+                    case llvm::BinaryOperator::BinaryOps::FMul:
+                    case llvm::BinaryOperator::BinaryOps::FSub:
+                        isMixModePossible = pBianaryOperator->getType()->isDoubleTy() == false &&
+                            (DetermineIfMixMode(pBianaryOperator->getOperand(0)) || DetermineIfMixMode(pBianaryOperator->getOperand(1)));
+                        break;
+                    default:
+                        break;
+                    }
+                }
+                else if (isa<FPTruncInst>(operand))
+                {
+                    FPTruncInst* fptruncInst = llvm::cast<FPTruncInst>(operand);
+                    isMixModePossible = fptruncInst->getSrcTy()->isDoubleTy() == false;
+                }
+                else if (isa<FPExtInst>(operand))
+                {
+                    FPExtInst* fpextInst = llvm::cast<FPExtInst>(operand);
+                    isMixModePossible = fpextInst->getDestTy()->isDoubleTy() == false;
+                }
             }
-        }
+            return isMixModePossible;
+        };
 
-        CanonicalizeInstPattern* pattern = new (m_allocator) CanonicalizeInstPattern(&I, isNeeded);
-        MarkAsSource(I.getOperand(0));
+        std::function<bool(llvm::Value*)> DetermineIfNeeded;
+        DetermineIfNeeded = [&DetermineIfNeeded, &DetermineIfMixMode](llvm::Value* operand) -> bool
+        {
+            bool isNeeded = true;
+            if (llvm::BinaryOperator* pBianaryOperator = llvm::dyn_cast<llvm::BinaryOperator>(operand))
+            {
+                // the switch instruction is to consider only the operations
+                // which support flushing denorms to zero.
+                switch (pBianaryOperator->getOpcode())
+                {
+                case llvm::BinaryOperator::BinaryOps::FAdd:
+                case llvm::BinaryOperator::BinaryOps::FMul:
+                case llvm::BinaryOperator::BinaryOps::FSub:
+                case llvm::BinaryOperator::BinaryOps::FDiv:
+                    isNeeded = DetermineIfMixMode(pBianaryOperator);
+                    break;
+                default:
+                    break;
+                }
+            }
+            else if(GenIntrinsicInst* intrin = dyn_cast<GenIntrinsicInst>(operand))
+            {
+                switch (intrin->getIntrinsicID())
+                {
+                case GenISAIntrinsic::GenISA_fsat:
+                    isNeeded = DetermineIfNeeded(intrin->getOperand(0));
+                    break;
+                default:
+                    break;
+                }
+            }
+            else if (IntrinsicInst* intrin = dyn_cast<IntrinsicInst>(operand))
+            {
+                switch (intrin->getIntrinsicID())
+                {
+                case Intrinsic::canonicalize:
+                    isNeeded = DetermineIfNeeded(intrin->getOperand(0));
+                    break;
+                default:
+                    break;
+                }
+            }
+            return isNeeded;
+        };
+
+        CanonicalizeInstPattern* pattern = new (m_allocator) CanonicalizeInstPattern(&I);
+        if (DetermineIfNeeded(I.getOperand(0)))
+        {
+            MarkAsSource(I.getOperand(0));
+        }
+        else
+        {
+            pattern->m_pPattern = Match(*llvm::cast<llvm::Instruction>(I.getOperand(0)));
+        }
 
         AddPattern(pattern);
         return true;