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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to