[llvm] [TypeProf][PGO]Skip vtable-based ICP for which type profiles are known to be unrepresentative (PR #110575)

Mingming Liu via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 1 12:25:25 PDT 2024


================
@@ -2060,6 +2062,24 @@ bool parser<float>::parse(Option &O, StringRef ArgName, StringRef Arg,
   return false;
 }
 
+// parser<DenseSet<StringRef> implementation
+//
+void parser<DenseSet<StringRef>>::printOptionDiff(
+    const Option &O, const DenseSet<StringRef> &V,
+    OptionValue<DenseSet<StringRef>> D, size_t GlobalWidth) const {}
+
+bool parser<DenseSet<StringRef>>::parse(Option &O, StringRef ArgName,
+                                        StringRef Arg,
----------------
minglotus-6 wrote:

> Are you certain that the contents of StringRef Arg outlives the DenseSet returned? All the others above return the contents by value.

I took another look (at a couple of callsites of `cl::ParseCommandLineOptions`), and couldn't easily figure out if referenced strings remain alive for all passes. [FrontendOptions.h](https://github.com/llvm/llvm-project/blob/32ffc9fdc2cd422c88c926b862adb3de726e3888/clang/include/clang/Frontend/FrontendOptions.h#L560-L562) and [cc1as_main.cpp](https://github.com/llvm/llvm-project/blob/32ffc9fdc2cd422c88c926b862adb3de726e3888/clang/tools/driver/cc1as_main.cpp#L115) are two places which holds `vector<std::string>` for LLVM args. But IMO if `cl::ParseCommandLineOptions` (as a frequently-called function in LLVM and downstream repositories) doesn't require caller keep the argument contents alive, it's not a good idea for `cl::opt` to even use StringRef (users will have a hard time).

I updated to `cl::list<std::string>` to save a call to `llvm::SplitStrings`. And one interesting finding is that both `cl::opt` and `cl::list` supports internal or external storage [1], with internal storage as the default.

[1] https://llvm.org/docs/CommandLine.html#the-cl-list-class

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


More information about the llvm-commits mailing list