[PATCH] D146557: [MLIR][OpenMP] Refactoring createTargetData in OMPIRBuilder

2023-06-19 Thread Akash Banerjee via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGa032dc139dda: [MLIR][OpenMP] Refactoring createTargetData in OMPIRBuilder (authored by TIFitis). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146557/new/ h

[PATCH] D146557: [MLIR][OpenMP] Refactoring createTargetData in OMPIRBuilder

2023-06-16 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146557/new/ https://reviews.llvm.org/D146557 ___

[PATCH] D146557: [MLIR][OpenMP] Refactoring createTargetData in OMPIRBuilder

2023-06-15 Thread Akash Banerjee via Phabricator via cfe-commits
TIFitis added inline comments. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:1487 + // possible, or else at the end of the function. + void emitBlock(BasicBlock *BB, Function *CurFn, bool IsFinished = false); + jdoerfert wrote: > jdoerfert wrote:

[PATCH] D146557: [MLIR][OpenMP] Refactoring createTargetData in OMPIRBuilder

2023-06-15 Thread Akash Banerjee via Phabricator via cfe-commits
TIFitis updated this revision to Diff 531760. TIFitis marked 3 inline comments as done. TIFitis added a comment. Addressed reviewer comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146557/new/ https://reviews.llvm.org/D146557 Files: llvm/i

[PATCH] D146557: [MLIR][OpenMP] Refactoring createTargetData in OMPIRBuilder

2023-06-14 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert added a comment. I think this is now style wise pretty good, I still found some potential problems below. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:1487 + // possible, or else at the end of the function. + void emitBlock(BasicBlock *BB, Function

[PATCH] D146557: [MLIR][OpenMP] Refactoring createTargetData in OMPIRBuilder

2023-06-12 Thread Akash Banerjee via Phabricator via cfe-commits
TIFitis added inline comments. Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:4741 + if (IsFinished && BB->use_empty()) { +delete BB; +return; jdoerfert wrote: > you should not just delete BB. eraseFromParent is a better way. That said, it > is a

[PATCH] D146557: [MLIR][OpenMP] Refactoring createTargetData in OMPIRBuilder

2023-06-12 Thread Akash Banerjee via Phabricator via cfe-commits
TIFitis updated this revision to Diff 530680. TIFitis added a comment. Update MLIR BodyGen callback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146557/new/ https://reviews.llvm.org/D146557 Files: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder

[PATCH] D146557: [MLIR][OpenMP] Refactoring createTargetData in OMPIRBuilder

2023-06-12 Thread Akash Banerjee via Phabricator via cfe-commits
TIFitis updated this revision to Diff 530671. TIFitis marked 6 inline comments as done. TIFitis added a comment. Addressed reviewer comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146557/new/ https://reviews.llvm.org/D146557 Files: llvm/

[PATCH] D146557: [MLIR][OpenMP] Refactoring createTargetData in OMPIRBuilder

2023-06-12 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert added inline comments. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:1487 + // possible, or else at the end of the function. + void emitBlock(BasicBlock *BB, Function *CurFn, bool IsFinished = false); + This does not mention the deletio

[PATCH] D146557: [MLIR][OpenMP] Refactoring createTargetData in OMPIRBuilder

2023-06-12 Thread Akash Banerjee via Phabricator via cfe-commits
TIFitis added inline comments. Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:4727 + Builder.ClearInsertionPoint(); +} + jdoerfert wrote: > This function doesn't make sense to me. For one, I don't know what a "unreal > block" is. Nor would I have expecte

[PATCH] D146557: [MLIR][OpenMP] Refactoring createTargetData in OMPIRBuilder

2023-06-12 Thread Akash Banerjee via Phabricator via cfe-commits
TIFitis updated this revision to Diff 530518. TIFitis marked 5 inline comments as done and an inline comment as not done. TIFitis added a comment. Addressed reviewer comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146557/new/ https://review

[PATCH] D146557: [MLIR][OpenMP] Refactoring createTargetData in OMPIRBuilder

2023-06-09 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert added inline comments. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:1365 + // possible, or else at the end of the function. + void EmitBlock(BasicBlock *BB, Function *CurFn, bool IsFinished = false); + why are these capitalized?

[PATCH] D146557: [MLIR][OpenMP] Refactoring createTargetData in OMPIRBuilder

2023-05-22 Thread Akash Banerjee via Phabricator via cfe-commits
TIFitis updated this revision to Diff 524333. TIFitis added a comment. Renamed from processMapOp to genMapInfos Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146557/new/ https://reviews.llvm.org/D146557 Files: llvm/include/llvm/Frontend/OpenMP/O

[PATCH] D146557: [MLIR][OpenMP] Refactoring createTargetData in OMPIRBuilder

2023-05-22 Thread Akash Banerjee via Phabricator via cfe-commits
TIFitis updated this revision to Diff 524301. TIFitis added a comment. Moved map processing code from functor to new static function. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146557/new/ https://reviews.llvm.org/D146557 Files: llvm/include/

[PATCH] D146557: [MLIR][OpenMP] Refactoring createTargetData in OMPIRBuilder

2023-05-18 Thread Akash Banerjee via Phabricator via cfe-commits
TIFitis updated this revision to Diff 523331. TIFitis added a comment. Try to fix patch application. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146557/new/ https://reviews.llvm.org/D146557 Files: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder

[PATCH] D146557: [MLIR][OpenMP] Refactoring createTargetData in OMPIRBuilder

2023-05-18 Thread Akash Banerjee via Phabricator via cfe-commits
TIFitis updated this revision to Diff 523318. TIFitis added a comment. Fix lint messages Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146557/new/ https://reviews.llvm.org/D146557 Files: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h llvm/li

[PATCH] D146557: [MLIR][OpenMP] Refactoring createTargetData in OMPIRBuilder

2023-05-18 Thread Akash Banerjee via Phabricator via cfe-commits
TIFitis updated this revision to Diff 523316. TIFitis added a comment. Rebased Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146557/new/ https://reviews.llvm.org/D146557 Files: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h llvm/lib/Frontend

[PATCH] D146557: [MLIR][OpenMP] Refactoring createTargetData in OMPIRBuilder

2023-05-18 Thread Akash Banerjee via Phabricator via cfe-commits
TIFitis updated this revision to Diff 523304. TIFitis edited the summary of this revision. TIFitis added a comment. Small fix to code comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146557/new/ https://reviews.llvm.org/D146557 Files: llv