[Mlir-commits] [flang] [mlir] [mlir][OpenMP] - Transform target offloading directives for easier translation to LLVMIR (PR #83966)

[Jan Leyonberg]

jsjodin wrote:

> > Is there anything preventing this from being done as part of PFT to MLIR lowering rather than a pass? It should be possible to make the `ClauseProcessor::processDepend` method create the outer `omp.task` with the corresponding DEPEND clause, and then have a simpler `ClauseProcessor::processTaskDepend` for the case when it's already attached to the TASK construct (so no new `omp.task` is created then). `omp.target*` ops wouldn't need any DEPEND-related arguments at that point.
> > In my opinion, that would be less expensive than adding a full pass, and in principle it shouldn't create any big compromises that I can think of. Do you agree with that? I don't mind this approach either, if it's generally preferred by others.
> 
> Thank you for your reply @skatrak. I don't think there is anything preventing it from being done in flang (PFT->MLIR lowering) and in fact, @ergawy had even suggested this in my discussion with him. However, my reasons for doing it this way largely boil down to preference for the following reasons (in no particular order)
> 
> * Keeping it in MLIR allows other frontends to use this because for any frontend to realize offloading in  LLVMIR, it'll have to go the route of creating an outer enclosing task
> * This way, arguably, keeps the interface of PFT->MLIR lowering cleaner - There will only be one process method handling `depend` -> `processDepend` and all ops using the `depend` clause will have the same interface (aka `processDepend`)
> * It separates the concerns of lowering (PFT->MLIR) and codegen (this transformation is needed essentially for codegen)
> 
> I felt at the time that these were reasons enough for paying the price of going over the IR in a new pass altogether. Having said that, it isn't a very very strong preference at all and I am happy to change my approach should that be required. How about I leave this open for others to comment on, while I test this with an end-to-end testcase by invoking this pass from within flang?

It seems like this is similar to what @kparzysz is working on addressing, not quite the same perhaps, but it would be nice if we could produce MLIR directly that has the omp.task op. If we do that, would it be possible to remove the depend clause from the omp.target op?

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