[flang][codegen] Add `fir.end` conversion placeholder

[awarzynski]

Summary

This patch extends the FIRToLLVMLowering pass in Flang by adding a
hook to transform fir.end. This is just a placeholder for now as fir.end is not required yet.

This is part of the upstreaming effort from the fir-dev branch in [1].

[1] https://github.com/flang-compiler/f18-llvm-project

Patch originally written by:
Co-authored-by: Eric Schweitz <[e-mail]>

[awarzynski]

Without the change in "FIROps.td" I was getting the following error:

error: custom op 'fir.end' has no custom assembly form

I'm not that familiar with the design, so perhaps this is not the right approach?

[mehdi_amini]

In D113295#3112136, @awarzynski wrote:

Without the change in "FIROps.td" I was getting the following error:

error: custom op 'fir.end' has no custom assembly form

I'm not that familiar with the design, so perhaps this is not the right approach?

flang/include/flang/Optimizer/Dialect/FIROps.td

812

You need this to be able to parse the "pretty" form. But do you need one?

flang/test/Fir/convert-to-llvm.fir

331

You could test it with just "fir.end"() : () -> () and not define a custom parser.

[mehdi_amini]

flang/include/flang/Optimizer/Dialect/FIROps.td

812

If you ever need a trivial custom form in the future, the preferred way would be to write it as let assemblyFormat = "attr-dict-with-keyword";

The assemblyFormat makes sure that you don't forget elements, like the optional dictionary of attributes here :)

[awarzynski]

flang/include/flang/Optimizer/Dialect/FIROps.td

812

You need this to be able to parse the "pretty" form. But do you need one?

I don't think I do - thanks for pointing this out! :)

flang/test/Fir/convert-to-llvm.fir

331

Ah, thanks! Yes, I will use that instead.

[awarzynski]

Remove the pretty printer and adjust the test accordingly

[mehdi_amini]

flang/lib/Optimizer/CodeGen/CodeGen.cpp

89

Nit: I think you can spell this rewriter.eraseOp for the same effect, but it expressed the intent a bit more clearly.

92

I was wondering if we need a pattern of its own for this op or if this should be handled by the parent operation?

The test shows it in isolation, but that does not exist, does it?

[mehdi_amini]

flang/lib/Optimizer/CodeGen/CodeGen.cpp

92

I even wonder if the verifier for fir.end shouldn't check on the actual parent op and limit it to be nested in some specific ones.

[awarzynski]

@mehdi_amini , these are much appreciated pointers, thank you!

flang/lib/Optimizer/CodeGen/CodeGen.cpp

92

I think that would make more sense. But, IIUC, in the current design there's only one such parent and it's only going to be needed for F2003. We are still working towards F95, so this is not required yet.

Perhaps we should just postpone this one until it's really needed?

[clementval]

flang/test/Fir/convert-to-llvm.fir

332

It would probably make more sense to have a real use case where the op is actually used as a terminator.

[clementval]

flang/lib/Optimizer/CodeGen/CodeGen.cpp

89

+1

[awarzynski]

fir.end is only used in fir.dispatch_table, and fir.dispatch_table is currently a notifyMatchFailure, i.e. it's not implemented. I think that that's what we should do with fir.end as well. Let me update this accordingly. @schweitz , any thoughts?

[awarzynski]

Leave fir.end as unimplemented

[schweitz]

In D113295#3134860, @awarzynski wrote:

fir.end is only used in fir.dispatch_table, and fir.dispatch_table is currently a notifyMatchFailure, i.e. it's not implemented. I think that that's what we should do with fir.end as well. Let me update this accordingly. @schweitz , any thoughts?

Seems reasonable. fir.end used to be widely applicable, but it's steadily been eroded.

In fact, given https://llvm.discourse.group/t/rfc-making-terminator-optional-for-single-block-graph-regions/2997 it seems like fir.end will be end of life soon.

[clementval]

Can you update the commit message since it has change from the first version of your patch.

flang/include/flang/Optimizer/Dialect/FIROps.td

