[flang][CodeGen] Transform `fir.emboxchar` to a sequence of LLVM MLIR

[awarzynski]

Summary

This patch extends the FIRToLLVMLowering pass in Flang by adding a
hook to transform fir.emboxchar to a sequence of LLVM MLIR
instructions.

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]

Move the test to flang/test/Fir/convert-to-llvm-target.fir, make it target specific

[awarzynski]

Based on the description of emboxchar, I'd expect it to work with fir.array. However, with fir.array the operation wouldn't match. Any pointers @schweitz ?

[awarzynski]

Remove auto, give variables more meaningful names, rebase

[clementval]

Don't forget to sync back to fir-dev. Especially when you make lots of changes (auto, variable names) otherwise it's gonna become a nightmare to track.

flang/lib/Optimizer/CodeGen/CodeGen.cpp

1403

This is often auto in MLIR code base.

1408

getLoc() is almost 100% of the time auto in the LLVM/MLIR code base.

[clementval]

LG. Just couple of nits

flang/lib/Optimizer/CodeGen/CodeGen.cpp

1414

clang-format.

[awarzynski]

In D113666#3126753, @clementval wrote:

Don't forget to sync back to fir-dev.

Yup, I was planning to send a bigger patch back to fir-dev once I finish fir.boxchar. Is that OK?

flang/lib/Optimizer/CodeGen/CodeGen.cpp

1403

Having seen LLVMContext, ASTContext, MCContext ... I just really like to know exactly which context I'm dealing with. But I guess that it's unlikely to be anything other than MLIRContext in this file.

1408

Almost:

grep  -iEr "Location getLoc" mlir/ | wc -l
       9

But auto is indeed more popular. Let me revert this.

[clementval]

In D113666#3126782, @awarzynski wrote:

In D113666#3126753, @clementval wrote:

Don't forget to sync back to fir-dev.

Yup, I was planning to send a bigger patch back to fir-dev once I finish fir.boxchar. Is that OK?

Yes. We should at least sync once a week if we had couple of patches. fir-dev is still moving and keeping it in sync will help to "freeze" some part of the tree in fir-dev later.

[awarzynski]

clang-format and use auto instead of mlir::Location (for consistency)

[mehdi_amini]

flang/lib/Optimizer/CodeGen/CodeGen.cpp

1408

getLoc() is almost 100% of the time auto in the LLVM/MLIR code base.

I don't think so.
I would use almost always Location: auto does not improve readability is enough for me to not use it (also it could be an SMLoc in some context) . When I use auto when writing code it is mostly out of laziness / convenience, but we should optimize for other to read code instead of the ease to write it.

I don't know how you grepped, but mine is:

$ git grep getLoc\( mlir | grep "loc =" | grep "auto" | wc -l
130
$ git grep getLoc\( mlir | grep "loc =" | grep -v "auto" | wc -l
237

So much more

[awarzynski]

flang/lib/Optimizer/CodeGen/CodeGen.cpp

1408

Thanks for checking! Clearly I wasn't awake when grepping.

we should optimize for other to read code instead of the ease to write it.

Agreed. Will update before merging.

flang/lib/Optimizer/CodeGen/CodeGen.cpp

mlir::LLVM::ConstantOp c0 =
         genConstantIndex(isPresent.getLoc(), idxTy, rewriter, 0);
     auto addr = rewriter.create<mlir::LLVM::PtrToIntOp>(loc, idxTy, ptr);
     rewriter.replaceOpWithNewOp<mlir::LLVM::ICmpOp>(
         isPresent, mlir::LLVM::ICmpPredicate::ne, addr, c0);
 
     return success();
   }
 };
