Revert 0a65cb9.

The rework of the record type translation in code gen is resulting in
many regressions in testing. Revert this upstreaming sync patch until
the problems can be fixed correctly.

flang/lib/Optimizer/CodeGen/CodeGen.cpp

@@ -86,6 +86,9 @@ static constexpr unsigned kAttrAllocatable = CFI_attribute_allocatable;
 // fir::LLVMTypeConverter for converting to LLVM IR dialect types.
 #include "TypeConverter.h"
 
+// Instantiate static data member of the type converter.
+StringMap<mlir::Type> fir::LLVMTypeConverter::identStructCache;
+
 static inline mlir::Type getVoidPtrType(mlir::MLIRContext *context) {
   return mlir::LLVM::LLVMPointerType::get(mlir::IntegerType::get(context, 8));
 }

flang/lib/Optimizer/CodeGen/TypeConverter.h

@@ -65,6 +65,8 @@ class LLVMTypeConverter : public mlir::LLVMTypeConverter {
         [&](fir::CharacterType charTy) { return convertCharType(charTy); });
     addConversion(
         [&](fir::ComplexType cmplx) { return convertComplexType(cmplx); });
+    addConversion(
+        [&](fir::RecordType derived) { return convertRecordType(derived); });
     addConversion([&](fir::FieldType field) {
       return mlir::IntegerType::get(field.getContext(), 32);
     });
@@ -87,11 +89,6 @@ class LLVMTypeConverter : public mlir::LLVMTypeConverter {
     });
     addConversion(
         [&](fir::PointerType pointer) { return convertPointerLike(pointer); });
