[flang-commits] [flang] [flang][OpenMP] Add TODOs for target [teams|parallel] private (PR #143706)

[Kareem Ergawy via flang-commits]

ergawy wrote:

> @ergawy: Kareem, these look like bugs, are those cases supposed to work with the current code?
> 
> 
> 
> I checked a simple testcase
> 
> ```
> 
> subroutine t
> 
>   integer :: i
> 
>   !$omp target parallel private(i)
> 
>   !$omp end target parallel
> 
> end
> 
> ```
> 
> 
> 
> The problem in the generated MLIR was that the argument to omp.parallel is defined outside of the omp.target region, which causes verification failure: "'omp.parallel' op using value defined outside the region".
> 
> 
> 
> ```
> 
>   "func.func"() <{function_type = () -> (), sym_name = "_QPt"}> ({
> 
>     %0 = "fir.dummy_scope"() : () -> !fir.dscope
> 
>     %1 = "fir.alloca"() <{bindc_name = "i", in_type = i32, operandSegmentSizes = array<i32: 0, 0>, uniq_name = "_QFtEi"}> : () -> !fir.ref<i32>
> 
>     %2:2 = "hlfir.declare"(%1) <{operandSegmentSizes = array<i32: 1, 0, 0, 0>, uniq_name = "_QFtEi"}> : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
> 
>     %3 = "omp.map.info"(%2#1) <{map_capture_type = #omp<variable_capture_kind(ByCopy)>, map_type = 512 : ui64, name = "i", operandSegmentSizes = array<i32: 1, 0, 0, 0>, partial_map = false, var_type = i32}> : (!fir.ref<i32>) -> !fir.ref<i32>
> 
>     "omp.target"(%3) <{operandSegmentSizes = array<i32: 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0>}> ({
> 
>     ^bb0(%arg0: !fir.ref<i32>):
> 
>       %4:2 = "hlfir.declare"(%arg0) <{operandSegmentSizes = array<i32: 1, 0, 0, 0>, uniq_name = "_QFtEi"}> : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
> 
>       %5 = "fir.undefined"() : () -> index
> 
>       "omp.parallel"(%2#0) <{operandSegmentSizes = array<i32: 0, 0, 0, 0, 1, 0>, private_syms = [@_QFtEi_private_i32]}> ({
> 
>       ^bb0(%arg1: !fir.ref<i32>):
> 
>         %6:2 = "hlfir.declare"(%arg1) <{operandSegmentSizes = array<i32: 1, 0, 0, 0>, uniq_name = "_QFtEi"}> : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
> 
>         "omp.terminator"() : () -> ()
> 
>       }) : (!fir.ref<i32>) -> ()
> 
>       "omp.terminator"() : () -> ()
> 
>     }) : (!fir.ref<i32>) -> ()
> 
>   }) : () -> ()
> 
> ```

Looks like incorrect symbol binding indeed. Thanks for bringing that to my attention. I will take a look tomorrow.

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