[Openmp-commits] [mlir] [openmp] [flang] [Flang][OpenMP] Remove use of non reference values from MapInfoOp (PR #72444)

[Akash Banerjee via Openmp-commits]

TIFitis wrote:

> I think you should not special case the load case but rather stay away from any cloning unless maybe when you detect a constant. Load op is not the only operation with side effects that may be emitted. Specification expressions may for instance contain pure calls (in the Fortran sense, which means they can still observe global states that could change).

I have special cased the `LoadOp` because that is the most common case we are probably going to encounter. These ops are only coming from the `BoxValue` bounds, so I imagine that we wouldn't be seeing too many variations.

I've added the other cases as a `TODO` failure, so we can handle them later as we expand support or come up with a more generalised approach for ops that might have side effects.

> So I would advise just making a temp from the bounds value instead of doing non trivial cloning (you can maybe just special case constants with fir::getIntIfConstant helper without doing any cloning with the cloning infrastructure).

I would like to add as little new temps inside the target region as possible. Using the `getIntIfConstant` we need to create the constantOps, shapeOps and potentially other ops such as select etc as well. Wouldn't it be cleaner to use the clone helper to create them rather than manually?

Is there any particular reason why we are avoiding the cloning infrastructure for trivial type values here?

Also the targetOp is protected with the `IsolatedFromAbove` trait, so we can be sure that we aren't unintentionally adding ops with side effects.



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