[PATCH] D72304: Summary: Add OpenMP Directives (master and critical) to OMPBuilder, and use them in clang.

jdoerfert added a comment. In D72304#1815793 , @fghanim wrote: > > Commits need a message that explains the change. > > Also the "Summary" should not be in the commit title. > > Is this something for future commits or do you want me to change these? >

[PATCH] D72304: Summary: Add OpenMP Directives (master and critical) to OMPBuilder, and use them in clang.

fghanim marked 10 inline comments as done. fghanim added a comment. > Commits need a message that explains the change. > Also the "Summary" should not be in the commit title. Is this something for future commits or do you want me to change these? If the latter, how do I do that? > This adds

[PATCH] D72304: Summary: Add OpenMP Directives (master and critical) to OMPBuilder, and use them in clang.

jdoerfert added a comment. I added quite a bunch of inline comments again :( and some high level stuff below. --- Commits need a message that explains the change. Also the "Summary" should not be in the commit title. --- This adds support for clang to build the directives via the

[PATCH] D72304: Summary: Add OpenMP Directives (master and critical) to OMPBuilder, and use them in clang.

fghanim marked 8 inline comments as done. fghanim added a comment. So I modified the patch based on your comments. I removed all the code that I intended for llvm pass writers for now. I will look at it again later, figure something to do with the whole switch-if-cascade thing, and will

[PATCH] D72304: Summary: Add OpenMP Directives (master and critical) to OMPBuilder, and use them in clang.

fghanim updated this revision to Diff 237149. fghanim added a comment. - Adding array types to OMPKinds.def. Inlining runtime function calls generation. reformatting with clang format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72304/new/

[PATCH] D72304: Summary: Add OpenMP Directives (master and critical) to OMPBuilder, and use them in clang.

jdoerfert added inline comments. Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:845 + default: + assert(false && "unknown OMP directive"); + While I hope this code goes away, I should mention that we don't write `assert(false &&

[PATCH] D72304: Summary: Add OpenMP Directives (master and critical) to OMPBuilder, and use them in clang.

jdoerfert added inline comments. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:250 + llvm::ArrayType *KmpCriticalNameTy; + llvm::PointerType *KmpCriticalNamePtrTy; + fghanim wrote: > jdoerfert wrote: > > If there is no good reason

[PATCH] D72304: Summary: Add OpenMP Directives (master and critical) to OMPBuilder, and use them in clang.

fghanim marked 5 inline comments as done. fghanim added a comment. I don't think there is enough in this patch to split it into two or three patches. The main part is `EmitOMPInlinedRegion` which does all the heavy lifting. At this point, both create `Master` & `Critical` are almost wrappers

[PATCH] D72304: Summary: Add OpenMP Directives (master and critical) to OMPBuilder, and use them in clang.

jdoerfert added a comment. Some initial comments. Can we split it in two patches (master/critical)? Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:250 + llvm::ArrayType *KmpCriticalNameTy; + llvm::PointerType *KmpCriticalNamePtrTy; +

[PATCH] D72304: Summary: Add OpenMP Directives (master and critical) to OMPBuilder, and use them in clang.

fghanim created this revision. fghanim added a reviewer: jdoerfert. Herald added subscribers: llvm-commits, cfe-commits, guansong, hiraditya. Herald added projects: clang, LLVM. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D72304 Files: clang/lib/CodeGen/CGStmtOpenMP.cpp