[PATCH] D91054: [Clang][OpenMP] Frontend work for sections - D89671

This revision was automatically updated to reflect the committed changes. Closed by commit rGc20410618827: [Clang][OpenMP] Frontend work for sections - D89671 (authored by AMDChirag). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91054/new/ https://

[PATCH] D91054: [Clang][OpenMP] Frontend work for sections - D89671

AMDChirag updated this revision to Diff 341503. AMDChirag added a comment. Rebase to retrigger build The build failed previously - seemingly with a test case that has nothing to do with this patch. Retriggering and Rebasing the commit to ensure the same. Repository: rG LLVM Github Monorepo

[PATCH] D91054: [Clang][OpenMP] Frontend work for sections - D89671

AMDChirag updated this revision to Diff 341436. AMDChirag added a comment. Rebased to HEAD Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91054/new/ https://reviews.llvm.org/D91054 Files: clang/lib/CodeGen/CGOpenMPRuntime.cpp clang/lib/CodeGen/

[PATCH] D91054: [Clang][OpenMP] Frontend work for sections - D89671

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. In D91054#2711654 , @fghanim wrote: >> This is generally fine with me, @fghanim @Meinersbur any concerns? > > I have none. All good for me Then L

[PATCH] D91054: [Clang][OpenMP] Frontend work for sections - D89671

Meinersbur added a comment. In D91054#2709712 , @jdoerfert wrote: > OK, we really need to provide the PrivCB impl so we can start removing clang > code. > This is generally fine with me, @fghanim @Meinersbur any concerns? It might be OK to start with th

[PATCH] D91054: [Clang][OpenMP] Frontend work for sections - D89671

fghanim added a comment. > This is generally fine with me, @fghanim @Meinersbur any concerns? I have none. All good for me Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91054/new/ https://reviews.llvm.org/D91054

[PATCH] D91054: [Clang][OpenMP] Frontend work for sections - D89671

jdoerfert added a comment. In D91054#2708924 , @AMDChirag wrote: > In D91054#2708873 , @jdoerfert wrote: > >> What is missing or not working in the IRBuilder impl that prevents us from >> turning it on by default?

[PATCH] D91054: [Clang][OpenMP] Frontend work for sections - D89671

AMDChirag added a comment. In D91054#2708873 , @jdoerfert wrote: > What is missing or not working in the IRBuilder impl that prevents us from > turning it on by default? Per the `TODO` in this patch (`CGStmtOpenMP.cpp - 3775`), `PrivatizationCallback` i

[PATCH] D91054: [Clang][OpenMP] Frontend work for sections - D89671

jdoerfert added a comment. What is missing or not working in the IRBuilder impl that prevents us from turning it on by default? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91054/new/ https://reviews.llvm.org/D91054 _

[PATCH] D91054: [Clang][OpenMP] Frontend work for sections - D89671

Meinersbur added a comment. thanks for the update Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91054/new/ https://reviews.llvm.org/D91054 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://

[PATCH] D91054: [Clang][OpenMP] Frontend work for sections - D89671

AMDChirag added a comment. For D89671 , Johannes had some comments which have been taken care of. I am just waiting for him to get back on it. I'll ping him there. This patch is done from my end unless someone has some comment/thought. Repository: rG LLVM Git

[PATCH] D91054: [Clang][OpenMP] Frontend work for sections - D89671

Meinersbur added a comment. What is the status of this? The parent D89671 has been accepted, but not committed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91054/new/ https://reviews.llvm.org/D91054 ___

[PATCH] D91054: [Clang][OpenMP] Frontend work for sections - D89671

AMDChirag added a comment. In D91054#2693514 , @fghanim wrote: > You can update the tests as long as long as the output is correct. for > example the difference is only in names, ordering of basicblocks and > instructions that doesn't affect correctness,

[PATCH] D91054: [Clang][OpenMP] Frontend work for sections - D89671

fghanim added a comment. You can update the tests as long as long as the output is correct. for example the difference is only in names, ordering of basicblocks and instructions that doesn't affect correctness, etc. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

[PATCH] D91054: [Clang][OpenMP] Frontend work for sections - D89671

