[llvm] [DirectX] Propagate shader flags mask of callees to callers (PR #118306)
S. Bharadwaj Yadavalli via llvm-commits
llvm-commits at lists.llvm.org
Wed Dec 4 09:33:40 PST 2024
================
@@ -81,18 +101,37 @@ void ComputedShaderFlags::print(raw_ostream &OS) const {
OS << ";\n";
}
+static bool compareShaderFlagsInfo(
+ const std::pair<const Function *, ComputedShaderFlags> FSM,
+ const Function *FindFunc) {
+ return (FSM.first < FindFunc);
+}
+
/// Return the shader flags mask of the specified function Func.
const ComputedShaderFlags &
ModuleShaderFlags::getFunctionFlags(const Function *Func) const {
- const auto Iter = llvm::lower_bound(
- FunctionFlags, Func,
- [](const std::pair<const Function *, ComputedShaderFlags> FSM,
- const Function *FindFunc) { return (FSM.first < FindFunc); });
+ const std::pair<const Function *, ComputedShaderFlags> *Iter =
+ llvm::lower_bound(FunctionFlags, Func, compareShaderFlagsInfo);
----------------
bharadwajy wrote:
> We shouldn't be storing pairs sorted by pointer and doing searches on a vector. A DenseMap will be algorithmically more efficient and is the correct data structure for the use case.
Collection of shader flags in the analysis pass and subsequent queries in later passes follows the "distinct pattern" outlined in [LLVM Programmer's Manual](https://llvm.org/docs/ProgrammersManual.html#a-sorted-vector). Hence the choice of `SmallVector` as opposed to other `Map`-like containers such as `DenseMap`. Other considerations (including those related to memory usage) listed in [DenseMap](https://llvm.org/docs/ProgrammersManual.html#llvm-adt-densemap-h) - with seemingly higher cost for desired functionality - also influenced the choice of `SmallVector`. As for the sort order, since the `Function` pointers are unique per compilation invocation and just need to be sorted in some order during such an invocation, `compareShaderFlagsInfo` explicitly compares function pointers (although default comparator of `std::pair` does the same).
That said, I am open to any strong recommendation to change it to `DenseMap` for reasons not considered above.
https://github.com/llvm/llvm-project/pull/118306
More information about the llvm-commits
mailing list