[PATCH] D142905: [Driver] Change multilib selection algorithm

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

I have just one comment related to efficiency, but if it turns out to be too difficult to address, I'd be also fine landing this change as is and addressing that issue in a follow up change.



================
Comment at: clang/lib/Driver/MultilibBuilder.cpp:94-100
+  Multilib::option_list PrintOptions;
+  for (StringRef Flag : Flags) {
+    if (Flag.front() == '+')
+      PrintOptions.push_back(("-" + Flag.substr(1)).str());
+  }
+  return Multilib(GCCSuffix, OSSuffix, IncludeSuffix,
+                  Multilib::flag_set(Flags.begin(), Flags.end()), PrintOptions);
----------------
I think we're optimizing for the wrong case here since `PrintOptions` is only needed for `-print-multi-lib`, which is infrequently used flag, but we always have pay the cost of constructing and keeping `PrintOptions` in memory, even when unused (which is going to vast majority of `Driver` uses). I think that constructing it lazily in `getPrintOptions()` (and using that in `print()`) would be more efficient.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142905/new/

https://reviews.llvm.org/D142905