[PATCH] D147172: [OpenMP][Flang][MLIR] Add lowering of TargetOp for host codegen

[Kiran Chandramohan via Phabricator via llvm-commits]

kiranchandramohan added a comment.

In D147172#4245879 <https://reviews.llvm.org/D147172#4245879>, @jsjodin wrote:

> In D147172#4244736 <https://reviews.llvm.org/D147172#4244736>, @jsjodin wrote:
>
>> In D147172#4240342 <https://reviews.llvm.org/D147172#4240342>, @kiranchandramohan wrote:
>>
>>> In D147172#4237283 <https://reviews.llvm.org/D147172#4237283>, @jsjodin wrote:
>>>
>>>> @kiranchandramohan I moved the outlining to the OpenMPIRBuilder, is this more in line of what you were thinking? There is a patch up by @TIFitis for Fortran->Fir lowering https://reviews.llvm.org/D147339. I will prune this patch to only have the OMP+LLVM Dialect -> LLVMIR portions and  appropriate tests.
>>>
>>> Thanks @jsjodin for the changes. Yes, moving the code to the OpenMPIRBuilder looks fine. I have not looked at the patch in detail. But I see that you have not used the OpenMPIRBuilder's outlining facility (OutlineInfo(s) and the PostOutlineCB). Are there any issues in using these here?
>>
>> I have reworked the patch a bit to simplify things and added tests. The reason why the OutlineInfos etc are not used for the target regions is probably because the outlining is more complicated. With offloading we have to generate an if-then-else that tries to offload via the openmp runtime and if the runtime fails to launch the kernel the fallback is executed instead.
>
> Also since the outlining is shared with the device codegen, that is also a reason why the OutlineInfos etc is not used.

We have handling for the `if clause` in both task and parallel. Will the handling in Target be substantially different from these?
Task : D130615 <https://reviews.llvm.org/D130615>
Parallel : D138495 <https://reviews.llvm.org/D138495> (The latest revision uses a runtime extension, so that we do not have to generate IR for both sequential and parallel version).


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

https://reviews.llvm.org/D147172