[Flang} Fix issues in lowering loop variable of io implied do loop

[kiranchandramohan]

Summary

The existing codegen was for loop variables in io implied do loop
was not working correctly for pointer, allocatable variables. Rather
than finding the symbol address directly, use Evaluate Expression to
find the correct symbol and find the expression address from it. This
is similar to the handling for loop variables in fir.do_loop.

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

[clementval]

Can you check why the windows buildbot is failing?

[kiranchandramohan]

@clementval I have no idea. I have not changed the result operation (fir.result). Not sure what is going wrong here and why it is different from linux. Posting the failure below in case @schweitz or @jeanPerier or you have any idea.

******************** TEST 'Flang :: Lower/io-implied-do-fixes.f90' FAILED ********************
Script:
--
: 'RUN: at line 1';   bbc -emit-fir C:\ws\w3\llvm-project\premerge-checks\flang\test\Lower\io-implied-do-fixes.f90 -o - | c:\ws\w3\llvm-project\premerge-checks\build\bin\filecheck.exe C:\ws\w3\llvm-project\premerge-checks\flang\test\Lower\io-implied-do-fixes.f90
--
Exit Code: 2
 
Command Output (stdout):
--
$ ":" "RUN: at line 1"
$ "bbc" "-emit-fir" "C:\ws\w3\llvm-project\premerge-checks\flang\test\Lower\io-implied-do-fixes.f90" "-o" "-"
# command stderr:
error: loc("C:\\ws\\w3\\llvm-project\\premerge-checks\\flang\\test\\Lower\\io-implied-do-fixes.f90":46:3): 'fir.result' op types mismatch between result op and its parent
FATAL: verification of lowering to FIR failed
error: command failed with exit status: 1
$ "c:\ws\w3\llvm-project\premerge-checks\build\bin\filecheck.exe" "C:\ws\w3\llvm-project\premerge-checks\flang\test\Lower\io-implied-do-fixes.f90"
# command stderr:
FileCheck error: '<stdin>' is empty.
FileCheck command line:  c:\ws\w3\llvm-project\premerge-checks\build\bin\filecheck.exe C:\ws\w3\llvm-project\premerge-checks\flang\test\Lower\io-implied-do-fixes.f90
 
error: command failed with exit status: 2

[schweitz]

It doesn't look familiar to me, @kiranchandramohan .

[awarzynski]