AMDChirag updated this revision to Diff 337654. AMDChirag added a comment. Herald added a subscriber: jfb. Fixed cancel construct and applied updates to its test cases The test cases are updated so that the IR generated with and without IRBuilder are considered as they are different. The cancel

[PATCH] D91054: [Clang][OpenMP] Frontend work for sections - D89671

AMDChirag added inline comments. Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:3579 +SectionCBVector.push_back(SectionCB); + } +} else { jdoerfert wrote: > Why do we unpack the children here instead of making a single call back for > the Captur

[PATCH] D91054: [Clang][OpenMP] Frontend work for sections - D89671

jdoerfert added a comment. In D91054#2548266 , @AMDChirag wrote: > @fghanim @jdoerfert please review the code if/when possible. Let's resolve the issue described in the TODO before we go ahead with this. There is little point reviewing something that can

[PATCH] D91054: [Clang][OpenMP] Frontend work for sections - D89671

AMDChirag added a comment. @fghanim @jdoerfert please review the code if/when possible. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91054/new/ https://reviews.llvm.org/D91054 ___ cfe-commits mailing li

[PATCH] D91054: [Clang][OpenMP] Frontend work for sections - D89671

AMDChirag updated this revision to Diff 319095. AMDChirag added a comment. Added FIXME comment for cancellation construct not working with sections construct Also updated the lit test cases to reflect this change. The lit test case modification will be removed once this issue is resolved. Repos

[PATCH] D91054: [Clang][OpenMP] Frontend work for sections - D89671

AMDChirag updated this revision to Diff 315708. AMDChirag added a comment. Added OMP delegating code for `createSection` (`EmitOMPSectionDirective`). @fghanim working on clang lit test case. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91054/new/

[PATCH] D91054: [Clang][OpenMP] Frontend work for sections - D89671

fghanim added a comment. Ping. Please add the Lit test for this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91054/new/ https://reviews.llvm.org/D91054 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D91054: [Clang][OpenMP] Frontend work for sections - D89671

AMDChirag updated this revision to Diff 313307. AMDChirag added a comment. Updated `BGenCallbackTy` to `StorableBodyGenCallbackTy`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91054/new/ https://reviews.llvm.org/D91054 Files: clang/lib/CodeGen

[PATCH] D91054: [Clang][OpenMP] Frontend work for sections - D89671

AMDChirag updated this revision to Diff 313303. AMDChirag marked an inline comment as done. AMDChirag added a comment. Updated code according to the changes in LLVM side of things. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91054/new/ https://re

[PATCH] D91054: [Clang][OpenMP] Frontend work for sections - D89671

AMDChirag added inline comments. Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:3576 +} else { + auto SectionCB = [this, CS](InsertPointTy AllocaIP, + InsertPointTy CodeGenIP, Meinersbur wrote: > In what situation would t

[PATCH] D91054: [Clang][OpenMP] Frontend work for sections - D89671

AMDChirag updated this revision to Diff 307022. AMDChirag added a comment. Fixed usage of BodyGenCallbackTy Removed ArrayRef variable Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91054/new/ https://reviews.llvm.org/D91054 Files: clang/lib/CodeG

[PATCH] D91054: [Clang][OpenMP] Frontend work for sections - D89671

fghanim added a comment. Thanks for doing this. Please add proper lit test. you can find that under `clang/test/OpenMP/sections_codegen.cpp`. add a case for using the OMPBuilder. As an example look in the same directory for `parallel_codegen.cpp` , `master_codegen.cpp` , `critical_codegen.cpp`

[PATCH] D91054: [Clang][OpenMP] Frontend work for sections - D89671

Meinersbur added inline comments. Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:3552-3554 +using BodyGenCallbackTy = +llvm::function_ref; Instead of duplicating it here, why not using `OpenMPIRBuilder::BodyGenCallbackTy`? Comment

[PATCH] D91054: [Clang][OpenMP] Frontend work for sections - D89671

AMDChirag added a comment. The test case will also be added here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91054/new/ https://reviews.llvm.org/D91054 ___ cfe-commits mailing list cfe-commits@lists.l