def fir_FirEndOp : fir_Op<"end", [Terminator]> {
   let summary = "the end instruction";
 
   let description = [{
     The end terminator is a special terminator used inside various FIR
     operations that have regions.  End is thus the custom invisible terminator
     for these operations.  It is implicit and need not appear in the textual
     representation.
   }];
+
+  let parser = "return mlir::success();";
 }
 
 def fir_HasValueOp : fir_Op<"has_value", [Terminator, HasParent<"GlobalOp">]> {
   let summary = "terminator for GlobalOp";
   let description = [{
     The terminator for a GlobalOp with a body.
 
     ```mlir

flang/lib/Optimizer/CodeGen/CodeGen.cpp

matchAndRewrite(fir::AddrOfOp addr, OpAdaptor adaptor,
                   mlir::ConversionPatternRewriter &rewriter) const override {
     auto ty = convertType(addr.getType());
     rewriter.replaceOpWithNewOp<mlir::LLVM::AddressOfOp>(
         addr, ty, addr.symbol().getRootReference().getValue());
     return success();
   }
 };
 
+/// `fir.end` -> ``
+struct FirEndOpConversion : public FIROpConversion<fir::FirEndOp> {
+  using FIROpConversion::FIROpConversion;
+
+  mlir::LogicalResult
+  matchAndRewrite(fir::FirEndOp op, OpAdaptor,
+                  mlir::ConversionPatternRewriter &rewriter) const override {
+    rewriter.replaceOp(op, {});
+    return success();
+  }
+};
+
 /// Lower `fir.has_value` operation to `llvm.return` operation.
 struct HasValueOpConversion : public FIROpConversion<fir::HasValueOp> {
   using FIROpConversion::FIROpConversion;
 
   mlir::LogicalResult
   matchAndRewrite(fir::HasValueOp op, OpAdaptor adaptor,
                   mlir::ConversionPatternRewriter &rewriter) const override {
     rewriter.replaceOpWithNewOp<LLVM::ReturnOp>(op, adaptor.getOperands());

 class FIRToLLVMLowering : public fir::FIRToLLVMLoweringBase<FIRToLLVMLowering> {
 public:
   mlir::ModuleOp getModule() { return getOperation(); }
 
   void runOnOperation() override final {
     auto *context = getModule().getContext();
     fir::LLVMTypeConverter typeConverter{getModule()};
     mlir::OwningRewritePatternList pattern(context);
-    pattern.insert<
-        AddrOfOpConversion, ExtractValueOpConversion, HasValueOpConversion,
-        GlobalOpConversion, InsertOnRangeOpConversion, InsertValueOpConversion,
+    pattern
+        .insert<AddrOfOpConversion, ExtractValueOpConversion,
+                FirEndOpConversion, HasValueOpConversion, GlobalOpConversion,
+                InsertOnRangeOpConversion, InsertValueOpConversion,
                 SelectOpConversion, SelectRankOpConversion, UndefOpConversion,
                 UnreachableOpConversion, ZeroOpConversion>(typeConverter);
     mlir::populateStdToLLVMConversionPatterns(typeConverter, pattern);
     mlir::arith::populateArithmeticToLLVMConversionPatterns(typeConverter,
                                                             pattern);
     mlir::ConversionTarget target{*context};
     target.addLegalDialect<mlir::LLVM::LLVMDialect>();
 
     // required NOPs for applying a full conversion
     target.addLegalOp<mlir::ModuleOp>();

flang/test/Fir/convert-to-llvm.fir

func @insert_tuple(%a : tuple<i32, f32>) {
   %1 = fir.insert_value %a, %f, [1 : index] : (tuple<i32, f32>, f32) -> tuple<i32, f32>
   return
 }
 
 // CHECK-LABEL: func @insert_tuple(
 // CHECK-SAME:                     %[[TUPLE:.*]]: !llvm.struct<(i32, f32)>
 // CHECK:         %{{.*}} = llvm.insertvalue %{{.*}}, %[[TUPLE]][1 : index] : !llvm.struct<(i32, f32)>
 // CHECK:         llvm.return
+
+// -----
+
+// Test fir.end
+
+fir.end
+
+// CHECK-LABEL: module  {
+// CHECK-NEXT: }