[flang-commits] [PATCH] D121200: [flang] Use unix logical representation for fir.logical

[Jean Perier via Phabricator via flang-commits]

jeanPerier created this revision.
jeanPerier added reviewers: clementval, schweitz.
jeanPerier added a project: Flang.
Herald added subscribers: mehdi_amini, jdoerfert.
Herald added a project: All.
jeanPerier requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

The front-end and the runtime are currently using the unix logical
representation, but lowering was not. These inconsistencies could
caused issues.

The only place that defines what the logical representation is in
lowering is the translation from FIR to LLVM (FIR is agnostic to the
actual representation). More precisely, the LLVM implementation of
`fir.convert` between `i1` and `fir.logcial` is what defines the
representation:

- `fir.convert` from `i1` to `fir.logical` defines the `.true.` and `.false.`

canonical representations

- `fir.convert` from `fir.logical` to `i1` decides what the test for

truth is.

Unix representation is:

- .true. canonical integer representation is 1
- .false. canonical integer representation is 0
- the test for truth is "integer representation != 0"

For the record, the previous representation that was used was in
codegen was:

- .true. canonical integer representation is -1 (all bits 1)
- .false. canonical integer representation is 0
- the test for truth is "integer representation lowest bit == 1"


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121200

Files:
  flang/lib/Optimizer/CodeGen/CodeGen.cpp
  flang/test/Fir/convert-to-llvm.fir


Index: flang/test/Fir/convert-to-llvm.fir
===================================================================
--- flang/test/Fir/convert-to-llvm.fir
+++ flang/test/Fir/convert-to-llvm.fir
@@ -1288,7 +1288,8 @@
 // CHECK-LABEL: llvm.func @select_case_logical(
 // CHECK-SAME:                                 %[[ARG0:.*]]: !llvm.ptr<i32>
 // CHECK:         %[[LOAD_ARG0:.*]] = llvm.load %[[ARG0]] : !llvm.ptr<i32>
-// CHECK:         %[[SELECT_VALUE:.*]] = llvm.trunc %[[LOAD_ARG0]] : i32 to i1
+// CHECK:         %[[CST_ZERO:.*]] = llvm.mlir.constant(0 : i64) : i32
+// CHECK:         %[[SELECT_VALUE:.*]] = llvm.icmp "ne" %[[LOAD_ARG0]], %[[CST_ZERO]] : i32
 // CHECK:         %[[CST_FALSE:.*]] = llvm.mlir.constant(false) : i1
 // CHECK:         %[[CST_TRUE:.*]] = llvm.mlir.constant(true) : i1
 // CHECK:         %[[CMPEQ:.*]] = llvm.icmp "eq" %[[SELECT_VALUE]], %[[CST_FALSE]] : i1
Index: flang/lib/Optimizer/CodeGen/CodeGen.cpp
===================================================================
--- flang/lib/Optimizer/CodeGen/CodeGen.cpp
+++ flang/lib/Optimizer/CodeGen/CodeGen.cpp
@@ -727,8 +727,10 @@
   mlir::LogicalResult
   matchAndRewrite(fir::ConvertOp convert, OpAdaptor adaptor,
                   mlir::ConversionPatternRewriter &rewriter) const override {
-    auto fromTy = convertType(convert.getValue().getType());
-    auto toTy = convertType(convert.getRes().getType());
+    auto fromFirTy = convert.getValue().getType();
+    auto toFirTy = convert.getRes().getType();
+    auto fromTy = convertType(fromFirTy);
+    auto toTy = convertType(toFirTy);
     mlir::Value op0 = adaptor.getOperands()[0];
     if (fromTy == toTy) {
       rewriter.replaceOp(convert, op0);
@@ -750,8 +752,7 @@
       return rewriter.create<mlir::LLVM::FPExtOp>(loc, toTy, val);
     };
     // Complex to complex conversion.
-    if (fir::isa_complex(convert.getValue().getType()) &&
-        fir::isa_complex(convert.getRes().getType())) {
+    if (fir::isa_complex(fromFirTy) && fir::isa_complex(toFirTy)) {
       // Special case: handle the conversion of a complex such that both the
       // real and imaginary parts are converted together.
       auto zero = mlir::ArrayAttr::get(convert.getContext(),
@@ -773,6 +774,22 @@
                                                              ic, one);
       return mlir::success();
     }
+
+    // Follow UNIX F77 convention for logicals:
+    // 1. underlying integer is not zero => logical is .TRUE.
+    // 2. logical is .TRUE. => set underlying integer to 1.
+    auto i1Type = mlir::IntegerType::get(convert.getContext(), 1);
+    if (fromFirTy.isa<fir::LogicalType>() && toFirTy == i1Type) {
+      mlir::Value zero = genConstantIndex(loc, fromTy, rewriter, 0);
+      rewriter.replaceOpWithNewOp<mlir::LLVM::ICmpOp>(
+          convert, mlir::LLVM::ICmpPredicate::ne, op0, zero);
+      return mlir::success();
+    }
+    if (fromFirTy == i1Type && toFirTy.isa<fir::LogicalType>()) {
+      rewriter.replaceOpWithNewOp<mlir::LLVM::ZExtOp>(convert, toTy, op0);
+      return mlir::success();
+    }
+
     // Floating point to floating point conversion.
     if (isFloatingPointTy(fromTy)) {
       if (isFloatingPointTy(toTy)) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D121200.413741.patch
Type: text/x-patch
Size: 3190 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/flang-commits/attachments/20220308/a8905bb2/attachment.bin>