[PATCH] D76602: [MLIR] Introduce std.alloca op

Mon Apr 6 05:55:17 PDT 2020

ftynse added inline comments.

================
Comment at: mlir/include/mlir/Dialect/StandardOps/IR/Ops.td:252
+  let description = [{
+    The "alloca" operation allocates memory on the stack, to be automatically
+    released when the stack frame is discarded. The amount of memory allocated
----------------
rriddle wrote:
> nit: "alloca" -> `alloca`
Nit: I think the intention of the comment above was to ask you to use backticks instead of double quotes.

================
Comment at: mlir/include/mlir/Dialect/StandardOps/IR/Ops.td:329
+    The "alloca" operation allocates memory on the stack, to be automatically
+    released when the stack frame is discarded. The amount of memory allocated
+    is specified by its memref and additional operands. For example:
----------------
Could you please elaborate what is a stack frame in MLIR? We don't seem to have this concept defined anywhere. In particular, is it only related to `std.func`, or can one register other ops that create stack frames?

================
Comment at: mlir/lib/Conversion/StandardToLLVM/StandardToLLVM.cpp:1349
+                SmallVectorImpl<Value> &sizes, Value &cumulativeSize,
+                Value &one) const {
+    sizes.reserve(memRefType.getRank());
----------------
Nit: can we rather define `one` at the call site and pass it here (and to another call) ?

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76602/new/

https://reviews.llvm.org/D76602