[PATCH] D102824: [OpenMP] Internalize functions in OpenMPOpt to improve IPO passes

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

LGTM, two minor nits.



================
Comment at: llvm/include/llvm/Transforms/IPO/Attributor.h:1608
+  /// linkage can be internalized because these linkages guarantee that other
+  /// definitions with the same name have the same semantics as this one
+  ///
----------------
The comment is not accurate anymore. State what linkages are not internalized. Also say below it returns nullptr if it was not internalized.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:2572
+          !OMPInModule.getKernels().contains(&F))
+        Attributor::internalizeFunction(F, /* Force */ true);
+
----------------
I guess we can put the internalized version in the SCC if there was one and otherwise the original function. That way we already ignore what we probably delete later anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102824