[flang] codegen for conversion between fp and logical

[tblah]

Summary

The IR verifier for fir.convert allows conversions between floating
point values and logical values, so I'm adding support here.

These converts can be generated using ENTRY statements with different
return types to the enclosing function and hlfir lowering.

[jeanPerier]

Could you provide an example with ENTRY for this use case, I am a bit surprised the conversion is not done at the storage level.

[tblah]

In D152830#4420052, @jeanPerier wrote:

Could you provide an example with ENTRY for this use case, I am a bit surprised the conversion is not done at the storage level.

Based on https://github.com/llvm/llvm-test-suite/blob/main/Fortran/gfortran/torture/execute/entry_4.f90

function f4 (a) result (r)
  logical r
  integer a
  double precision t
  entry g4 (a) result (t)
  r = a .lt. a
end function

https://reviews.llvm.org/D152725 is needed to get to this error, but I don't think this is the convert added by that patch. It is possible that this bug will go away once I move the convert to before the hlfir.declare as you suggested. I haven't got that far yet. I already had this code written so I thought I may as well share it. Either way, I think we should either support this case in codegen or catch this in the verifier.

[jeanPerier]

In D152830#4420408, @tblah wrote:

https://reviews.llvm.org/D152725 is needed to get to this error, but I don't think this is the convert added by that patch. It is possible that this bug will go away once I move the convert to before the hlfir.declare as you suggested. I haven't got that far yet. I already had this code written so I thought I may as well share it. Either way, I think we should either support this case in codegen or catch this in the verifier.

Thanks a lot for the explanation! I also think this fir.convert is a bug in the entry case. So I would favor a verifier error until there is a valid use case to ensure we catch such bugs.

It seems invalid to me to do a cast at the value level between the result types. The test is only testing returning .false. (a.lt.a) in a double precision storage. The test is not very thorough since .false. is implemented as zero, so the cast at the value level has the same effect as casting the storages. If the test was also testing the true case, I think the convert at the value level would likely give bad results:

module m
contains
function f4 (a) result (r)
  logical a, r
  real t
  entry g4 (a) result (t)
  r = a
end function
end module

  use m
  print*,  f4(.false.), g4(.false.)
  print*,  f4(.true.), g4(.true.)
end

With the logical representation that flang has, this should print (like gfortran):

F   0.00000000    
T   1.40129846E-45

But with a value level cast, this would likely print (I did not check):

F   0.00000000    
T   1.

Note that the original test is also a bit undefined depending on how g4() result is used because only part of the result storage is properly set by the function (double precision is bigger than default logical, so the upper bit of g4 result are undefined, they will likely be zeros, but not always....). Hence I used default real in my test variant.

[tblah]

In D152830#4420679, @jeanPerier wrote:

In D152830#4420408, @tblah wrote:

https://reviews.llvm.org/D152725 is needed to get to this error, but I don't think this is the convert added by that patch. It is possible that this bug will go away once I move the convert to before the hlfir.declare as you suggested. I haven't got that far yet. I already had this code written so I thought I may as well share it. Either way, I think we should either support this case in codegen or catch this in the verifier.

Thanks a lot for the explanation! I also think this fir.convert is a bug in the entry case. So I would favor a verifier error until there is a valid use case to ensure we catch such bugs.

It seems invalid to me to do a cast at the value level between the result types. The test is only testing returning .false. (a.lt.a) in a double precision storage. The test is not very thorough since .false. is implemented as zero, so the cast at the value level has the same effect as casting the storages. If the test was also testing the true case, I think the convert at the value level would likely give bad results:

module m
contains
function f4 (a) result (r)
  logical a, r
  real t
  entry g4 (a) result (t)
  r = a
end function
end module

  use m
  print*,  f4(.false.), g4(.false.)
  print*,  f4(.true.), g4(.true.)
end

With the logical representation that flang has, this should print (like gfortran):

F   0.00000000    
T   1.40129846E-45

But with a value level cast, this would likely print (I did not check):

F   0.00000000    
T   1.

Note that the original test is also a bit undefined depending on how g4() result is used because only part of the result storage is properly set by the function (double precision is bigger than default logical, so the upper bit of g4 result are undefined, they will likely be zeros, but not always....). Hence I used default real in my test variant.

Ahh good point. I will abandon this patch and fix the verifier. See https://reviews.llvm.org/D152935

flang/lib/Optimizer/CodeGen/CodeGen.cpp

