[llvm] [ThinLTO] Shrink GlobalValueSummary by 8 bytes (PR #107342)

Teresa Johnson via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 5 09:51:20 PDT 2024


================
@@ -99,10 +99,11 @@ extern cl::opt<bool> MemProfReportHintedSizes;
 // can only take an address of basic block located in the same function.
 // Set `RefLocalLinkageIFunc` to true if the analyzed value references a
 // local-linkage ifunc.
-static bool findRefEdges(ModuleSummaryIndex &Index, const User *CurUser,
-                         SetVector<ValueInfo, std::vector<ValueInfo>> &RefEdges,
-                         SmallPtrSet<const User *, 8> &Visited,
-                         bool &RefLocalLinkageIFunc) {
+static bool
+findRefEdges(ModuleSummaryIndex &Index, const User *CurUser,
+             SetVector<ValueInfo, SmallVector<ValueInfo, 0>> &RefEdges,
----------------
teresajohnson wrote:

According to https://llvm.org/docs/ProgrammersManual.html#llvm-adt-smallvector-h (see the "Note" box):

```
Prefer to use ArrayRef<T> or SmallVectorImpl<T> as a parameter type.

It’s rarely appropriate to use SmallVector<T, N> as a parameter type. If an API only reads from the vector, it should use ArrayRef.
```

I see in some places you have used SmallVectorImpl for the parameter but others like here just SmallVector. These should presumably all be either SmallVectorImpl or ArrayRef.

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


More information about the llvm-commits mailing list