It is a nice implementation dealing with all compilation stages, kudos for that! I am not in favor of extending TypeCategory with Vector, which is rather central in your patch after parsing, and I am suggesting an alternative (please wait for more feedback before spending too much time trying it though).

Thanks, this looks great to me.

Looks good to me, thanks for simplifying codegen and making transformations more modular.

LGTM

Thanks, LGTM. Please beware there is a bot failure, but I think it is unrelated to your patch (you could rebase if this was fixed to check that).

Looks good.

LGTM

LGTM

I think the cause of the issue mentioned in the second bullet point is that the patterns were actually not using the OpAdaptor adaptor to get the operands as they should have in a full translation pass (sorry for not catching that, I always miss this....). E.g: sum.getArray() sould have been getBufferizedExprStorage(adaptor.getArray()). And the hlfir::AsExprOp in processReturnValue could have been repaced by packageBufferizedExpr(loc, builder, *resultEntity, mustBeFreed). This would have avoided using/introducing again hlfir.expr in the pass that gets rid of them. This would also address the first point in some way (but I agree the hlfir.expr operands would still also be transformed).

LGTM

LGTM

Looks great, thanks.

Makes sense to me.

Looks good to me

LGTM

It makes sense to me to add this interface to fir.if and LGTM. Just adding @vzakhari here to ensure I am not missing something.

Thanks for having dealt with all the comments, LGTM

LGTM

LGTM

Looks good

In D144869#4158655, @agozillon wrote:

From what I can tell there unfortunately isn't sadly, but my knowledge of LLVM/MLIR's pretty printing is a little lack luster.

Too bad, but not a blocker on my side. We will revisit pretty printing at some point to enable type alias usage for FIR derived types. Maybe we could see if it is possible to not add extra indentation to module content at that point.

Great job finding out the pieces, designator lowering is not the easiest piece of lowering. Looks good overall, some comment inline.

Thanks a lot, I think the IR can be slightly further simplify by merging the fir.absent and fir.emboxproc into a single fir.absent. Looks great otherwise.

In D144876#4155708, @pscoro wrote:

We are looking for community input regarding the error handling for these intrinsics (and more PowerPC intrinsics to come):

• Are we ok we doing semantic checking in the handler functions like we are doing for mtfsf/mtfsfi (ie checking if constant op and in valid range)

Correct typo in verifier error message.

LGTM

LGTM

It's great to have a generic solution to deal with the clean-ups of construct entities when branching out of them, thanks Val.

Thanks Pete, LGTM.

adding it as a discussion point in one of the weekly meetings or is an RFC on discourse perhaps better?

We can briefly discuss it at the meeting tomorrow to check no one rely on not having the module printed.

LGTM

Nits in the result type computation, looks good otherwise.

Small nit, looks good otherwise.

Thanks a lot for the fix!

I think removing this check is not correct from a semantic point of view because we are required to gracefully catch double allocation errors when STAT/ERRMSG is provided.

LGTM

Remove genTempFromSourceBox that is now unused. Thanks Pete.