[flang-commits] [flang] [flang][OpenMP] incorrect handling for local variable in OpenMP parallel workshare firstprivate(P) (PR #195616)

[Tom Eccles via flang-commits]

tblah wrote:

I'm sorry it took me so long to get back to this. I have been on vacation.

> > I'm worried fir.ptr might lead to more pessimistic handling by fir::AliasAnalysis.
> > I think the problem here is trying to make a pointer to a pointer. Off the top of my head I can't remember another case where this is done in Flang. This suggests to me a broader design problem that is going unaddressed.

I mean that I don't think using !fir.ref<!fir.ptr<T>> as a way to get around being unable to create !fir.ref<!fir.ref<T>> is the right approach. This might be functional but it weakens FIR's type semantics. This has bad design smell because I don't know of any other precedent in flang. Why exactly do you need a reference to a reference here?

> Are you referring to examples like:
> 
> ```
>   real(8), pointer :: z(:), a(:)
>   !$omp parallel workshare firstprivate(z)
>   a = z + 1.0
>   !$omp end parallel workshare
> ```
> 
>  > If you want a private copy then you need to duplicate the type which is being pointed to not just make a copy of the !fir.ref. It is by design that you can't have two references pointing at the same memory. I'd be worried about having one pointer and one ref engineered to be the same memory - I imagine this will break an alias analysis assumption somewhere.
> 
> I have looked at the -emit-hlfir and -emit-mlir outputs. Here the private/duplicate copies of the z in the thread will be pointing to the same memory as the original z. It is preferable to avoid deep copy of the pointers. If I have to then please point me to currently existing precedence of this in the code. I will ensure a call is done to it.

Yes you are correct that the private copy of z and the original copy should both (initially) point to the same storage. If there was later a pointer assignment or re-allocation then the private z may no longer match the original z.

---

I think by `-emit-mlir` you mean `-emit-fir`? I know we do support that flag (although I think we probably shouldn't). `-emit-mlir` is a bit ambiguous. `-emit-hlfir` is valid mlir. `-emit-fir` is valid mlir. `flang -fc1 -emit-hlfir file.f90 | tco -test-gen` is also valid mlir. 

---

Thanks for fixing dynamic characters. Please add a regression test for this.

https://github.com/llvm/llvm-project/pull/195616