[PATCH] D72304: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

jdoerfert added a comment. In D72304#1877855 , @fhahn wrote: > It looks like this patch (or the other related changes committed recently) > may cause ASan failures running `Transforms/OpenMP/gtid.ll ` and > `Transforms/OpenMP/parallel_deletion.ll` >

[PATCH] D72304: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

fhahn added a comment. It looks like this patch (or the other related changes committed recently) may cause ASan failures running `Transforms/OpenMP/gtid.ll ` and `Transforms/OpenMP/parallel_deletion.ll` http://green.lab.llvm.org/green/job/clang-stage2-cmake-RgSan/6960/consoleFull It would be

[PATCH] D72304: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

jdoerfert added a comment. In D72304#1877676 , @MaskRay wrote: > `test/CodeGen/opt-record-1.c` triggers an assertion failure. I hope the macro reordering in rGa0236de7a927 did the

[PATCH] D72304: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

jdoerfert added a comment. I did run clang-format on the OMPIRBuilder files and replaced tabs with spaces. I think you need to teach your editor to use spaces in the future. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72304/new/

[PATCH] D72304: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

MaskRay added a comment. `test/CodeGen/opt-record-1.c` triggers an assertion failure. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72304/new/ https://reviews.llvm.org/D72304 ___ cfe-commits mailing

[PATCH] D72304: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

This revision was automatically updated to reflect the committed changes. Closed by commit rG7438059a9032: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder. (authored by fghanim, committed by jdoerfert). Changed prior to commit:

[PATCH] D72304: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

fghanim updated this revision to Diff 244249. fghanim added a comment. Remove unnecessary preservation of builder insertion point in `emitCommonDirectiveExit` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72304/new/

[PATCH] D72304: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

fghanim updated this revision to Diff 243839. fghanim added a comment. fixing unused variable. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72304/new/ https://reviews.llvm.org/D72304 Files: clang/lib/CodeGen/CGStmtOpenMP.cpp

[PATCH] D72304: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

kadircet added a comment. this breaks under asan http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/38597/steps/check-clang%20asan/logs/stdio Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72304/new/

[PATCH] D72304: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

This revision was automatically updated to reflect the committed changes. Closed by commit rGe8a436c5ea26: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder. (authored by fady f...@lap118789.ornl.gov, committed by jdoerfert). Changed prior to commit:

[PATCH] D72304: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

fghanim added a comment. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72304/new/ https://reviews.llvm.org/D72304 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D72304: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

fghanim updated this revision to Diff 242144. fghanim added a comment. Addressing errors/warnings from commit build Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72304/new/ https://reviews.llvm.org/D72304 Files:

[PATCH] D72304: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

jdoerfert reopened this revision. jdoerfert added a comment. This revision is now accepted and ready to land. Reverted in 9dcfc7cd64abb301124cafaa95661b76a1fc5032 because of breakages and warnings:

[PATCH] D72304: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

This revision was automatically updated to reflect the committed changes. Closed by commit rG1ca740387b9b: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder. (authored by fghanim, committed by jdoerfert). Changed prior to commit:

[PATCH] D72304: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

fghanim added a comment. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72304/new/ https://reviews.llvm.org/D72304 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D72304: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

fghanim added a comment. Everything is fine! I just cloned llvm from git hub, added this revision with `arc patch`. then: `cmake -G"Unix Makefiles" -DLLVM_ENABLE_PROJECTS="clang;openmp" -DCMAKE_BUILD_TYPE=Release -DLLVM_OPTIMIZED_TABLEGEN=1 -DLLVM_USE_LINKER=gold ../llvm-project/llvm` `make` ,

[PATCH] D72304: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

jdoerfert added a comment. In D72304#1846115 , @fghanim wrote: > Thanks for trying. > > It compiles fine with me. How can I reproduce the problems you're getting? is > there a way to pack what you have and send it to me so I can have a look? Did you

[PATCH] D72304: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

fghanim added a comment. Thanks for trying. It compiles fine with me. How can I reproduce the problems you're getting? is there a way to pack what you have and send it to me so I can have a look? Also, a second question, how trustworthy is harbormaster when it says something is buildable?

[PATCH] D72304: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

jdoerfert added a comment. I tried to push this today but the build failed: /data/src/llvm-project/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:252:36: error: use of undeclared identifier 'BumpPtrAllocator' StringMap, BumpPtrAllocator> InternalVars;

[PATCH] D72304: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

fghanim added a comment. No, I don't have commit privileges. I'd appreciate if you'd commit this for me. Thanks :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72304/new/ https://reviews.llvm.org/D72304

[PATCH] D72304: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

fghanim updated this revision to Diff 238858. fghanim added a comment. Addressing reviewer's comments regarding styling. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72304/new/ https://reviews.llvm.org/D72304 Files:

[PATCH] D72304: [OpenMP]{OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

jdoerfert added a comment. The commit title has a stray `{` instead of `[` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72304/new/ https://reviews.llvm.org/D72304 ___ cfe-commits mailing list

[PATCH] D72304: [OpenMP]{OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

jdoerfert added a comment. Phab is not involved in the merge process, it just is a normal git push on top of the llmv-project. Do you have commit rights already? If not I can commit this for you in a few days. Comment at: llvm/lib/Frontend/OpenMP/OMPConstants.cpp:45 ///{ -

[PATCH] D72304: [OpenMP]{OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

fghanim marked an inline comment as done and an inline comment as not done. fghanim added a comment. Thanks for reviewing this. After I address the last two comments below, How do I merge with llvm using phab? I'd appreciate an llvm specific link if possible. Comment at:

[PATCH] D72304: [OpenMP]{OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

fghanim updated this revision to Diff 238591. fghanim marked 9 inline comments as done. fghanim added a comment. Addressing reviewer's comments - fixed styling and naming according to llvm conventions Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D72304: [OpenMP]{OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LGTM with some minor things you should address before the merge. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPConstants.h:90 +// extern ArrayType

[PATCH] D72304: [OpenMP]{OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

fghanim updated this revision to Diff 238301. fghanim marked 25 inline comments as done. fghanim added a comment. Addressing reviewer comments - Added regression tests - styling - adding asserts and todo where needed. Also, Now `EmitInlinedRegion` will merge blocks where legal. Repository:

[PATCH] D72304: [OpenMP]{OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

fghanim marked 4 inline comments as done. fghanim added a comment. I am working on a patch. In the mean time ;) Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:3020 + if (AllocaIP.isSet()) +AllocaInsertPt = &*AllocaIP.getPoint(); + auto OldReturnBlock =