[PATCH] D140415: [flang] stack arrays pass

Wed Jan 25 08:29:59 PST 2023

tblah added a comment.

In D140415#4080080 <https://reviews.llvm.org/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 only look at explicit Flang generated values, right?

Yes only heap allocations added by flang are considered. `allocate` statements in source code are not changed.

> Are you handling unwinding/exceptions, especially in-between the allocation and deallocation?

There is no special handling for exceptions.

> Are you handling non-accessible stacks (e.g., on GPUs) for escaping pointers?

I am not. I am unfamiliar with this area, do you have any suggestions?

> Do you check the size to (reasonably) ensure we don't overflow the stack?

This pass avoids placing stack allocations inside loops, but does not check the size of the stack allocations themselves. In general, Flang will place local arrays of any size on the stack. These allocations can be moved to the heap using the MemoryAllocationOpt pass. In https://reviews.llvm.org/D140972 I made that pass mutually exclusive with this one, but so far as I know, it should be possible to run MemoryAllocaitonOpt after this one to move some of the temporary allocations back to the heap again. Note: you have to set non-default options for the MemoryAllocationOpt pass to move any allocations.

> Are you trying to move the alloca into the entry, if possible?

Yes

> Did you check LLVM's heap2stack and the corresponding tests?
> https://github.com/llvm/llvm-project/blob/c68af565ff0c2fdc5537e9ac0c2d7c75df44b035/llvm/lib/Transforms/IPO/AttributorAttributes.cpp#L6480
> https://github.com/llvm/llvm-project/blob/main/llvm/test/Transforms/Attributor/heap_to_stack.ll
> https://github.com/llvm/llvm-project/blob/main/llvm/test/Transforms/Attributor/heap_to_stack_gpu.ll

No I have not seen that. I will take a look, thank you.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140415