if (fromFirTy.isa<fir::LogicalType>() || toFirTy.isa<fir::LogicalType>()) {
       // integer<->logical conversion requires value normalization.
       // Conversion from wide logical to narrow logical must set the result
       // to non-zero iff the input is non-zero - the easiest way to implement
       // it is to compare the input agains zero and set the result to
       // the canonical 0/1.
       // Conversion from narrow logical to wide logical may be implemented
       // as a zero or sign extension of the input, but it may use value
       // normalization as well.
+
+      if (isFloatingPointTy(fromTy)) {
+        // convert fp to integer
+        auto integer = rewriter.create<mlir::LLVM::FPToSIOp>(loc, toTy, op0);
+        // normalise integer
+        mlir::Value zero = genConstantIndex(loc, toTy, rewriter, 0);
+        auto isTrue = rewriter.create<mlir::LLVM::ICmpOp>(
+            loc, mlir::LLVM::ICmpPredicate::ne, integer, zero);
+        rewriter.replaceOpWithNewOp<mlir::LLVM::ZExtOp>(convert, toTy, isTrue);
+        return mlir::success();
+      }
+      if (isFloatingPointTy(toTy)) {
+        rewriter.replaceOpWithNewOp<mlir::LLVM::SIToFPOp>(convert, toTy, op0);
+        return mlir::success();
+      }
       if (!fromTy.isa<mlir::IntegerType>() || !toTy.isa<mlir::IntegerType>())
         return mlir::emitError(loc)
                << "unsupported types for logical conversion: " << fromTy
                << " -> " << toTy;
 
       // Do folding for constant inputs.
       if (auto constVal = getIfConstantIntValue(op0)) {
         mlir::Value normVal =

flang/test/Fir/logical-convert.fir

 // CHECK: [[t0:%[0-9]*]] = llvm.mlir.constant(0 : i64) : i64
 // CHECK: [[t1:%[0-9]*]] = llvm.icmp "ne" %arg0, [[t0]] : i64
 // CHECK: [[t2:%[0-9]*]] = llvm.zext [[t1]] : i1 to i32
 // CHECK: llvm.return [[t2]] : i32
 func.func @test_l8_l4(%arg0: !fir.logical<8>) -> !fir.logical<4> {
   %0 = fir.convert %arg0 : (!fir.logical<8>) -> !fir.logical<4>
   return %0 : !fir.logical<4>
 }
+// -----
+// CHECK-LABEL: @test_f32_l4
+// CHECK: %[[INT:.*]] = llvm.fptosi %arg0 : f32 to i32
+// CHECK: %[[ZERO:.*]] = llvm.mlir.constant(0 : i64) : i32
+// CHECK: %[[CMP:.*]] = llvm.icmp "ne" %[[INT]], %[[ZERO]] : i32
+// CHECK: %[[EXT:.*]] = llvm.zext %[[CMP]] : i1 to i32
+// CHECK: llvm.return %[[EXT]]
+func.func @test_f32_l4(%arg0: f32) -> !fir.logical<4> {
+  %0 = fir.convert %arg0 : (f32) -> !fir.logical<4>
+  return %0 : !fir.logical<4>
+}
+// -----
+// CHECK-LABEL: @test_f64_l4
+// CHECK: %[[INT:.*]] = llvm.fptosi %arg0 : f64 to i32
+// CHECK: %[[ZERO:.*]] = llvm.mlir.constant(0 : i64) : i32
+// CHECK: %[[CMP:.*]] = llvm.icmp "ne" %[[INT]], %[[ZERO]] : i32
+// CHECK: %[[EXT:.*]] = llvm.zext %[[CMP]] : i1 to i32
+// CHECK: llvm.return %[[EXT]]
+func.func @test_f64_l4(%arg0: f64) -> !fir.logical<4> {
+  %0 = fir.convert %arg0 : (f64) -> !fir.logical<4>
+  return %0 : !fir.logical<4>
+}
+// -----
+// CHECK-LABEL: @test_f32_l8
+// CHECK: %[[INT:.*]] = llvm.fptosi %arg0 : f32 to i64
+// CHECK: %[[ZERO:.*]] = llvm.mlir.constant(0 : i64) : i64
+// CHECK: %[[CMP:.*]] = llvm.icmp "ne" %[[INT]], %[[ZERO]] : i64
+// CHECK: %[[EXT:.*]] = llvm.zext %[[CMP]] : i1 to i64
+// CHECK: llvm.return %[[EXT]]
+func.func @test_f32_l8(%arg0: f32) -> !fir.logical<8> {
+  %0 = fir.convert %arg0 : (f32) -> !fir.logical<8>
+  return %0 : !fir.logical<8>
+}
+// -----
+// CHECK-LABEL: @test_f64_l8
+// CHECK: %[[INT:.*]] = llvm.fptosi %arg0 : f64 to i64
+// CHECK: %[[ZERO:.*]] = llvm.mlir.constant(0 : i64) : i64
+// CHECK: %[[CMP:.*]] = llvm.icmp "ne" %[[INT]], %[[ZERO]] : i64
+// CHECK: %[[EXT:.*]] = llvm.zext %[[CMP]] : i1 to i64
+// CHECK: llvm.return %[[EXT]]
+func.func @test_f64_l8(%arg0: f64) -> !fir.logical<8> {
+  %0 = fir.convert %arg0 : (f64) -> !fir.logical<8>
+  return %0 : !fir.logical<8>
+}