[flang-commits] [flang] [flang][openacc] Allow open acc routines from other modules. (PR #136012)

[via flang-commits]

https://github.com/jeanPerier commented:

The story about the FIROpBuilder is that they assumed that their insertion point is nested inside a fun.func. Right now this is visible with the `getFunction()` usages, although I think the original issue was that `getRegion()` would not work at the top level of the module and `getRegion` is used in many contexts by the builder. I am sure it was at least true at the time when module were not "normal operations" ([there was such a time](https://groups.google.com/a/tensorflow.org/g/mlir/c/efTa3yF_b6I/m/c9Kh42XXAQAJ)), I am not sure about today.

That is why createGlobalOutsideOfFunctionLowering is creating a mock fun.func context.

Another point was that some part of the Bridge could have been parallelized and therefore using a fir::FIROpBuilder per procedure being lowered was seen as step towards that. But the fact that procedure lowering requires creating some extra fun.func and fir.global "on the fly" prevents leveraging mlir parallelism without extra care, so I would be OK to just go with a single fir::FIROpBuilder now. That will simplify things a bit.

Anyway, the point is:

It was bad to create fir::FIROpBuilder outside of fun.func at some point, so stages that happened before fun.func creation only used the mlir::MLIRContext features (example: type conversion and function signature creation), or a mock function was created. Maybe the OpenACC helper could use the MLIRContext directly, but I understand this may not be straightforward.

Either it is still bad, and then your patch is a bit problematic because it introduces a fir::FirOpBuilder that is unsafe to use, or it is not, and then it is fine and could be simplified by using a single builder as you suggested.

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