[Flang] Temporary fix for conversion materialization

[kiranchandramohan]

Summary

Simply add a source and target materialization handler that do nothing
and that override the default handlers that would add illegal
LLVM::DialectCastOp otherwise.

This is the simplest workaround, but not an actual fix, something may be
inconsistent after D82831 (most likely fir lowering to llvm happens in a
way that mlir infrastructure is not expecting in D82831).

Here is a minimal reproducer of what the issue was:

func @foop(%a : !fir.real<4>) -> ()
func @bar(%a : !fir.real<2>) {
  %1 = fir.convert %a : (!fir.real<2>) -> !fir.real<4>
  call @foop(%1) : (!fir.real<4>) -> ()
  return
}

tco -o - output was:

error: 'llvm.mlir.cast' op type must be non-index integer types, float types, or vector of mentioned types.
llvm.func @foop(!llvm.float)
llvm.func @bar(%arg0: !llvm.half) {
  %0 = llvm.fpext %arg0 : !llvm.half to !llvm.float
  %1 = llvm.mlir.cast %0 : !llvm.float to !fir.real<4>
  llvm.call @foop(%1) : (!fir.real<4>) -> ()
  llvm.return
}

This patch disable the introduction of the llvm.mlir.cast and preserve the previous behavior.

Also fixes https://github.com/llvm/llvm-project/issues/55210.

Note: This is part of upstreaming from the fir-dev branch of
https://github.com/flang-compiler/f18-llvm-project.

Co-authored-by: Jean Perier <[e-mail]>
Co-authored-by: Eric Schweitz <[e-mail]>

[awarzynski]

This was added as a workaround on fir-dev (marked as FIXME), but we haven't identified a better approach just yet and we need this in order to progress with upstreaming (and to resolve #55210).

Makes sense to me, thanks for sending!

[clementval]

In D127212#3565614, @awarzynski wrote:

This was added as a workaround on fir-dev (marked as FIXME), but we haven't identified a better approach just yet and we need this in order to progress with upstreaming (and to resolve #55210).

Makes sense to me, thanks for sending!

I'm curious about other approach you tried here. Do you mind to share?

[kiranchandramohan]

In D127212#3566507, @clementval wrote:

In D127212#3565614, @awarzynski wrote:

This was added as a workaround on fir-dev (marked as FIXME), but we haven't identified a better approach just yet and we need this in order to progress with upstreaming (and to resolve #55210).

Makes sense to me, thanks for sending!

I'm curious about other approach you tried here. Do you mind to share?

Jean's original patch (https://github.com/flang-compiler/f18-llvm-project/pull/313) called it a workaround hence I called it a temporary fix in this patch.
We have not tried anything else. There was a bit of discussion in https://github.com/llvm/llvm-project/issues/55210 about some context-sensitive lowering of fir.box. We prioritised upstreaming over attempting to fix.

[clementval]

In D127212#3569402, @kiranchandramohan wrote:

Jean's original patch (https://github.com/flang-compiler/f18-llvm-project/pull/313) called it a workaround hence I called it a temporary fix in this patch.
We have not tried anything else. There was a bit of discussion in https://github.com/llvm/llvm-project/issues/55210 about some context-sensitive lowering of fir.box. We prioritised upstreaming over attempting to fix.

Ok, @awarzynski comment made it sound like there was some investigations done on this. Would be good to check with MLIR folks how we can avoid this workaround.

[awarzynski]

In D127212#3572462, @clementval wrote:

Ok, @awarzynski comment made it sound like there was some investigations done on this.

Oh, I was just referring to the discussion on GitHub.

Would be good to check with MLIR folks how we can avoid this workaround.

+1

flang/lib/Optimizer/CodeGen/TypeConverter.h

addConversion([&](mlir::TupleType tuple) {
       }
       return mlir::LLVM::LLVMStructType::getLiteral(&getContext(), members,
                                                     /*isPacked=*/false);
     });
     addConversion([&](mlir::NoneType none) {
       return mlir::LLVM::LLVMStructType::getLiteral(
           none.getContext(), llvm::None, /*isPacked=*/false);
     });
