[llvm] [DirectX] Propagate shader flags mask of callees to callers (PR #118306)

Justin Bogner via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 8 20:11:31 PST 2025


bogner wrote:

> > A few more comments on the algorithm. Also we should probably have a test that has an SCC of size greater than one.
> 
> Call Graph of code in `propagate-function-flags-test.ll` has an SCC with size 3. Let me know if you had some other test scenario in mind. Thanks!

Ah, that's true, but it doesn't seem to be a very good test case. Consider:
```diff
diff --git a/llvm/lib/Target/DirectX/DXILShaderFlags.cpp b/llvm/lib/Target/DirectX/DXILShaderFlags.cpp
index 0392d3c84735..01bf300a7f02 100644
--- a/llvm/lib/Target/DirectX/DXILShaderFlags.cpp
+++ b/llvm/lib/Target/DirectX/DXILShaderFlags.cpp
@@ -97,19 +97,12 @@ void ModuleShaderFlags::initialize(Module &M) {
       // Update combined shader flags mask for all functions in this SCC
       SCCSF.merge(CSF);

-      CurSCCFuncs.push_back(F);
+      FunctionFlags[F].merge(SCCSF);
     }

     // Update combined shader flags mask for all functions of the module
     CombinedSFMask.merge(SCCSF);

-    // Shader flags mask of each of the functions in an SCC of the call graph is
-    // the union of all functions in the SCC. Update shader flags masks of
-    // functions in CurSCC accordingly. This is trivially true if SCC contains
-    // one function.
-    for (Function *F : CurSCCFuncs)
-      // Merge SCCSF with that of F
-      FunctionFlags[F].merge(SCCSF);
   }
 }
```

With this, `propagate-function-flags-test.ll` still passes (perhaps by coincidence). So it doesn't seem to be a very good test of how we're handling SCCs with more than one function.

https://github.com/llvm/llvm-project/pull/118306


More information about the llvm-commits mailing list