Fix function side-effectness prop logic. (#2875)

1 files changed, 218 insertions, 75 deletions

diff --git a/source/slang/slang-ir-propagate-func-properties.cpp b/source/slang/slang-ir-propagate-func-properties.cpp
index 7ce4bfc80..7b29aaf14 100644
--- a/source/slang/slang-ir-propagate-func-properties.cpp
+++ b/source/slang/slang-ir-propagate-func-properties.cpp
@@ -7,7 +7,112 @@
 
 namespace Slang
 {
-bool propagateFuncProperties(IRModule* module)
+class FuncPropertyPropagationContext
+{
+public:
+    virtual bool canProcess(IRFunc* f) = 0;
+    virtual bool propagate(IRBuilder& builder, IRFunc* func) = 0;
+};
+
+class ReadNoneFuncPropertyPropagationContext : public FuncPropertyPropagationContext
+{
+public:
+    virtual bool canProcess(IRFunc* f) override
+    {
+        // If the func has already been marked with any decorations, skip.
+        for (auto decoration : f->getDecorations())
+        {
+            switch (decoration->getOp())
+            {
+            case kIROp_ReadNoneDecoration:
+            case kIROp_TargetIntrinsicDecoration:
+                return false;
+            }
+        }
+        return true;
+    }
+
+    virtual bool propagate(IRBuilder& builder, IRFunc* f) override
+    {
+        bool hasSideEffectCall = false;
+        for (auto block : f->getBlocks())
+        {
+            for (auto inst : block->getChildren())
+            {
+                // Is this inst known to not have global side effect/analyzable?
+                if (inst->mightHaveSideEffects())
+                {
+                    switch (inst->getOp())
+                    {
+                    case kIROp_ifElse:
+                    case kIROp_unconditionalBranch:
+                    case kIROp_Switch:
+                    case kIROp_Return:
+                    case kIROp_loop:
+                    case kIROp_Call:
+                    case kIROp_Param:
+                    case kIROp_Unreachable:
+                    case kIROp_Store:
+                    case kIROp_SwizzledStore:
+                        break;
+                    default:
+                        // We have a inst that has side effect and is not understood by this method.
+                        // e.g. bufferStore, discard, etc.
+                        hasSideEffectCall = true;
+                        break;
+                    }
+                }
+
+                if (auto call = as<IRCall>(inst))
+                {
+                    auto callee = getResolvedInstForDecorations(call->getCallee());
+                    switch (callee->getOp())
+                    {
+                    default:
+                        // We are calling an unknown function, so we have to assume
+                        // there are side effects in the call.
+                        hasSideEffectCall = true;
+                        break;
+                    case kIROp_Func:
+                        if (!callee->findDecoration<IRReadNoneDecoration>())
+                        {
+                            hasSideEffectCall = true;
+                            break;
+                        }
+                    }
+                }
+
+                // Do any operands defined have pointer type of global or
+                // unknown source? Passing them into a readNone callee may cause
+                // side effects that breaks the readNone property.
+                for (UInt o = 0; o < inst->getOperandCount(); o++)
+                {
+                    auto operand = inst->getOperand(o);
+                    if (as<IRConstant>(operand))
+                        continue;
+                    if (as<IRType>(operand))
+                        continue;
+                    if (isGlobalOrUnknownMutableAddress(f, operand))
+                    {
+                        hasSideEffectCall = true;
+                        break;
+                    }
+                    break;
+                }
+            }
+            if (hasSideEffectCall)
+                break;
+        }
+        if (!hasSideEffectCall)
+        {
+            builder.addDecoration(f, kIROp_ReadNoneDecoration);
+            return true;
+        }
+        return false;
+    }
+};
+
+bool propagateFuncPropertiesImpl(IRModule* module, FuncPropertyPropagationContext* context)
 {
     bool result = false;
     List<IRFunc*> workList;
@@ -61,7 +166,7 @@ bool propagateFuncProperties(IRModule* module)
             }
             if (auto func = as<IRFunc>(inst))
             {
-                if (func->findDecoration<IRReadNoneDecoration>())
+                if (context->canProcess(func))
                 {
                     addCallersToWorkList(func);
                 }
@@ -87,101 +192,139 @@ bool propagateFuncProperties(IRModule* module)
         for (Index i = 0; i < workList.getCount(); i++)
         {
             auto f = workList[i];
-            bool hasSideEffectCall = false;
-            if (f->findDecoration<IRReadNoneDecoration>())
+            if (!context->canProcess(f))
                 continue;
+
             // Never propagate to functions without a body.
             if (f->getFirstBlock() == nullptr)
                 continue;
-            if (f->findDecoration<IRTargetIntrinsicDecoration>())
-                continue;
-            for (auto block : f->getBlocks())
+
+            if (context->propagate(builder, f))
+            {
+                addCallersToWorkList(f);
+                changed = true;
+            }
+        }
+        result |= changed;
+        if (!changed)
+            break;
+    }
+    return result;
+}
+
+class NoSideEffectFuncPropertyPropagationContext : public FuncPropertyPropagationContext
+{
+public:
+    virtual bool canProcess(IRFunc* f) override
+    {
+        // If the func has already been marked with any decorations, skip.
+        for (auto decoration : f->getDecorations())
+        {
+            switch (decoration->getOp())
             {
-                for (auto inst : block->getChildren())
+            case kIROp_ReadNoneDecoration:
+            case kIROp_NoSideEffectDecoration:
+            case kIROp_TargetIntrinsicDecoration:
+                return false;
+            }
+        }
+        return true;
+    }
+
+    virtual bool propagate(IRBuilder& builder, IRFunc* f) override
+    {
+        bool hasSideEffectCall = false;
+        for (auto block : f->getBlocks())
+        {
+            for (auto inst : block->getChildren())
+            {
+                // Is this inst known to not have global side effect/analyzable?
+                if (inst->mightHaveSideEffects())
                 {
-                    // Is this inst known to not have global side effect/analyzable?
-                    if (inst->mightHaveSideEffects())
+                    switch (inst->getOp())
                     {
-                        switch (inst->getOp())
-                        {
-                        case kIROp_ifElse:
-                        case kIROp_unconditionalBranch:
-                        case kIROp_Switch:
-                        case kIROp_Return:
-                        case kIROp_loop:
-                        case kIROp_Store:
-                        case kIROp_Call:
-                        case kIROp_Param:
-                        case kIROp_Unreachable:
-                            break;
-                        default:
-                            // We have a inst that has side effect and is not understood by this method.
-                            // e.g. bufferStore, discard, etc.
-                            hasSideEffectCall = true;
-                            break;
-                        }
+                    case kIROp_ifElse:
+                    case kIROp_unconditionalBranch:
+                    case kIROp_Switch:
+                    case kIROp_Return:
+                    case kIROp_loop:
+                    case kIROp_Call:
+                    case kIROp_Param:
+                    case kIROp_Unreachable:
+                    case kIROp_Store:
+                    case kIROp_SwizzledStore:
+                        break;
+                    default:
+                        // We have a inst that has side effect and is not understood by this method.
+                        // e.g. bufferStore, discard, etc.
+                        hasSideEffectCall = true;
+                        break;
                     }
+                }
+                else
+                {
+                    // A side effect free inst can't generate side effects for the function.
+                    continue;
+                }
 
-                    if (auto call = as<IRCall>(inst))
+                if (auto call = as<IRCall>(inst))
+                {
+                    auto callee = getResolvedInstForDecorations(call->getCallee());
+                    switch (callee->getOp())
                     {
-                        auto callee = getResolvedInstForDecorations(call->getCallee());
-                        switch (callee->getOp())
+                    default:
+                        // We are calling an unknown function, so we have to assume
+                        // there are side effects in the call.
+                        hasSideEffectCall = true;
+                        break;
+                    case kIROp_Func:
+                        if (!callee->findDecoration<IRReadNoneDecoration>() &&
+                            !callee->findDecoration<IRNoSideEffectDecoration>())
                         {
-                        default:
-                            // We are calling an unknown function, so we have to assume
-                            // there are side effects in the call.
                             hasSideEffectCall = true;
                             break;
-                        case kIROp_Func:
-                            if (!callee->findDecoration<IRReadNoneDecoration>())
-                            {
-                                hasSideEffectCall = true;
-                                break;
-                            }
                         }
                     }
-                    
-                    // Are any operands defined in global scope?
-                    for (UInt o = 0; o < inst->getOperandCount(); o++)
+                }
+
+                // Do any operands defined have pointer type of global or
+                // unknown source? Passing them into a NoSideEffect callee may cause
+                // side effects that breaks the NoSideEffect property.
+                for (UInt o = 0; o < inst->getOperandCount(); o++)
+                {
+                    auto operand = inst->getOperand(o);
+                    if (as<IRConstant>(operand))
+                        continue;
+                    if (as<IRType>(operand))
+                        continue;
+                    if (isGlobalOrUnknownMutableAddress(f, operand))
                     {
-                        auto operand = inst->getOperand(o);
-                        if (getParentFunc(operand) == f)
-                            continue;
-                        if (as<IRConstant>(operand))
-                            continue;
-                        if (as<IRType>(operand))
-                            continue;
-                        switch (operand->getOp())
-                        {
-                        case kIROp_Specialize:
-                        case kIROp_LookupWitness:
-                        case kIROp_StructKey:
-                        case kIROp_WitnessTable:
-                        case kIROp_WitnessTableEntry:
-                        case kIROp_undefined:
-                        case kIROp_Func:
-                            continue;
-                        default:
-                            break;
-                        }
                         hasSideEffectCall = true;
                         break;
                     }
-                }
-                if (hasSideEffectCall)
                     break;
+                }
             }
-            if (!hasSideEffectCall)
-            {
-                builder.addDecoration(f, kIROp_ReadNoneDecoration);
-                addCallersToWorkList(f);
-                changed = true;
-            }
+            if (hasSideEffectCall)
+                break;
         }
-        result |= changed;
-        if (!changed)
-            break;
+        if (!hasSideEffectCall)
+        {
+            builder.addDecoration(f, kIROp_NoSideEffectDecoration);
+            return true;
+        }
+        return false;
     }
-    return result;
+};
+
+bool propagateFuncProperties(IRModule* module)
+{
+    ReadNoneFuncPropertyPropagationContext readNoneContext;
+    bool changed = propagateFuncPropertiesImpl(module, &readNoneContext);
+
+    NoSideEffectFuncPropertyPropagationContext noSideEffectContext;
+    changed|= propagateFuncPropertiesImpl(module, &noSideEffectContext);
+
+    return changed;
 }
 }