[flang][hlfir] support hlfir.assign with unconvertable types

[tblah]

Summary

Use of the entry statement can cause different variables with different,
unconvertable types to share the same storage. A hlfir.assign for these
types cannot convert the rhs, so instead the lhs must be converted
(which is how assignments are lowered in FIR).

Example code this fixes:

complex function f1()
  logical e1
  f1 = complex(1.0, 2.0)
  entry e1()
  e1 = .false.
end function

[tblah]

Remove extra line in test

[jeanPerier]

Thanks for working on this issue!
I think we should try to approach it the other way round: insert the fir.convert before the hlfir.declare so that the symbol get the correct type (might be required anyway to get correct debug info later). Relaxing genScalarAssignment may also silently allow some future bug to compile.

There may also be other places that do not like getting the right type than assignment but for which we lack tests.

The convert should be added on primaryFuncResultStorage before calling mapSymbolAttributes here https://github.com/llvm/llvm-project/blob/6e3a8720474528f8f752d0afbc6b8b9efab96325/flang/lib/Lower/Bridge.cpp#L3996.

[tblah]

In D152725#4417572, @jeanPerier wrote:

Thanks for working on this issue!
I think we should try to approach it the other way round: insert the fir.convert before the hlfir.declare so that the symbol get the correct type (might be required anyway to get correct debug info later). Relaxing genScalarAssignment may also silently allow some future bug to compile.

There may also be other places that do not like getting the right type than assignment but for which we lack tests.

The convert should be added on primaryFuncResultStorage before calling mapSymbolAttributes here https://github.com/llvm/llvm-project/blob/6e3a8720474528f8f752d0afbc6b8b9efab96325/flang/lib/Lower/Bridge.cpp#L3996.

Thanks for pointing me at this, I agree this is a better solution. I've done that in https://reviews.llvm.org/D152931 and will abandon this patch.

flang/lib/Optimizer/Builder/FIRBuilder.cpp

void fir::factory::genScalarAssignment(fir::FirOpBuilder &builder,
   } else if (type.isa<fir::RecordType>()) {
     fir::factory::genRecordAssignment(builder, loc, lhs, rhs);
   } else {
     assert(!fir::hasDynamicSize(type));
     auto rhsVal = fir::getBase(rhs);
     if (fir::isa_ref_type(rhsVal.getType()))
       rhsVal = builder.create<fir::LoadOp>(loc, rhsVal);
     mlir::Value lhsAddr = fir::getBase(lhs);
-    rhsVal = builder.createConvert(loc, fir::unwrapRefType(lhsAddr.getType()),
-                                   rhsVal);
+    mlir::Type lhsTargetType = fir::unwrapRefType(lhsAddr.getType());
+    mlir::Type rhsTy = rhsVal.getType();
+    if (rhsTy != lhsTargetType) {
+      if (fir::ConvertOp::canBeConverted(rhsTy, lhsTargetType))
+        rhsVal = builder.createConvert(
+            loc, fir::unwrapRefType(lhsAddr.getType()), rhsVal);
+      else
+        // we can't convert rhs to match lhs, so instead convert lhs e.g.
+        // convert !fir.ref<A> to !fir.ref<B>
+        lhsAddr =
+            builder.createConvert(loc, ReferenceType::get(rhsTy), lhsAddr);
+    }
     builder.create<fir::StoreOp>(loc, rhsVal, lhsAddr);
   }
 }
 
 static void genComponentByComponentAssignment(fir::FirOpBuilder &builder,
                                               mlir::Location loc,
                                               const fir::ExtendedValue &lhs,
                                               const fir::ExtendedValue &rhs) {

flang/test/HLFIR/assign-codegen.fir

 // CHECK:           %[[VAL_6:.*]] = fir.embox %[[VAL_4]] : (!fir.ref<!fir.type<_QFtestTt1{a:!fir.box<!fir.heap<!fir.array<?xi32>>>}>>) -> !fir.box<!fir.type<_QFtestTt1{a:!fir.box<!fir.heap<!fir.array<?xi32>>>}>>
 // CHECK:           fir.store %[[VAL_5]] to %[[VAL_2]] : !fir.ref<!fir.box<!fir.type<_QFtestTt1{a:!fir.box<!fir.heap<!fir.array<?xi32>>>}>>>
 // CHECK:           %[[VAL_10:.*]] = fir.convert %[[VAL_2]] : (!fir.ref<!fir.box<!fir.type<_QFtestTt1{a:!fir.box<!fir.heap<!fir.array<?xi32>>>}>>>) -> !fir.ref<!fir.box<none>>
 // CHECK:           %[[VAL_11:.*]] = fir.convert %[[VAL_6]] : (!fir.box<!fir.type<_QFtestTt1{a:!fir.box<!fir.heap<!fir.array<?xi32>>>}>>) -> !fir.box<none>
 // CHECK:           %[[VAL_12:.*]] = fir.convert %{{.*}} : (!fir.ref<!fir.char<1,{{.*}}>>) -> !fir.ref<i8>
 // CHECK:           %[[VAL_13:.*]] = fir.call @_FortranAAssign(%[[VAL_10]], %[[VAL_11]], %[[VAL_12]], %{{.*}}) : (!fir.ref<!fir.box<none>>, !fir.box<none>, !fir.ref<i8>, i32) -> none
 // CHECK:           return
 // CHECK:         }
+
+// Test we can assign types which cannot be converted e.g. logical -> complex
+func.func @_QPe1() -> !fir.logical<4> {
+  %0 = fir.alloca !fir.complex<4> {bindc_name = "f1", uniq_name = "_QFf1Ef1"}
+  %1:2 = hlfir.declare %0 {uniq_name = "_QFf1Ef1"} : (!fir.ref<!fir.complex<4>>) -> (!fir.ref<!fir.complex<4>>, !fir.ref<!fir.complex<4>>)
+  %2:2 = hlfir.declare %1#1 {uniq_name = "_QFf1Ee1"} : (!fir.ref<!fir.complex<4>>) -> (!fir.ref<!fir.complex<4>>, !fir.ref<!fir.complex<4>>)
+  %false = arith.constant false
+  %3 = fir.convert %false : (i1) -> !fir.logical<4>
+  // this is the important bit:
+  hlfir.assign %3 to %2#0 : !fir.logical<4>, !fir.ref<!fir.complex<4>>
+  %4 = fir.convert %2#1 : (!fir.ref<!fir.complex<4>>) -> !fir.ref<!fir.logical<4>>
+  %5 = fir.load %4 : !fir.ref<!fir.logical<4>>
+  return %5 : !fir.logical<4>
+}
+// CHECK-LABEL: func.func @_QPe1()
+// CHECK-NEXT:    %[[F1_ALLOC:.*]] = fir.alloca !fir.complex<4>
+// CHECK-NEXT:    %[[F1_DECL:.*]] = fir.declare %[[F1_ALLOC]]
+// CHECK-NEXT:    %[[E1_DECL:.*]] = fir.declare %[[F1_DECL]]
+// CHECK-NEXT:    %[[FALSE:.*]] = arith.constant false
+// CHECK-NEXT:    %[[FALSE_LOGICAL:.*]] = fir.convert %[[FALSE]]
+// this is the important bit:
+// CHECK-NEXT:    %[[CONV:.*]] = fir.convert %[[E1_DECL]] : (!fir.ref<!fir.complex<4>>) -> !fir.ref<!fir.logical<4>>
+// CHECK-NEXT:    fir.store %[[FALSE_LOGICAL]] to %[[CONV]]