+
+/// Convert `!fir.emboxchar<!fir.char<KIND, ?>, #n>` into a sequence of
+/// instructions that generate `!llvm.struct<(ptr<ik>, i64)>`. The 1st element
+/// in this struct is a pointer. Its type is determined from `KIND`. The 2nd
+/// element is the length of the character buffer (`#n`).
+struct EmboxCharOpConversion : public FIROpConversion<fir::EmboxCharOp> {
+  using FIROpConversion::FIROpConversion;
+
+  mlir::LogicalResult
+  matchAndRewrite(fir::EmboxCharOp emboxChar, OpAdaptor adaptor,
+                  mlir::ConversionPatternRewriter &rewriter) const override {
+    mlir::ValueRange operands = adaptor.getOperands();
+    MLIRContext *ctx = emboxChar.getContext();
+
+    mlir::Value charBuffer = operands[0];
+    mlir::Value charBufferLen = operands[1];
+
+    mlir::Location loc = emboxChar.getLoc();
+    mlir::Type llvmStructTy = convertType(emboxChar.getType());
+    auto llvmStruct = rewriter.create<mlir::LLVM::UndefOp>(loc, llvmStructTy);
+
+    mlir::Type lenTy =
+        llvmStructTy.cast<mlir::LLVM::LLVMStructType>().getBody()[1];
+    mlir::Value lenAfterCast = integerCast(loc, rewriter, lenTy, charBufferLen);
+
+    auto c0 = mlir::ArrayAttr::get(ctx, rewriter.getI32IntegerAttr(0));
+    auto c1 = mlir::ArrayAttr::get(ctx, rewriter.getI32IntegerAttr(1));
+    auto insertBufferOp = rewriter.create<mlir::LLVM::InsertValueOp>(
+        loc, llvmStructTy, llvmStruct, charBuffer, c0);
+    rewriter.replaceOpWithNewOp<mlir::LLVM::InsertValueOp>(
+        emboxChar, llvmStructTy, insertBufferOp, lenAfterCast, c1);
+
+    return success();
+  }
+};
 } // namespace
 
 namespace {
 /// Convert FIR dialect to LLVM dialect
 ///
 /// This pass lowers all FIR dialect operations to LLVM IR dialect. An
 /// MLIR pass is used to lower residual Std dialect to LLVM IR dialect.
 ///
void runOnOperation() override final {
     fir::LLVMTypeConverter typeConverter{getModule()};
     mlir::OwningRewritePatternList pattern(context);
     pattern.insert<
         AbsentOpConversion, AddcOpConversion, AddrOfOpConversion,
         AllocaOpConversion, BoxAddrOpConversion, BoxDimsOpConversion,
         BoxEleSizeOpConversion, BoxIsAllocOpConversion, BoxIsArrayOpConversion,
         BoxIsPtrOpConversion, BoxRankOpConversion, CallOpConversion,
         ConvertOpConversion, DispatchOpConversion, DispatchTableOpConversion,
-        DTEntryOpConversion, DivcOpConversion, ExtractValueOpConversion,
-        HasValueOpConversion, GlobalOpConversion, InsertOnRangeOpConversion,
-        InsertValueOpConversion, IsPresentOpConversion, LoadOpConversion,
-        NegcOpConversion, MulcOpConversion, SelectCaseOpConversion,
-        SelectOpConversion, SelectRankOpConversion, StoreOpConversion,
-        SubcOpConversion, UndefOpConversion, UnreachableOpConversion,
-        ZeroOpConversion>(typeConverter);
+        DTEntryOpConversion, DivcOpConversion, EmboxCharOpConversion,
+        ExtractValueOpConversion, HasValueOpConversion, GlobalOpConversion,
+        InsertOnRangeOpConversion, InsertValueOpConversion,
+        IsPresentOpConversion, LoadOpConversion, NegcOpConversion,
+        MulcOpConversion, SelectCaseOpConversion, SelectOpConversion,
+        SelectRankOpConversion, StoreOpConversion, SubcOpConversion,
+        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-target.fir

+// RUN: fir-opt --split-input-file --fir-to-llvm-ir="target=x86_64-unknown-linux-gnu" %s | FileCheck %s --check-prefix INT64
+// RUN: fir-opt --split-input-file --fir-to-llvm-ir="target=aarch64-unknown-linux-gnu" %s | FileCheck %s --check-prefixes INT64
+// RUN: fir-opt --split-input-file --fir-to-llvm-ir="target=i386-unknown-linux-gnu" %s | FileCheck %s --check-prefixes INT32
+// RUN: fir-opt --split-input-file --fir-to-llvm-ir="target=powerpc64le-unknown-linux-gn" %s | FileCheck %s --check-prefixes INT64
+
+//=============================================================================
+// SUMMARY: Tests for FIR --> LLVM MLIR conversion that *depend* on the target
+//=============================================================================
+
+// Test fir.emboxchar
+
+func @test_embox(%char_array : !fir.ref<!fir.char<1,?>>) -> () {
+  %c10 = arith.constant 10 : i64
+  %box_char = fir.emboxchar %char_array, %c10 : (!fir.ref<!fir.char<1,?>>, i64) -> !fir.boxchar<1>
+  return
+}
+
+// INT64-LABEL: test_embox
+// INT64-SAME: (%[[char_array:.*]]: !llvm.ptr<i8>)
+// INT64:    %[[c10:.*]] = llvm.mlir.constant(10 : i64) : i64
+// INT64:    %[[empty_struct:.*]] = llvm.mlir.undef : !llvm.struct<(ptr<i8>, i{{.*}})>
+// INT64:    %[[struct_with_buffer:.*]] = llvm.insertvalue %[[char_array]], %[[empty_struct]][0 : i32] : !llvm.struct<(ptr<i8>, i{{.*}})>
+// INT64:    %{{.*}} = llvm.insertvalue %[[c10]], %[[struct_with_buffer]][1 : i32] : !llvm.struct<(ptr<i8>, i{{.*}})>
+// INT64-NEXT:    llvm.return
+
+// INT32-LABEL:  llvm.func @test_embox
+// INT32-SAME: %[[char_array:.*]]: !llvm.ptr<i8>)
+// INT32:    %[[c10:.*]] = llvm.mlir.constant(10 : i64) : i64
+// INT32:    %[[empty_struct:.*]] = llvm.mlir.undef : !llvm.struct<(ptr<i8>, i32)>
+// INT32:    %[[c10_truncated:.*]] = llvm.trunc %[[c10]] : i64 to i32
+// INT32:    %[[struct_with_buffer:.*]] = llvm.insertvalue %[[char_array]], %[[empty_struct]][0 : i32] : !llvm.struct<(ptr<i8>, i32)>
+// INT32:    %{{.*}} = llvm.insertvalue %[[c10_truncated:.*]], %[[struct_with_buffer]][1 : i32] : !llvm.struct<(ptr<i8>, i32)>
+// INT32-NEXT:    llvm.return

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

 // RUN: fir-opt --split-input-file --fir-to-llvm-ir="target=x86_64-unknown-linux-gnu" %s | FileCheck %s
 // RUN: fir-opt --split-input-file --fir-to-llvm-ir="target=aarch64-unknown-linux-gnu" %s | FileCheck %s
 // RUN: fir-opt --split-input-file --fir-to-llvm-ir="target=i386-unknown-linux-gnu" %s | FileCheck %s
 // RUN: fir-opt --split-input-file --fir-to-llvm-ir="target=powerpc64le-unknown-linux-gn" %s | FileCheck %s
 
+//=============================================================================
+// SUMMARY: Tests for FIR --> LLVM MLIR conversion independent of the target
+//=============================================================================
 
 // Test simple global LLVM conversion
 
 fir.global @g_i0 : i32 {
   %1 = arith.constant 0 : i32
   fir.has_value %1 : i32
 }