[PATCH] D75153: [ThinLTO] Allow usage of all SMT threads in the system

[Alexandre Ganea via Phabricator via llvm-commits]

aganea marked 3 inline comments as done.
aganea added a subscriber: respindola.
aganea added a comment.

In D75153#1906580 <https://reviews.llvm.org/D75153#1906580>, @MaskRay wrote:

> Does `taskset -c 0-3 lld -flavor ...` restrict the number of cores?
>
>   cpu_set_t cpu;
>   sched_getaffinity(0, sizeof(cpu), &cpu);
>   CPU_COUNT(&cpu)


Thanks for raising this! This does not seem to work (I currently only have WSL at hand, no real Linux machine). I don't think it worked before my patch. The current code in LLVM is written such as: (//note the "if" statement//)

  #if defined(HAVE_SCHED_GETAFFINITY) && defined(HAVE_CPU_COUNT)
    cpu_set_t Set;
    if (sched_getaffinity(0, sizeof(Set), &Set))
      return CPU_COUNT(&Set);
  #endif

The doc <https://linux.die.net/man/2/sched_getaffinity> for `sched_getaffinity` says:

> On success, sched_setaffinity() and sched_getaffinity() return 0. On error, -1 is returned, and errno is set appropriately.

So it would always fall back to `std::thread::hardware_concurrency`, which apparently does not always take affinity into account, according to @respindola (please see rG8c0ff9508da5f02e8ce6580a126a2018c9bf702a <https://reviews.llvm.org/rG8c0ff9508da5f02e8ce6580a126a2018c9bf702a>).

I'll write a follow-up patch to test affinity on Linux and Windows.



================
Comment at: llvm/include/llvm/Support/Threading.h:201
+  /// hardware core is used.
+  inline ThreadPoolStrategy heavyweight_hardware_concurrency(StringRef Num) {
+    Optional<ThreadPoolStrategy> S =
----------------
abrachet wrote:
> Nit: Remove `inline` https://llvm.org/docs/CodingStandards.html#don-t-use-inline-when-defining-a-function-in-a-class-definition
After discussing offling with @abrachet , I'll leave the `inline` for now. It makes the symbol weak, removing `inline` would otherwise fail linking. I can move the function(s) to the .CPP after this patch to save on link time.


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

https://reviews.llvm.org/D75153