Ignoring construct field warnings on delegatory methods (shader-slang…

…#4911)

* Ignoring construct field warnings on delegatory methods

* Generalizing instruction usage type interface

* Skip collection when searching for stores

* Adding separate construct delegation tests

* Treating differentiable functions as stores

source/slang/slang-ir-use-uninitialized-values.cpp

@@ -214,8 +214,16 @@ namespace Slang
 
         return addresses;
     }
-
-    static void checkCallUsage(List<IRInst*>& stores, List<IRInst*>& loads, IRCall* call, IRInst* inst)
+
+    enum InstructionUsageType
+    {
+        None,           // Instruction neither stores nor loads from the soruce (e.g. meta operations)
+        Store,          // Instruction acts as a write to the source
+        StoreParent,    // Instruction's parent acts as a write to the source
+        Load            // Instruciton acts as a load from the source
+    };
+
+    static InstructionUsageType getCallUsageType(IRCall* call, IRInst* inst)
     {
         IRInst* callee = call->getCallee();
 
@@ -224,10 +232,13 @@ namespace Slang
         IRFuncType* ftype = nullptr;
         if (auto spec = as<IRSpecialize>(callee))
             ftn = as<IRFunc>(resolveSpecialization(spec));
-        else if (auto fwd = as<IRForwardDifferentiate>(callee))
-            ftn = as<IRFunc>(fwd->getBaseFn());
-        else if (auto rev = as<IRBackwardDifferentiate>(callee))
-            ftn = as<IRFunc>(rev->getBaseFn());
+
+        // Differentiable functions are mostly ignored, treated as having inout parameters
+        else if (as<IRForwardDifferentiate>(callee))
+            return Store;
+        else if (as<IRBackwardDifferentiate>(callee))
+            return Store;
+
         else if (auto wit = as<IRLookupWitnessMethod>(callee))
             ftype = as<IRFuncType>(wit->getFullType());
         else
@@ -250,79 +261,82 @@ namespace Slang
             ftype = as<IRFuncType>(ftn->getFullType());
 
         if (!ftype)
-            return;
+            return None;
 
         // Consider it as a store if its passed
         // as an out/inout/ref parameter
         IRType* type = ftype->getParamType(index);
-        if (as<IROutType>(type) || as<IRInOutType>(type) || as<IRRefType>(type))
-            stores.add(call);
-        else
-            loads.add(call);
-    }
-
-    static void collectSpecialCaseInstructions(List<IRInst*>& stores, IRBlock* block)
-    {
-        for (auto inst = block->getFirstInst(); inst; inst = inst->next)
-        {
-            if (as<IRGenericAsm>(inst))
-                stores.add(inst);
-        }
+        return (as<IROutType>(type) || as<IRInOutType>(type) || as<IRRefType>(type)) ? Store : Load;
     }
 
-    static void collectLoadStore(List<IRInst*>& stores, List<IRInst*>& loads, IRInst* user, IRInst* inst)
+    static InstructionUsageType getInstructionUsageType(IRInst* user, IRInst* inst)
     {
         // Meta intrinsics (which evaluate on type) do nothing
         if (isMetaOp(user))
-            return;
+            return None;
 
         // Ignore instructions generating more aliases
         if (isAliasable(user))
-            return;
+            return None;
 
         switch (user->getOp())
         {
         case kIROp_loop:
         case kIROp_unconditionalBranch:
             // TODO: Ignore branches for now
-            return;
+            return None;
 
         case kIROp_Call:
             // Function calls can be either
             // stores or loads depending on
             // whether the callee takes it
             // in as a out parameter or not
-            return checkCallUsage(stores, loads, as<IRCall>(user), inst);
+            return getCallUsageType(as<IRCall>(user), inst);
 
         // These instructions will store data...
         case kIROp_Store:
         case kIROp_SwizzledStore:
         case kIROp_SPIRVAsm:
-            stores.add(user);
-            break;
+            return Store;
 
         case kIROp_SPIRVAsmOperandInst:
             // For SPIRV asm instructions, need to check out the entire
             // block when doing reachability checks
-            stores.add(user->getParent());
-            break;
+            return StoreParent;
 
         case kIROp_MakeExistential:
         case kIROp_MakeExistentialWithRTTI:
             // For specializing generic structs
-            stores.add(user);
-            break;
+            return Store;
 
         // Miscellaenous cases
         case kIROp_ManagedPtrAttach:
         case kIROp_Unmodified:
-            stores.add(user);
-            break;
+            return Store;
 
         // ... and the rest will load/use them
         default:
-            loads.add(user);
-            break;
+            return Load;
+        }
+    }
+
+    static void collectSpecialCaseInstructions(List<IRInst*>& stores, IRBlock* block)
+    {
+        for (auto inst = block->getFirstInst(); inst; inst = inst->next)
+        {
+            if (as<IRGenericAsm>(inst))
+                stores.add(inst);
+        }
+    }
+
+    static void collectInstructionByUsage(List<IRInst*>& stores, List<IRInst*>& loads, IRInst* user, IRInst* inst)
+    {
+        InstructionUsageType usage = getInstructionUsageType(user, inst);
+        switch (usage)
+        {
+        case Load: return loads.add(user);
+        case Store: return stores.add(user);
+        case StoreParent: return stores.add(user->getParent());
         }
     }
 
