[PATCH] D140415: [flang] stack arrays pass

[Jean Perier via Phabricator via cfe-commits]

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