[Openmp-commits] [PATCH] D95816: [OpenMP] libomp: minor changes to improve library performance

Johannes Doerfert via Phabricator via Openmp-commits openmp-commits at lists.llvm.org
Sun Feb 7 15:22:51 PST 2021


jdoerfert added a comment.

Granted, it's not inlining but partial inlining (the conditional from the 6 statement function).
I'm not happy about that either but I guess we can dot that. However, given that the
conditional is gone inside the function there is now an implicit calling invariant the user
has to know about. I don't use ITT but I feel this is not great. Two potential ways to
remedy this, neither is required to land the patch but they are simply suggestions:

1. Keep the conditional in the function. It will introduce minimal cost in ITT runs but none otherwise.
2. Create a wrapper in the header which encapsulates the `__kmp_itt_taskwait_object` call with the conditional exposed. This way we don't need to manually duplicate it in three locations.

ITT folks should accept the patch, no blocking objections from my side.

---

I agree, the CMake LTO stuff is unrelated and can be tackled independently.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95816



More information about the Openmp-commits mailing list