[Autobackout][FuncReg]Revert of change: 7aae34a

Allowing to combine canonicalization and saturation
Considering mix mode operations for flushing denorms to zero

Author: krystian-andrzejewski

IGC/Compiler/CISACodeGen/EmitVISAPass.cpp

@@ -8836,6 +8836,10 @@ 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");
@@ -16845,7 +16849,7 @@ void EmitPass::emitFrc(llvm::GenIntrinsicInst* inst)
     m_encoder->Frc(m_destination, src0);
 }
 
-void IGC::EmitPass::emitCanonicalize(llvm::Instruction* inst, const DstModifier& modifier)
+void IGC::EmitPass::emitCanonicalize(llvm::Instruction* inst)
 {
     // Force to flush denormal fp value to zero. Select one of two possible solutions:
     // 1. add inputVal, -0.0
@@ -16857,11 +16861,10 @@ void IGC::EmitPass::emitCanonicalize(llvm::Instruction* inst, const DstModifier&
     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 || modifier.sat)
+    if (flushVal)
     {
         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, const DstModifier& modifier);
+    void emitCanonicalize(llvm::Instruction* inst);
     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,9 +1220,6 @@ 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
@@ -1629,9 +1626,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}
@@ -3104,121 +3101,44 @@ namespace IGC
     {
         struct CanonicalizeInstPattern : Pattern
         {
-            CanonicalizeInstPattern(llvm::Instruction* pInst) : m_pInst(pInst) {}
+            CanonicalizeInstPattern(llvm::Instruction* pInst, bool isNeeded) : m_pInst(pInst), m_IsNeeded(isNeeded) {}
 
             llvm::Instruction* m_pInst;
-            Pattern* m_pPattern = nullptr;
+            bool m_IsNeeded;
 
             virtual void Emit(EmitPass* pass, const DstModifier& modifier)
             {
-                if (m_pPattern)
+                IGC_ASSERT(modifier.sat == false && modifier.flag == nullptr);
+                if (m_IsNeeded)
                 {
-                    m_pPattern->Emit(pass, modifier);
-                }
-                else
-                {
-                    pass->emitCanonicalize(m_pInst, modifier);
+                    pass->emitCanonicalize(m_pInst);
                 }
             }
         };
 
+        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.
-        // 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
+        if (llvm::BinaryOperator * pBianaryOperator = llvm::dyn_cast<llvm::BinaryOperator>(I.getOperand(0)))
         {
-            bool isMixModePossible = false;
-            if (m_Platform.supportMixMode())
+            switch (pBianaryOperator->getOpcode())
             {
-                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;
-        };
-
-        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;
-                }
+            case llvm::BinaryOperator::BinaryOps::FAdd:
+            case llvm::BinaryOperator::BinaryOps::FMul:
+            case llvm::BinaryOperator::BinaryOps::FSub:
+            case llvm::BinaryOperator::BinaryOps::FDiv:
+                isNeeded = false;
+            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)));
         }
 
+        CanonicalizeInstPattern* pattern = new (m_allocator) CanonicalizeInstPattern(&I, isNeeded);
+        MarkAsSource(I.getOperand(0));
+
         AddPattern(pattern);
         return true;
     }