I am not entirely surprised that IO contains something os-specific (that's what the failure would suggest). But it is hard to pin it down without the actual output, which is impossible without a Windows machine. I'd be OK with marking this particular test as unsupported on Windows and creating a GitHub ticket to track this. While it would be great to make it work on Windows, IMHO upstreaming is of much higher priority for us right now.

Btw, from the logs it seems that only ido3 is failing. Perhaps extract it to a separate file?

One last point, I find "io-implied-do-fixes.f90" to be a bit confusing. Without a bug report (or "before" vs "after" comments) it doesn't really feel like there are any "fixes" in this test file. It simply verifies the lowering of implied DO loops.

flang/test/Lower/io-implied-do-fixes.f90

1

Could add a RUN line with flang-new as well?

4

[nit] Why J_<name> everywhere?

7

What's CVT1?

9

I'm guessing that there's a call to print somewhere here?

12

Could subroutine names have a bit more descriptive name so that it's clear what's unique about each one of them? Alternatively, you could add a comment.

22

Would it be possible to replace %{{.*}} to %{{.*}} step %{{.*}} with some values from print *, (iptr,iptr=1,10)? Basically, something that would make it obvious that one corresponds to the other (might help to change indices in print).

[kiranchandramohan]

This went in as part of D120743, D128634. Created an issue for the failure on Windows https://github.com/llvm/llvm-project/issues/56723.

flang/lib/Lower/IO.cpp

 //===-- IO.cpp -- IO statement lowering -----------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
static void genIoLoop(Fortran::lower::AbstractConverter &converter,
                       bool isFormatted, bool checkResult, mlir::Value &ok,
                       bool inLoop, Fortran::lower::StatementContext &stmtCtx) {
   fir::FirOpBuilder &builder = converter.getFirOpBuilder();
   mlir::Location loc = converter.getCurrentLocation();
   makeNextConditionalOn(builder, loc, checkResult, ok, inLoop);
   const auto &itemList = std::get<0>(ioImpliedDo.t);
   const auto &control = std::get<1>(ioImpliedDo.t);
   const auto &loopSym = *control.name.thing.thing.symbol;
-  mlir::Value loopVar = converter.getSymbolAddress(loopSym);
+  mlir::Value loopVar = fir::getBase(converter.genExprAddr(
+      Fortran::evaluate::AsGenericExpr(loopSym).value(), stmtCtx));
   auto genControlValue = [&](const Fortran::parser::ScalarIntExpr &expr) {
     mlir::Value v = fir::getBase(
         converter.genExprValue(*Fortran::semantics::GetExpr(expr), stmtCtx));
     return builder.createConvert(loc, builder.getIndexType(), v);
   };
   mlir::Value lowerValue = genControlValue(control.lower);
   mlir::Value upperValue = genControlValue(control.upper);
   mlir::Value stepValue =
else
                         ok, /*inLoop=*/true, loopCtx);
   };
   if (!checkResult) {
     // No IO call result checks - the loop is a fir.do_loop op.
     auto doLoopOp = builder.create<fir::DoLoopOp>(
         loc, lowerValue, upperValue, stepValue, /*unordered=*/false,
         /*finalCountValue=*/true);
     builder.setInsertionPointToStart(doLoopOp.getBody());
-    mlir::Value lcv = builder.createConvert(loc, converter.genType(loopSym),
-                                            doLoopOp.getInductionVar());
+    mlir::Value lcv = builder.createConvert(
+        loc, fir::unwrapRefType(loopVar.getType()), doLoopOp.getInductionVar());
     builder.create<fir::StoreOp>(loc, lcv, loopVar);
     genItemList(ioImpliedDo);
     builder.setInsertionPointToEnd(doLoopOp.getBody());
     mlir::Value result = builder.create<mlir::arith::AddIOp>(
         loc, doLoopOp.getInductionVar(), doLoopOp.getStep());
     builder.create<fir::ResultOp>(loc, result);
     builder.setInsertionPointAfter(doLoopOp);
     // The loop control variable may be used after the loop.
-    lcv = builder.createConvert(loc, converter.genType(loopSym),
+    lcv = builder.createConvert(loc, fir::unwrapRefType(loopVar.getType()),
                                 doLoopOp.getResult(0));
     builder.create<fir::StoreOp>(loc, lcv, loopVar);
     return;
   }
   // Check IO call results - the loop is a fir.iterate_while op.
   if (!ok)
     ok = builder.createBool(loc, true);
   auto iterWhileOp = builder.create<fir::IterWhileOp>(
       loc, lowerValue, upperValue, stepValue, ok, /*finalCountValue*/ true);
   builder.setInsertionPointToStart(iterWhileOp.getBody());
-  mlir::Value lcv = builder.createConvert(loc, converter.genType(loopSym),
-                                          iterWhileOp.getInductionVar());
+  mlir::Value lcv =
+      builder.createConvert(loc, fir::unwrapRefType(loopVar.getType()),
   builder.create<fir::StoreOp>(loc, lcv, loopVar);
   ok = iterWhileOp.getIterateVar();
   mlir::Value falseValue =
       builder.createIntegerConstant(loc, builder.getI1Type(), 0);
   genItemList(ioImpliedDo);
   // Unwind nested IO call scopes, filling in true and false ResultOp's.
   for (mlir::Operation *op = builder.getBlock()->getParentOp();
        mlir::isa<fir::IfOp>(op); op = op->getBlock()->getParentOp()) {
auto inductionResult1 = builder.create<mlir::arith::AddIOp>(
       loc, inductionResult0, iterWhileOp.getStep());
   auto inductionResult = builder.create<mlir::arith::SelectOp>(
       loc, iterateResult, inductionResult1, inductionResult0);
   llvm::SmallVector<mlir::Value> results = {inductionResult, iterateResult};
   builder.create<fir::ResultOp>(loc, results);
   ok = iterWhileOp.getResult(1);
   builder.setInsertionPointAfter(iterWhileOp);
   // The loop control variable may be used after the loop.
-  lcv = builder.createConvert(loc, converter.genType(loopSym),
+  lcv = builder.createConvert(loc, fir::unwrapRefType(loopVar.getType()),
                               iterWhileOp.getResult(0));
   builder.create<fir::StoreOp>(loc, lcv, loopVar);
 }
 
 //===----------------------------------------------------------------------===//
 // Default argument generation.
 //===----------------------------------------------------------------------===//
 

flang/test/Lower/io-implied-do-fixes.f90

+! RUN: bbc -emit-fir %s -o - | FileCheck %s
+
+! CHECK-LABEL: func @_QPido1
+! CHECK: %[[J_REF_ADDR:.*]] = fir.alloca !fir.ptr<i32> {uniq_name = "_QFido1Eiptr.addr"}
+! CHECK: %[[J_ADDR:.*]] = fir.load %[[J_REF_ADDR]] : !fir.ref<!fir.ptr<i32>>
+! CHECK: %[[J_VAL_FINAL:.*]] = fir.do_loop %[[J_VAL:.*]] = %{{.*}} to %{{.*}} step %{{.*}} -> index {
+! CHECK:   %[[J_VAL_CVT1:.*]] = fir.convert %[[J_VAL]] : (index) -> i32
+! CHECK:   fir.store %[[J_VAL_CVT1]] to %[[J_ADDR]] : !fir.ptr<i32>
+! CHECK: }
+! CHECK: %[[J_VAL_CVT2:.*]] = fir.convert %[[J_VAL_FINAL]] : (index) -> i32
+! CHECK: fir.store %[[J_VAL_CVT2]] to %[[J_ADDR]] : !fir.ptr<i32>
+subroutine ido1
+  integer, pointer :: iptr
+  integer, target :: itgt
+  iptr => itgt
+  print *, (iptr,iptr=1,10)
+end subroutine
+
+! CHECK-LABEL: func @_QPido2
+! CHECK: %[[J_REF_ADDR:.*]] = fir.alloca !fir.heap<i32> {uniq_name = "_QFido2Eiptr.addr"}
+! CHECK: %[[J_ADDR:.*]] = fir.load %[[J_REF_ADDR]] : !fir.ref<!fir.heap<i32>>
+! CHECK: %[[J_VAL_FINAL:.*]] = fir.do_loop %[[J_VAL:.*]] = %{{.*}} to %{{.*}} step %{{.*}} -> index {
+! CHECK: %[[J_VAL_CVT1:.*]] = fir.convert %[[J_VAL]] : (index) -> i32
+! CHECK: fir.store %[[J_VAL_CVT1]] to %[[J_ADDR]] : !fir.heap<i32>
+! CHECK: }
+! CHECK: %[[J_VAL_CVT2:.*]] = fir.convert %[[J_VAL_FINAL]] : (index) -> i32
+! CHECK: fir.store %[[J_VAL_CVT2]] to %[[J_ADDR]] : !fir.heap<i32>
+subroutine ido2
+  integer, allocatable :: iptr
+  allocate(iptr)
+  print *, (iptr,iptr=1,10)
+end subroutine
+
+! CHECK-LABEL: func @_QPido3
+! CHECK:  %[[J_REF_ADDR:.*]] = fir.alloca !fir.heap<i32> {uniq_name = "_QFido3Ej.addr"}
+! CHECK:  %[[J_ADDR:.*]] = fir.load %[[J_REF_ADDR]] : !fir.ref<!fir.heap<i32>>
+! CHECK:  %[[J_VAL_FINAL:.*]]:2 = fir.iterate_while (%[[J_VAL:.*]] = %{{.*}} to %{{.*}} step %{{.*}}) and ({{.*}}) -> (index, i1) {
+! CHECK:    %[[J_VAL_CVT1:.*]] = fir.convert %[[J_VAL]] : (index) -> i32
+! CHECK:    fir.store %[[J_VAL_CVT1]] to %[[J_ADDR]] : !fir.heap<i32>
+! CHECK:  }
+! CHECK:  %[[J_VAL_CVT2:.*]] = fir.convert %[[J_VAL_FINAL]]#0 : (index) -> i32
+! CHECK:  fir.store %[[J_VAL_CVT2]] to %[[J_ADDR]] : !fir.heap<i32
+subroutine ido3
+  integer, allocatable :: j
+  allocate(j)
+  write(*,*,err=404) (j,j=1,10)
+404 continue
+end subroutine