[flang-commits] [PATCH] D144102: [flang][hlfir] Array constructor lowering [part 1/4]

[Tom Eccles via Phabricator via flang-commits]

tblah added inline comments.


================
Comment at: flang/lib/Lower/ConvertArrayConstructor.cpp:87
+    llvm::SmallVector<mlir::Value, 1> extents{extent};
+    mlir::Value tempStorage = builder.createHeapTemporary(
+        loc, declaredType, tempName, extents, lengths);
----------------
jeanPerier wrote:
> tblah wrote:
> > I thought the plan was to centralize all array temporary generation in the hlfir bufferization pass? I guess there is not a way to write "give me an uninitialised hlfir.expr which will be given a buffer later"?
> > 
> > It would be nice if expression buffer allocation could be done in one place so that, eventually, the stack arrays pass (and all its messy data flow analysis) can go away.
> > 
> > Another option would be to always allocate temporaries on the stack (no matter if `-fstack-arrays` or `-Ofast` were specified) and rely on the memory-allocation-opt pass to move large allocations to the heap, if that's desired. As I understand it, non-temporary array variables will already always be allocated on the stack (no matter their size).
> > 
> > Feel free to ignore this comment if this was a deliberate design decision.
> > I thought the plan was to centralize all array temporary generation in the hlfir bufferization pass?
> 
> It is as much as possible. But in some case, it is hard to implement the Fortran concept without using memory.
> The plan is to "pay the abstraction price" of removing the early temp creation where it makes an impact, but lowering may still resort to creating temps in some cases.
> 
> > I guess there is not a way to write "give me an uninitialised hlfir.expr which will be given a buffer later"?
> 
> Indeed, I think doing so and using chain of inserts would have a quite complex code generation to ensure a new temporary is not created at each insert.
> 
> > It would be nice if expression buffer allocation could be done in one place so that, eventually, the stack arrays pass (and all its messy data flow analysis) can go away.
> 
> Agree. I will look into it. I think I first need to propagate the option here, so I will do it in a separate patch.
> 
> To be fair, I am not saying it is impossible/undesired to have a powerful abstraction to represent all kinds of array constructors in HLFIR. I think it will need some careful design to cope with all cases and ensure it "plugs" well with the optimization passes. I am thinking we could have something like:
> 
> ```
> %expr = hlfir.array_ctor [%optional_shape] [%optional_length_parameters] {
>   fir.do_loop {
>     hlfir.push_value %acValue ...
>   }
>   hlfir.push_value %acValue ....
> }
> ```
> 
> The obvious benefit is that if %expr is then assigned, we could assume that the left hand side (if it is not a whole allocatable assignment) has the right storage size, and use that info to create the temp if needed, or if aliasing analysis proves the LHS is not used inside the array_ctor region.
> Same if we have Array + array_ctor_hard_to_predict_size, we could use conformability requirement to deduce the array constructor size.
> 
> I am not going that way now because I would like to better think the implication of such a new operation with region.
> 
Thanks for explaining. I think it is fine to come back to this at a later time


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144102