[PATCH] D90909: [OpenMPOpt][WIP] Expand parallel region merging

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:613
+                              {OMPRTL___kmpc_push_proc_bind, true},
+                              {OMPRTL___kmpc_push_num_threads, true}};
 
----------------
ggeorgakoudis wrote:
> jdoerfert wrote:
> > Don't we have to proof the absence of these calls and not "look for them". I mean `foobar` is not one of them but could call one of them, right?
> Yup, you're absolutely right. I was short sighted. We either have to check that there is no callpath from an in-between instruction to one of those functions and abort merging the affected regions, or entirely bail out from merging when there is any one of them declared, as we were doing before. I probably need to include omp_proc_bind and look for others too.
> 
> Also it got me thinking. What if there's a kpmc_fork_call inside a function call in the sequential region? Can a fork call be called within a master sequential region? Need to find out more.
You cannot have "any" OpenMP runtime calls in the guarded parts. One easy way is to disallow any call. Next step is to add an Attributor abstract attribute (as part of OpenMPOpt) which will deduce if a function might call an OpenMP runtime function, e.g., if it never calls an external function we don't know it cannot call an OpenMP runtime function. You will also be able to use the `omp assume` attribtues that will be introduced shortly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90909