[PATCH] D79677: [clang][OpenMP][OMPIRBuilder] Adding some Privatization clauses to OpenMP `parallel` Directive

Thu May 14 09:44:02 PDT 2020

fghanim marked 2 inline comments as done.
fghanim added a comment.

In D79677#2032942 <https://reviews.llvm.org/D79677#2032942>, @jdoerfert wrote:

> Generally you copied the existing Clang logic, correct?

Well, Yes and no. I tried to keep as much as I can of the original implementation, however, some required more extensive changes.

The things added to `emitparalleldirective` are all new
Almost the latter half of the OMPBuilder version of `emitfirstprivateclause`
somethings in the the OMPBuilder version of `emitcopyinclause`
The rest had minor or smaller changes, but generally the same
the lit tests had some changes

================
Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:803
-                                                                      OrigVD);
-        else
-          (void)CGM.getOpenMPRuntime().registerTargetFirstprivateCopy(*this,
----------------
jdoerfert wrote:
> Wasn't this part of D79675?
> 
> (btw I tried to comprehend why this is needed and it is on my list for things we replace eventually).
ignore - I originally wanted to use the original `emitfirstprivate`, before I had to make some changes. This is remaining from that code. The same comment / todo is in OMPBuilder specific version below.

I also, removed this from D79676

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79677