-    addConversion([&](fir::RecordType derived,
-                      SmallVectorImpl<mlir::Type> &results,
-                      ArrayRef<mlir::Type> callStack) {
-      return convertRecordType(derived, results, callStack);
-    });
     addConversion(
         [&](fir::RealType real) { return convertRealType(real.getFKind()); });
     addConversion(
@@ -158,29 +155,6 @@ class LLVMTypeConverter : public mlir::LLVMTypeConverter {
   // i64 can be used to index into aggregates like arrays
   mlir::Type indexType() { return mlir::IntegerType::get(&getContext(), 64); }
 
-  // fir.type<name(p : TY'...){f : TY...}>  -->  llvm<"%name = { ty... }">
-  llvm::Optional<LogicalResult>
-  convertRecordType(fir::RecordType derived,
-                    SmallVectorImpl<mlir::Type> &results,
-                    ArrayRef<mlir::Type> callStack) {
-    auto name = derived.getName();
-    auto st = mlir::LLVM::LLVMStructType::getIdentified(&getContext(), name);
-    // We are using an O(n) function (llvm::count) since we expect the stack
-    // size to be small.
-    if (llvm::count(callStack, derived) > 1) {
-      results.push_back(st);
-      return success();
-    }
-    llvm::SmallVector<mlir::Type> members;
-    for (auto mem : derived.getTypeList())
-      members.push_back(convertType(mem.second).cast<mlir::Type>());
-
-    if (mlir::failed(st.setBody(members, /*isPacked=*/false)))
-      return failure();
-    results.push_back(st);
-    return success();
-  }
-
   // Is an extended descriptor needed given the element type of a fir.box type ?
   // Extended descriptors are required for derived types.
   bool requiresExtendedDesc(mlir::Type boxElementType) {
@@ -214,9 +188,11 @@ class LLVMTypeConverter : public mlir::LLVMTypeConverter {
     descFields.push_back(
         getDescFieldTypeModel<kVersionPosInBox>()(&getContext()));
     // rank
-    descFields.push_back(getDescFieldTypeModel<kRankPosInBox>()(&getContext()));
+    descFields.push_back(
+        getDescFieldTypeModel<kRankPosInBox>()(&getContext()));
     // type
-    descFields.push_back(getDescFieldTypeModel<kTypePosInBox>()(&getContext()));
+    descFields.push_back(
+        getDescFieldTypeModel<kTypePosInBox>()(&getContext()));
     // attribute
     descFields.push_back(
         getDescFieldTypeModel<kAttributePosInBox>()(&getContext()));
@@ -347,6 +323,28 @@ class LLVMTypeConverter : public mlir::LLVMTypeConverter {
     return fromRealTypeID(kindMapping.getRealTypeID(kind), kind);
   }
 
+  // fir.type<name(p : TY'...){f : TY...}>  -->  llvm<"%name = { ty... }">
+  mlir::Type convertRecordType(fir::RecordType derived) {
+    auto name = derived.getName();
+    // The cache is needed to keep a unique mapping from name -> StructType
+    auto iter = identStructCache.find(name);
+    if (iter != identStructCache.end())
+      return iter->second;
+    auto st = mlir::LLVM::LLVMStructType::getIdentified(&getContext(), name);
+    identStructCache[name] = st;
+    llvm::SmallVector<mlir::Type> members;
+    for (auto mem : derived.getTypeList()) {
+      // Prevent fir.box from degenerating to a pointer to a descriptor in the
+      // context of a record type.
+      if (auto box = mem.second.dyn_cast<fir::BoxType>())
+        members.push_back(convertBoxTypeAsStruct(box));
+      else
+        members.push_back(convertType(mem.second).cast<mlir::Type>());
+    }
+    (void)st.setBody(members, /*isPacked=*/false);
+    return st;
+  }
+
   // fir.array<c ... :any>  -->  llvm<"[...[c x any]]">
   mlir::Type convertSequenceType(SequenceType seq) {
     auto baseTy = convertType(seq.getEleTy());
@@ -375,7 +373,7 @@ class LLVMTypeConverter : public mlir::LLVMTypeConverter {
         mlir::IntegerType::get(&getContext(), 8));
   }
 
-  /// Convert llvm::Type::TypeID to mlir::Type
+   /// Convert llvm::Type::TypeID to mlir::Type
   mlir::Type fromRealTypeID(llvm::Type::TypeID typeID, fir::KindTy kind) {
     switch (typeID) {
     case llvm::Type::TypeID::HalfTyID:
@@ -402,6 +400,7 @@ class LLVMTypeConverter : public mlir::LLVMTypeConverter {
 private:
   KindMapping kindMapping;
   std::unique_ptr<CodeGenSpecifics> specifics;
+  static llvm::StringMap<mlir::Type> identStructCache;
 };
 
 } // namespace fir

flang/test/Fir/recursive-type.fir

@@ -1,19 +1,11 @@
-// Test lowering FIR to LLVM IR for recursive types
+// Test lowering FIR to LLVM IR for a recursive type
 
-// RUN: fir-opt --fir-to-llvm-ir="target=x86_64-unknown-linux-gnu" %s | FileCheck %s
-// RUN: fir-opt --fir-to-llvm-ir="target=aarch64-unknown-linux-gnu" %s | FileCheck %s
-// RUN: fir-opt --fir-to-llvm-ir="target=i386-unknown-linux-gnu" %s | FileCheck %s
-// RUN: fir-opt --fir-to-llvm-ir="target=powerpc64le-unknown-linux-gn" %s | FileCheck %s
+// RUN: tco %s | FileCheck %s
 
-!t1 = type !fir.type<t1 {a1:!fir.ptr<!fir.type<t1>>}>
-!t2 = type !fir.type<t2 {b1:f32,b2:!fir.ptr<!fir.type<t2>>,b3:i32,b4:!fir.ptr<!fir.type<t2>>}>
-!t3 = type !fir.type<t3 {c1:!fir.ptr<!fir.type<t4>>}>
-!t4 = type !fir.type<t4 {d1:!fir.ptr<!fir.type<t3>>}>
+// CHECK-LABEL: %t = type { %t* }
+!t = type !fir.type<t {p : !fir.ptr<!fir.type<t>>}>
 
-// CHECK-LABEL: llvm.func @recursiveTypes
-// CHECK-SAME: %{{.*}}: !llvm.struct<"[[T1:.*]]", (ptr<struct<"[[T1]]">>)>
-// CHECK-SAME: %{{.*}}: !llvm.struct<"[[T2:.*]]", (f32, ptr<struct<"[[T2]]">>, i32, ptr<struct<"[[T2]]">>)>
-// CHECK-SAME: %{{.*}}: !llvm.struct<"[[T3:.*]]", (ptr<struct<"[[T4:.*]]", (ptr<struct<"[[T3]]">>)>>)>, %{{.*}}: !llvm.struct<"[[T4]]", (ptr<struct<"[[T3]]", (ptr<struct<"[[T4]]">>)>>)>)
-func @recursiveTypes(%a : !t1, %b : !t2, %c : !t3, %d : !t4) {
+// CHECK-LABEL: @a(%t %{{.*}})
+func @a(%a : !t) {
   return
 }

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

@@ -28,9 +28,6 @@ func private @foo0(%arg0: !fir.ref<i32>)
 func private @foo1(%arg0: !fir.ref<!fir.array<10xf32>>)
 // CHECK-LABEL: foo1
 // CHECK-SAME: !llvm.ptr<array<10 x f32>>
-func private @foo2(%arg0: !fir.ref<!fir.box<!fir.heap<!fir.type<_QMs1Ta1{x:i32,y:f32}>>>>)
-// CHECK-LABEL: foo2
-// CHECK-SAME: !llvm.ptr<struct<(ptr<struct<"_QMs1Ta1", (i32, f32)>>, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, ptr<i{{.*}}>, array<1 x i{{.*}}>)>>
 
 // -----
 
@@ -52,10 +49,6 @@ func private @foo3(%arg0: !fir.box<!fir.type<derived{f:f32}>>)
 // CHECK-LABEL: foo3
 // CHECK-SAME: !llvm.ptr<struct<(ptr<struct<"derived", (f32)>>, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, ptr<i{{.*}}>, array<1 x i{{.*}}>)>>
 
-func private @foo4(%arg0: !fir.box<!fir.heap<!fir.type<_QMs1Ta1{x:i32,y:f32}>>>)
-// CHECK-LABEL: foo4
-// CHECK-SAME: !llvm.ptr<struct<(ptr<struct<"_QMs1Ta1", (i32, f32)>>, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, ptr<i{{.*}}>, array<1 x i{{.*}}>)>>
-
 // -----
 
 // Test `!fir.logical<KIND>` conversion.
@@ -134,45 +127,6 @@ func private @foo5(%arg0: complex<f128>)
 
 // -----
 
-// Test `!fir.heap<>` conversion.
-func private @foo0(%arg0: !fir.heap<i32>)
-// CHECK-LABEL: foo0
-// CHECK-SAME: !llvm.ptr<i32>
-
-func private @foo1(%arg0: !fir.heap<!fir.array<4xf32>>)
-// CHECK-LABEL: foo1
-// CHECK-SAME: !llvm.ptr<array<4 x f32>>
-
-func private @foo2(%arg0: !fir.heap<!fir.array<?xf32>>)
-// CHECK-LABEL: foo2
-// CHECK-SAME: !llvm.ptr<f32>
-
-func private @foo3(%arg0: !fir.heap<!fir.char<1,10>>)
-// CHECK-LABEL: foo3
-// CHECK-SAME: !llvm.ptr<array<10 x i8>>
-
-func private @foo4(%arg0: !fir.heap<!fir.char<1,?>>)
-// CHECK-LABEL: foo4
-// CHECK-SAME: !llvm.ptr<i8>
-
-func private @foo5(%arg0: !fir.heap<!fir.array<2xf32>>)
-// CHECK-LABEL: foo5
-// CHECK-SAME: !llvm.ptr<array<2 x f32>>
-
-func private @foo6(%arg0: !fir.heap<!fir.array<?x?xf32>>)
-// CHECK-LABEL: foo6
-// CHECK-SAME: !llvm.ptr<f32>
-
-func private @foo7(%arg0: !fir.heap<!fir.type<ZT>>)
-// CHECK-LABEL: foo7
-// CHECK-SAME: !llvm.ptr<struct<"ZT", ()>>
-
-func private @foo8(%arg0: !fir.heap<!fir.type<_QMalloc_assignTt{i:i32}>>)
-// CHECK-LABEL: foo8
-// CHECK-SAME: !llvm.ptr<struct<"_QMalloc_assignTt", (i32)>>
-
-// -----
-
 // Test `!fir.llvm_ptr` conversion.
 
 func private @foo0(%arg0: !fir.llvm_ptr<i8>)

mlir/docs/DialectConversion.md

@@ -307,14 +307,6 @@ class TypeConverter {
   ///       existing value are expected to be removed during conversion. If
   ///       `llvm::None` is returned, the converter is allowed to try another
   ///       conversion function to perform the conversion.
-  ///   * Optional<LogicalResult>(T, SmallVectorImpl<Type> &, ArrayRef<Type>)
-  ///     - This form represents a 1-N type conversion supporting recursive
-  ///       types. The first two arguments and the return value are the same as
-  ///       for the regular 1-N form. The third argument is contains is the
-  ///       "call stack" of the recursive conversion: it contains the list of
-  ///       types currently being converted, with the current type being the
-  ///       last one. If it is present more than once in the list, the
-  ///       conversion concerns a recursive type.
   /// Note: When attempting to convert a type, e.g. via 'convertType', the
   ///       mostly recently added conversions will be invoked first.
   template <typename FnT,

mlir/include/mlir/Transforms/DialectConversion.h

@@ -101,14 +101,6 @@ class TypeConverter {
   ///       existing value are expected to be removed during conversion. If
   ///       `llvm::None` is returned, the converter is allowed to try another
   ///       conversion function to perform the conversion.
-  ///   * Optional<LogicalResult>(T, SmallVectorImpl<Type> &, ArrayRef<Type>)
-  ///     - This form represents a 1-N type conversion supporting recursive
-  ///       types. The first two arguments and the return value are the same as
-  ///       for the regular 1-N form. The third argument is contains is the
-  ///       "call stack" of the recursive conversion: it contains the list of
-  ///       types currently being converted, with the current type being the
-  ///       last one. If it is present more than once in the list, the
-  ///       conversion concerns a recursive type.
   /// Note: When attempting to convert a type, e.g. via 'convertType', the
   ///       mostly recently added conversions will be invoked first.
   template <typename FnT, typename T = typename llvm::function_traits<
@@ -230,8 +222,8 @@ class TypeConverter {
   /// The signature of the callback used to convert a type. If the new set of
   /// types is empty, the type is removed and any usages of the existing value
   /// are expected to be removed during conversion.
-  using ConversionCallbackFn = std::function<Optional<LogicalResult>(
-      Type, SmallVectorImpl<Type> &, ArrayRef<Type>)>;
+  using ConversionCallbackFn =
+      std::function<Optional<LogicalResult>(Type, SmallVectorImpl<Type> &)>;
 
   /// The signature of the callback used to materialize a conversion.
   using MaterializationCallbackFn =
@@ -249,44 +241,28 @@ class TypeConverter {
   template <typename T, typename FnT>
   std::enable_if_t<llvm::is_invocable<FnT, T>::value, ConversionCallbackFn>
   wrapCallback(FnT &&callback) {
-    return wrapCallback<T>(
-        [callback = std::forward<FnT>(callback)](
-            T type, SmallVectorImpl<Type> &results, ArrayRef<Type>) {
-          if (Optional<Type> resultOpt = callback(type)) {
-            bool wasSuccess = static_cast<bool>(resultOpt.getValue());
-            if (wasSuccess)
-              results.push_back(resultOpt.getValue());
-            return Optional<LogicalResult>(success(wasSuccess));
-          }
-          return Optional<LogicalResult>();
-        });
-  }
-  /// With callback of form: `Optional<LogicalResult>(T, SmallVectorImpl<Type>
-  /// &)`
+    return wrapCallback<T>([callback = std::forward<FnT>(callback)](
+                               T type, SmallVectorImpl<Type> &results) {
+      if (Optional<Type> resultOpt = callback(type)) {
+        bool wasSuccess = static_cast<bool>(resultOpt.getValue());
+        if (wasSuccess)
+          results.push_back(resultOpt.getValue());
+        return Optional<LogicalResult>(success(wasSuccess));
+      }
+      return Optional<LogicalResult>();
+    });
+  }
+  /// With callback of form: `Optional<LogicalResult>(T, SmallVectorImpl<> &)`
   template <typename T, typename FnT>
-  std::enable_if_t<llvm::is_invocable<FnT, T, SmallVectorImpl<Type> &>::value,
-                   ConversionCallbackFn>
-  wrapCallback(FnT &&callback) {
-    return wrapCallback<T>(
-        [callback = std::forward<FnT>(callback)](
-            T type, SmallVectorImpl<Type> &results, ArrayRef<Type>) {
-          return callback(type, results);
-        });
-  }
-  /// With callback of form: `Optional<LogicalResult>(T, SmallVectorImpl<Type>
-  /// &, ArrayRef<Type>)`.
-  template <typename T, typename FnT>
-  std::enable_if_t<llvm::is_invocable<FnT, T, SmallVectorImpl<Type> &,
-                                      ArrayRef<Type>>::value,
-                   ConversionCallbackFn>
+  std::enable_if_t<!llvm::is_invocable<FnT, T>::value, ConversionCallbackFn>
   wrapCallback(FnT &&callback) {
     return [callback = std::forward<FnT>(callback)](
-               Type type, SmallVectorImpl<Type> &results,
-               ArrayRef<Type> callStack) -> Optional<LogicalResult> {
+               Type type,
+               SmallVectorImpl<Type> &results) -> Optional<LogicalResult> {
       T derivedType = type.dyn_cast<T>();
       if (!derivedType)
         return llvm::None;
-      return callback(derivedType, results, callStack);
+      return callback(derivedType, results);
     };
   }
 
@@ -325,10 +301,6 @@ class TypeConverter {
   DenseMap<Type, Type> cachedDirectConversions;
   /// This cache stores the successful 1->N conversions, where N != 1.
   DenseMap<Type, SmallVector<Type, 2>> cachedMultiConversions;
-
-  /// Stores the types that are being converted in the case when convertType
-  /// is being called recursively to convert nested types.
-  SmallVector<Type, 2> conversionCallStack;
 };
 
 //===----------------------------------------------------------------------===//

mlir/lib/Transforms/Utils/DialectConversion.cpp

@@ -14,7 +14,6 @@
 #include "mlir/IR/FunctionSupport.h"
 #include "mlir/Rewrite/PatternApplicator.h"
 #include "mlir/Transforms/Utils.h"
-#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/Support/Debug.h"
@@ -2521,12 +2520,8 @@ LogicalResult TypeConverter::convertType(Type t,
   // Walk the added converters in reverse order to apply the most recently
   // registered first.
   size_t currentCount = results.size();
-  conversionCallStack.push_back(t);
-  auto popConversionCallStack =
-      llvm::make_scope_exit([this]() { conversionCallStack.pop_back(); });
   for (ConversionCallbackFn &converter : llvm::reverse(conversions)) {
-    if (Optional<LogicalResult> result =
-            converter(t, results, conversionCallStack)) {
+    if (Optional<LogicalResult> result = converter(t, results)) {
       if (!succeeded(*result)) {
         cachedDirectConversions.try_emplace(t, nullptr);
         return failure();

mlir/test/Transforms/test-legalize-type-conversion.mlir

@@ -113,26 +113,3 @@ func @test_block_argument_not_converted() {
   }) : () -> ()
   return
 }
-
-// -----
-
-// Make sure argument type changes aren't implicitly forwarded.
-func @test_signature_conversion_no_converter() {
-  "test.signature_conversion_no_converter"() ({
-  // expected-error@below {{failed to materialize conversion for block argument #0 that remained live after conversion}}
-  ^bb0(%arg0: f32):
-    // expected-note@below {{see existing live user here}}
-    "test.type_consumer"(%arg0) : (f32) -> ()
-    "test.return"(%arg0) : (f32) -> ()
-  }) : () -> ()
-  return
-}
-
-// -----
-
-// CHECK-LABEL: @recursive_type_conversion
-func @recursive_type_conversion() {
-  // CHECK:  !test.test_rec<outer_converted_type, smpla>
-  "test.type_producer"() : () -> !test.test_rec<something, test_rec<something>>
-  return
-}