+    // FIXME: https://reviews.llvm.org/D82831 introduced an automatic
+    // materliazation of conversion around function calls that is not working
+    // well with fir lowering to llvm (incorrect llvm.mlir.cast are inserted).
+    // Workaround until better analysis: register a handler that does not insert
+    // any conversions.
+    addSourceMaterialization(
+        [&](mlir::OpBuilder &builder, mlir::Type resultType,
+            mlir::ValueRange inputs,
+            mlir::Location loc) -> llvm::Optional<mlir::Value> {
+          if (inputs.size() != 1)
+            return llvm::None;
+          return inputs[0];
+        });
+    // Similar FIXME workaround here (needed for compare.fir/select-type.fir
+    // tests).
+    addTargetMaterialization(
+        [&](mlir::OpBuilder &builder, mlir::Type resultType,
+            mlir::ValueRange inputs,
+            mlir::Location loc) -> llvm::Optional<mlir::Value> {
+          if (inputs.size() != 1)
+            return llvm::None;
+          return inputs[0];
+        });
   }
 
   // i32 is used here because LLVM wants i32 constants when indexing into struct
   // types. Indexing into other aggregate types is more flexible.
   mlir::Type offsetType() { return mlir::IntegerType::get(&getContext(), 32); }
 
   // i64 can be used to index into aggregates like arrays
   mlir::Type indexType() { return mlir::IntegerType::get(&getContext(), 64); }

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

 // CHECK-COUNT-24:   %{{.*}} = llvm.insertvalue %[[CST]], %{{.*}}[{{.*}}] : !llvm.array<32 x i32>
 // CHECK:            %{{.*}} = llvm.insertvalue %[[CST]], %{{.*}}[31] : !llvm.array<32 x i32>
 // CHECK-NOT:        llvm.insertvalue
 // CHECK:            llvm.return %{{.*}} : !llvm.array<32 x i32>
 // CHECK:          }
 
 // -----
 
+// Test global with box
+
+fir.global internal @_QFEx : !fir.box<!fir.ptr<i32>> {
+  %0 = fir.zero_bits !fir.ptr<i32>
+  %1 = fir.embox %0 : (!fir.ptr<i32>) -> !fir.box<!fir.ptr<i32>>
+  fir.has_value %1 : !fir.box<!fir.ptr<i32>>
+}
+
+// CHECK-LABEL: llvm.mlir.global internal @_QFEx()
+// CHECK-SAME:  !llvm.struct<([[DES_FIELDS:.*]])>
+// CHECK:         %[[T0:.*]] = llvm.mlir.undef : !llvm.struct<([[DES_FIELDS]])>
+// CHECK:         %[[T1:.*]] = llvm.insertvalue %{{.*}}, %[[T0]][1 : i32] : !llvm.struct<([[DES_FIELDS]])>
+// CHECK:         %[[T2:.*]] = llvm.insertvalue %{{.*}}, %[[T1]][2 : i32] : !llvm.struct<([[DES_FIELDS]])>
+// CHECK:         %[[T3:.*]] = llvm.insertvalue %{{.*}}, %[[T2]][3 : i32] : !llvm.struct<([[DES_FIELDS]])>
+// CHECK:         %[[T4:.*]] = llvm.insertvalue %{{.*}}, %[[T3]][4 : i32] : !llvm.struct<([[DES_FIELDS]])>
+// CHECK:         %[[T5:.*]] = llvm.insertvalue %{{.*}}, %[[T4]][5 : i32] : !llvm.struct<([[DES_FIELDS]])>
+// CHECK:         %[[T6:.*]] = llvm.insertvalue %{{.*}}, %[[T5]][6 : i32] : !llvm.struct<([[DES_FIELDS]])>
+// CHECK:         %[[GDES:.*]] = llvm.insertvalue %{{.*}}, %[[T6]][0 : i32] : !llvm.struct<([[DES_FIELDS]])>
+// CHECK:         llvm.return %[[GDES]] : !llvm.struct<([[DES_FIELDS]])>
+
+// -----
+
 // Test fir.zero_bits operation with LLVM ptr type
 
 func.func @zero_test_ptr() {
   %z = fir.zero_bits !llvm.ptr<f32>
   return
 }
 
 // CHECK: %{{.*}} = llvm.mlir.null : !llvm.ptr<f32>