[PATCH] D140415: [flang] stack arrays pass

This revision was automatically updated to reflect the committed changes. Closed by commit rGcc14bf22bddf: [flang] add a pass to move array temporaries to the stack (authored by tblah). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140415/new/ https

[PATCH] D140415: [flang] stack arrays pass

jeanPerier accepted this revision. jeanPerier added a comment. I do not have any further comments, thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140415/new/ https://reviews.llvm.org/D140415 ___ c

[PATCH] D140415: [flang] stack arrays pass

tblah updated this revision to Diff 495073. tblah added a comment. Changes: inline mlir::blockIsInLoop Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140415/new/ https://reviews.llvm.org/D140415 Files: clang/docs/tools/clang-formatted-files.txt

[PATCH] D140415: [flang] stack arrays pass

kiranchandramohan accepted this revision. kiranchandramohan added a comment. This revision is now accepted and ready to land. LGTM. Please wait till end of day Monday before you submit to provide other reviewers with a chance to provide further comments or request changes. You can consider inlin

[PATCH] D140415: [flang] stack arrays pass

tblah added inline comments. Comment at: flang/lib/Optimizer/Transforms/StackArrays.cpp:732 + mlir::applyPartialConversion(func, target, std::move(patterns { +mlir::emitError(func->getLoc(), "error in stack arrays optimization\n"); +signalPassFailure(); -

[PATCH] D140415: [flang] stack arrays pass

tblah added a comment. In D140415#4098170 , @kiranchandramohan wrote: > Looks OK. I have a few questions and some minor comments inline. It might be > good to pull in a bit of info from the RFC into the Summary, particularly > saying why a dataflow ana

[PATCH] D140415: [flang] stack arrays pass

tblah updated this revision to Diff 494283. tblah marked 8 inline comments as done. tblah added a comment. Changes: fix nits from review Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140415/new/ https://reviews.llvm.org/D140415 Files: clang/docs

[PATCH] D140415: [flang] stack arrays pass

kiranchandramohan added a comment. Looks OK. I have a few questions and some minor comments inline. It might be good to pull in a bit of info from the RFC into the Summary, particularly saying why a dataflow analysis is necessary, what operations are involved in the analysis etc. Could we have

[PATCH] D140415: [flang] stack arrays pass

tblah added inline comments. Comment at: flang/lib/Optimizer/Transforms/StackArrays.cpp:433 + llvm::DenseSet freedValues; + func->walk([&](mlir::func::ReturnOp child) { +const LatticePoint *lattice = solver.lookupState(child); kiranchandramohan wrote: > Do

[PATCH] D140415: [flang] stack arrays pass

tblah updated this revision to Diff 493692. tblah marked 10 inline comments as done. tblah added a comment. Thanks for review. Changes: - Join the lattices at each return operation to ensure that values are freed at *all* returns, not only *some* return - Add tests with multiple return operatio

[PATCH] D140415: [flang] stack arrays pass

kiranchandramohan added a comment. Thanks for this patch @tblah. I had a first look. See comments inline. Have not gone through the core part in full yet. Comment at: flang/lib/Optimizer/Transforms/StackArrays.cpp:433 + llvm::DenseSet freedValues; + func->walk([&](mlir::func

[PATCH] D140415: [flang] stack arrays pass

tblah added a comment. In D140415#4080080 , @jdoerfert wrote: > Did you check LLVM's heap2stack and the corresponding tests? > https://github.com/llvm/llvm-project/blob/c68af565ff0c2fdc5537e9ac0c2d7c75df44b035/llvm/lib/Transforms/IPO/AttributorAttributes.

[PATCH] D140415: [flang] stack arrays pass

tblah added a comment. In D140415#4080080 , @jdoerfert wrote: > Thanks for taking a look, see my responses inline. For more information, the RFC is at https://reviews.llvm.org/D139617 > Quick questions, and they might not apply here since you seem to

[PATCH] D140415: [flang] stack arrays pass

jdoerfert added a comment. Quick questions, and they might not apply here since you seem to only look at explicit Flang generated values, right? Are you handling unwinding/exceptions, especially in-between the allocation and deallocation? Are you handling non-accessible stacks (e.g., on GPUs) fo

[PATCH] D140415: [flang] stack arrays pass

tblah added inline comments. Comment at: flang/lib/Optimizer/Transforms/StackArrays.cpp:352 + LLVM_DEBUG(llvm::dbgs() + << "--Allocation is not for an array: skipping\n"); +} jeanPerier wrote: > I think the early return may be missing her

[PATCH] D140415: [flang] stack arrays pass

tblah updated this revision to Diff 492044. tblah marked 5 inline comments as done. tblah added a comment. - Add missing early return for allocations not for arrays - Remove braces from if statement with a single statement in its body - Assert that a correct insertion point is found for the alloca

[PATCH] D140415: [flang] stack arrays pass

jeanPerier added a comment. Thanks for all the updates. This looks functionally correct to me. Since I am not very familiar with this kind of analysis and transformation, it would be great if another reviewer could give his/her opinion. But otherwise, given this solution is well isolated from a

[PATCH] D140415: [flang] stack arrays pass

tblah added a comment. Ping for review Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140415/new/ https://reviews.llvm.org/D140415 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llv

[PATCH] D140415: [flang] stack arrays pass

tblah updated this revision to Diff 490810. tblah added a comment. Fix newly added tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140415/new/ https://reviews.llvm.org/D140415 Files: clang/docs/tools/clang-formatted-files.txt flang/include

[PATCH] D140415: [flang] stack arrays pass

tblah added inline comments. Comment at: flang/lib/Optimizer/Transforms/StackArrays.cpp:120 + // which operations we intend to rewrite before it calls the pattern rewriter + llvm::SmallDenseMap insertionPoints; + jeanPerier wrote: > It is definitely weird to me

[PATCH] D140415: [flang] stack arrays pass

tblah updated this revision to Diff 489753. tblah marked 7 inline comments as done. tblah added a comment. - Implement operator!= as !(operator==) - Move insertion point computation to StackArraysAnalysisWrapper::analyseFunction - Remove special-casing for join(allocated, unknown) - Add processin

[PATCH] D140415: [flang] stack arrays pass

jeanPerier added inline comments. Herald added a subscriber: sunshaoce. Comment at: flang/lib/Optimizer/Transforms/StackArrays.cpp:104 + + bool operator!=(const InsertionPoint &rhs) const { +return (location != rhs.location) || It's better to negate the `==

[PATCH] D140415: [flang] stack arrays pass

tblah updated this revision to Diff 488914. tblah added a comment. Updating patch context Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140415/new/ https://reviews.llvm.org/D140415 Files: clang/docs/tools/clang-formatted-files.txt flang/includ

[PATCH] D140415: [flang] stack arrays pass

tblah updated this revision to Diff 488685. tblah added a comment. Herald added a reviewer: jdoerfert. Herald added subscribers: cfe-commits, sstefan1. Herald added projects: clang, Flang. - Do not move allocations outside of openmp regions - Detect loops in the control flow graph - Attempt to use