@@ -349,10 +363,7 @@ namespace Slang
         {
             // TODO: Mark specific parts assigned to for partial initialization checks
             for (auto use = alias->firstUse; use; use = use->nextUse)
-            {
-                IRInst* user = use->getUser();
-                collectLoadStore(stores, loads, user, alias);
-            }
+                collectInstructionByUsage(stores, loads, use->getUser(), alias);
         }
     }
 
@@ -400,10 +411,7 @@ namespace Slang
         for (auto alias : getAliasableInstructions(inst))
         {
             for (auto use = alias->firstUse; use; use = use->nextUse)
-            {
-                IRInst* user = use->getUser();
-                collectLoadStore(stores, loads, user, alias);
-            }
+                collectInstructionByUsage(stores, loads, use->getUser(), alias);
         }
 
         for (auto store : stores)
@@ -434,13 +442,44 @@ namespace Slang
         return false;
     }
 
+    static bool isWrittenTo(IRInst* inst)
+    {
+        for (auto alias : getAliasableInstructions(inst))
+        {
+            for (auto use = alias->firstUse; use; use = use->nextUse)
+            {
+                InstructionUsageType usage = getInstructionUsageType(use->getUser(), alias);
+                if (usage == Store || usage == StoreParent)
+                    return true;
+            }
+        }
+
+        return false;
+    }
+
+    static bool isDirectlyWrittenTo(IRInst* inst)
+    {
+        for (auto use = inst->firstUse; use; use = use->nextUse)
+        {
+            InstructionUsageType usage = getInstructionUsageType(use->getUser(), inst);
+            if (usage == Store || usage == StoreParent)
+                return true;
+        }
+
+        return false;
+    }
+
     static List<IRStructField*> checkFieldsFromExit(ReachabilityContext& reachability, IRReturn* ret, IRStructType* type)
     {
         IRInst* origin = traceInstOrigin(ret->getVal());
 
         // We don't want to warn on delegated construction
         if (!isUninitializedValue(origin))
             return {};
+
+        // Check if the origin instruction is ever written to
+        if (isDirectlyWrittenTo(origin))
+            return {};
 
         // Now we can look for all references to fields
         HashSet<IRStructKey*> usedKeys;
@@ -545,21 +584,8 @@ namespace Slang
         }
 
         // If there is at least one write...
-        List<IRInst*> stores;
-        List<IRInst*> loads;
-
-        for (auto alias : getAliasableInstructions(param))
-        {
-            for (auto use = alias->firstUse; use; use = use->nextUse)
-            {
-                IRInst* user = use->getUser();
-                collectLoadStore(stores, loads, user, alias);
-
-                // ...we will ignore the rest...
-                if (stores.getCount())
-                    return;
-            }
-        }
+        if (isWrittenTo(param))
+            return;
 
         // ...or if there is an intrinsic_asm instruction
         for (const auto& b : func->getBlocks())
@@ -672,22 +698,17 @@ namespace Slang
 
         auto addresses = getAliasableInstructions(variable);
 
-        List<IRInst*> stores;
         List<IRInst*> loads;
-
         for (auto alias : addresses)
         {
             for (auto use = alias->firstUse; use; use = use->nextUse)
             {
-                IRInst* user = use->getUser();
-                collectLoadStore(stores, loads, user, alias);
-
-                // Disregard if there is at least one store,
-                // since we cannot tell what the control flow is
-                if (stores.getCount())
+                InstructionUsageType usage = getInstructionUsageType(use->getUser(), alias);
+                if (usage == Store || usage == StoreParent)
                     return;
 
-                // TODO: see if we can do better here (another kind of reachability check?)
+                if (usage == Load)
+                    loads.add(use->getUser());
             }
         }
 

tests/diagnostics/uninitialized-fields-delegated.slang

@@ -0,0 +1,46 @@
+//TEST:SIMPLE(filecheck=CHK): -target spirv
+
+// Delegated constructors
+struct Impl
+{
+    float x;
+
+    __init(float val)
+    {
+        x = val;
+    }
+
+    __init()
+    {
+        float val = 2.0;
+
+        // Shouldn't trigger a warning here
+        return Impl(val);
+    }
+}
+
+// Calling a method from a constructor to initialize fields
+struct HitInfo
+{
+    float3 barycentrics;
+    uint primitiveIndex;
+
+    [[mutating]] void init(float2 hitBarycentrics, uint hitPrimitiveIndex)
+    {
+        barycentrics = { 1.0 - hitBarycentrics.x - hitBarycentrics.y, hitBarycentrics.x, hitBarycentrics.y };
+        primitiveIndex = hitPrimitiveIndex;
+    }
+
+    __init(float2 hitBarycentrics, uint hitPrimitiveIndex)
+    {
+        init(hitBarycentrics, hitPrimitiveIndex);
+    }
+
+    __init(BuiltInTriangleIntersectionAttributes attr)
+    {
+        init(attr.barycentrics, PrimitiveIndex());
+    }
+}
+
+//CHK-NOT: warning 41020
+//CHK-NOT: warning 41021

tests/diagnostics/uninitialized-fields.slang

@@ -86,23 +86,5 @@ struct Pass
     int y = 0;
 }
 
-struct Impl
-{
-    float x;
-
-    __init(float val)
-    {
-        x = val;
-    }
-
-    __init()
-    {
-        float val = 2.0;
-
-        // Shouldn't trigger a warning here
-        return Impl(val);
-    }
-}
-
 //CHK-NOT: warning 41020
 //CHK-NOT: warning 41021