Introduce the alloca op for stack memory allocation. When converting to the
LLVM dialect, this is lowered to an llvm.alloca.
Signed-off-by: Uday Bondhugula <uday@polymagelabs.com>
Paths
| Differential D76602
[MLIR] Introduce std.alloca op ClosedPublic Authored by bondhugula on Mar 23 2020, 5:21 AM.
Details Summary Introduce the alloca op for stack memory allocation. When converting to the Signed-off-by: Uday Bondhugula <uday@polymagelabs.com>
Diff Detail
Event TimelineComment Actions The amount of copy-pasta is uncanny, is there a way to factor out the 90%+ common part? This revision now requires changes to proceed.Mar 23 2020, 5:57 AM Comment Actions
Can the assembly format support its parsing and printing? Both affine.apply and alloc don't use the assembly format and have identical operand syntax (affine.apply was recently migrated to ODS but not auto print / parse). All these three require two variadic operand lists. Let me know. Comment Actions
I was thinking the same. The duplication is unfortunate and it's all in StandardToLLVM - it's possible to factor it all out if we get rid of useAlloca because the AllocOp lowering is conditional on that. Should I remove useAlloca in this patch itself and mostly merge AllocOpLowering and AllocaOpLowering? Merging the parse/print methods is straightforward. Comment Actions
@nicolasvasilache Done with the refactoring here (except for the llvm lowering - see question above). Comment Actions Thanks for adding the alloca op! Really needed. Comment Actions
alloca -> salloca sounds good to me! Note that the 'a' suffix in 'alloca' is for 'automatic' freeing (originating from early Unix's and BSDs and it has always meant allocating from the caller's stack). So, alloc -> malloca, alloca -> salloc would be inconsistent. Since 'alloc' currently doesn't specify where it's from the stack/heap and specifies it's explicitly freed via dealloc, we can leave it like that. Comment Actions
Thanks for clarifying! I thought it was just a short for 'allocate' :)
stack_alloc sounds better to me, thanks.
It sounds good. I think we are mapping 'alloc' to static allocation by using a flag in the llvm lowering. Maybe can create a simple pass to do a more proper alloc->static_alloc conversion in the future and leave 'alloc' only for heap allocation. Comment Actions @bondhugula Dropping the LLVM flag and refactoring the impl further sounds like the right thing to do IMO. Comment Actions
This sounds the best to me too. I'll do it in this patch. Comment Actions Refactored std to llvm lowering for alloc op to reuse for alloca. The lowering Comment Actions
Done @nicolasvasilache PTAL.
bondhugula added inline comments.
bondhugula added inline comments.
Comment Actions Thanks Uday, looks good! Accepting conditioned on the LLVM options struct change.
This revision is now accepted and ready to land.Apr 6 2020, 9:06 AM ftynse added inline comments.
This revision now requires changes to proceed.Apr 7 2020, 2:42 AM
bondhugula marked 8 inline comments as done. Comment ActionsAddress @nicolasvasilache review comments.
bondhugula added inline comments.
Closed by commit rG7023f4b4cb01: [MLIR] Introduce std.alloca op (authored by bondhugula). · Explain Why This revision was automatically updated to reflect the committed changes.
bondhugula added inline comments.
ftynse added inline comments.
bondhugula added inline comments.
bondhugula added inline comments.
bondhugula added inline comments.
Revision Contents
Diff 255319 mlir/include/mlir/Conversion/Passes.td
mlir/include/mlir/Conversion/StandardToLLVM/ConvertStandardToLLVMPass.hmlir/include/mlir/Dialect/StandardOps/IR/Ops.td
mlir/lib/Conversion/StandardToLLVM/StandardToLLVM.cpp
mlir/lib/Dialect/StandardOps/IR/Ops.cpp
mlir/test/Conversion/StandardToLLVM/convert-dynamic-memref-ops.mlir
mlir/test/Conversion/StandardToLLVM/convert-static-memref-ops.mlir
mlir/test/IR/memory-ops.mlir
mlir/test/Transforms/canonicalize.mlir
|
clang-format-diff not found in user's PATH; not linting file.