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 code point of view and can be turned and
on/off easily, I'll be glad to approve it.
================
Comment at: flang/lib/Optimizer/Transforms/StackArrays.cpp:352
+ LLVM_DEBUG(llvm::dbgs()
+ << "--Allocation is not for an array: skipping\n");
+ }
----------------
I think the early return may be missing here.
================
Comment at: flang/lib/Optimizer/Transforms/StackArrays.cpp:446
+ candidateOps.insert({allocmem, insertionPoint});
+ }
+ }
----------------
nit: MLIR/LLVM coding style do not use `{}` for single line if.
================
Comment at: flang/lib/Optimizer/Transforms/StackArrays.cpp:642
+ rewriter.setInsertionPointAfter(op);
+ else if (mlir::Block *block = insertionPoint.tryGetBlock())
+ rewriter.setInsertionPointToStart(block);
----------------
If this case must succeed when the other failed, it may be better to place it
in an `else {` and assert that a block was obtained, so that it is certain that
the insertion point was correctly set when looking at this code.
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.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
