diff --git a/flang/include/flang/Lower/PFTBuilder.h b/flang/include/flang/Lower/PFTBuilder.h index 388efbe0ed4d1..65ff0b2c2416d 100644 --- a/flang/include/flang/Lower/PFTBuilder.h +++ b/flang/include/flang/Lower/PFTBuilder.h @@ -169,9 +169,9 @@ static constexpr bool isIntermediateConstructStmt{common::HasMember< template static constexpr bool isNopConstructStmt{common::HasMember< - A, std::tuple>}; + A, std::tuple>}; template static constexpr bool isExecutableDirective{common::HasMember< @@ -276,9 +276,8 @@ struct Evaluation : EvaluationVariant { /// from one or more enclosing constructs. Evaluation &nonNopSuccessor() const { Evaluation *successor = lexicalSuccessor; - if (successor && successor->isNopConstructStmt()) { + if (successor && successor->isNopConstructStmt()) successor = successor->parentConstruct->constructExit; - } assert(successor && "missing successor"); return *successor; } diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp index 76f1696bdee94..fb94218b606ae 100644 --- a/flang/lib/Lower/Bridge.cpp +++ b/flang/lib/Lower/Bridge.cpp @@ -85,29 +85,6 @@ struct IncrementLoopInfo { bool isStructured() const { return !headerBlock; } - /// \return true if for this do loop its do-variable's value - /// is represented as the block argument of the do loop's - /// region. In this case the data type of the block argument - /// matches the original data type of the do-variable as written - /// in user code, and the value is adjusted using the step value - /// on each iteration of the do loop. - /// - /// When do-variable's data type is an integer type shorter - /// than IndexType, processing the do-variable separately - /// from the do loop's iteration index allows getting rid - /// of type casts, which can make backend optimizations easier. - /// In particular, computing the do variable value from - /// the iteration index may introduce chains like trunc->arith->sext, - /// which may be optimized into sequences of shift operations - /// in InstCombine, which then prevents vectorizer from recognizing - /// unit-strided accesses. - /// - /// We could have disabled the extra iteration variable usage - /// for cases when its data type is not shorter than IndexType, - /// but this requires having proper DataLayout set up for the enclosing - /// module. This is currently blocked by llvm-project#57230 issue. - bool doVarIsALoopArg() const { return isStructured() && !isUnordered; } - mlir::Type getLoopVariableType() const { assert(loopVariable && "must be set"); return fir::unwrapRefType(loopVariable.getType()); @@ -1393,16 +1370,25 @@ class FirConverter : public Fortran::lower::AbstractConverter { if (!infiniteLoop && !whileCondition) genFIRIncrementLoopBegin(incrementLoopNestInfo); - // Loop body code - NonLabelDoStmt and EndDoStmt code is generated here. - // Their genFIR calls are nops except for block management in some cases. - for (Fortran::lower::pft::Evaluation &e : eval.getNestedEvaluations()) - genFIR(e, unstructuredContext); + // Loop body code. + auto iter = eval.getNestedEvaluations().begin(); + for (auto end = --eval.getNestedEvaluations().end(); iter != end; ++iter) + genFIR(*iter, unstructuredContext); + + // An EndDoStmt in unstructured code may start a new block. + Fortran::lower::pft::Evaluation &endDoEval = *iter; + assert(endDoEval.getIf() && "no enddo stmt"); + if (unstructuredContext) + maybeStartBlock(endDoEval.block); // Loop end code. if (infiniteLoop || whileCondition) genFIRBranch(headerBlock); else genFIRIncrementLoopEnd(incrementLoopNestInfo); + + // This call may generate a branch in some contexts. + genFIR(endDoEval, unstructuredContext); } /// Generate FIR to begin a structured or unstructured increment loop nest. @@ -1450,28 +1436,26 @@ class FirConverter : public Fortran::lower::AbstractConverter { // Structured loop - generate fir.do_loop. if (info.isStructured()) { - mlir::Value doVarInit = nullptr; - if (info.doVarIsALoopArg()) - doVarInit = builder->createConvert(loc, info.getLoopVariableType(), - lowerValue); - - info.doLoop = builder->create( - loc, lowerValue, upperValue, info.stepValue, info.isUnordered, - /*finalCountValue=*/!info.isUnordered, - doVarInit ? mlir::ValueRange{doVarInit} : mlir::ValueRange{}); - builder->setInsertionPointToStart(info.doLoop.getBody()); - mlir::Value value; - if (!doVarInit) { - // Update the loop variable value, as it may have non-index - // references. - value = builder->createConvert(loc, info.getLoopVariableType(), - info.doLoop.getInductionVar()); + mlir::Type loopVarType = info.getLoopVariableType(); + mlir::Value loopValue; + if (info.isUnordered) { + // The loop variable value is explicitly updated. + info.doLoop = builder->create( + loc, lowerValue, upperValue, info.stepValue, /*unordered=*/true); + builder->setInsertionPointToStart(info.doLoop.getBody()); + loopValue = builder->createConvert(loc, loopVarType, + info.doLoop.getInductionVar()); } else { - // The loop variable value is the region's argument rather - // than the DoLoop's index value. - value = info.doLoop.getRegionIterArgs()[0]; + // The loop variable is a doLoop op argument. + info.doLoop = builder->create( + loc, lowerValue, upperValue, info.stepValue, /*unordered=*/false, + /*finalCountValue=*/true, + builder->createConvert(loc, loopVarType, lowerValue)); + builder->setInsertionPointToStart(info.doLoop.getBody()); + loopValue = info.doLoop.getRegionIterArgs()[0]; } - builder->create(loc, value, info.loopVariable); + // Update the loop variable value in case it has non-index references. + builder->create(loc, loopValue, info.loopVariable); if (info.maskExpr) { Fortran::lower::StatementContext stmtCtx; mlir::Value maskCond = createFIRExpr(loc, info.maskExpr, stmtCtx); @@ -1559,40 +1543,28 @@ class FirConverter : public Fortran::lower::AbstractConverter { IncrementLoopInfo &info = *it; if (info.isStructured()) { // End fir.do_loop. - if (!info.isUnordered) { - builder->setInsertionPointToEnd(info.doLoop.getBody()); - llvm::SmallVector results; - results.push_back(builder->create( - loc, info.doLoop.getInductionVar(), info.doLoop.getStep())); - if (info.doVarIsALoopArg()) { - // If we use an extra iteration variable of the same data - // type as the original do-variable, we have to increment - // it by the step value. Note that the step has 'index' - // type, so we need to cast it, first. - mlir::Value stepCast = builder->createConvert( - loc, info.getLoopVariableType(), info.doLoop.getStep()); - mlir::Value doVarValue = - builder->create(loc, info.loopVariable); - results.push_back(builder->create( - loc, doVarValue, stepCast)); - } - builder->create(loc, results); - } - builder->setInsertionPointAfter(info.doLoop); - if (info.isUnordered) + if (info.isUnordered) { + builder->setInsertionPointAfter(info.doLoop); continue; - // The loop control variable may be used after loop execution. - mlir::Value lcv = nullptr; - if (info.doVarIsALoopArg()) { - // Final do-variable value is the second result of the DoLoop. - assert(info.doLoop.getResults().size() == 2 && - "invalid do-variable handling"); - lcv = info.doLoop.getResult(1); - } else { - lcv = builder->createConvert(loc, info.getLoopVariableType(), - info.doLoop.getResult(0)); } - builder->create(loc, lcv, info.loopVariable); + // Decrement tripVariable. + builder->setInsertionPointToEnd(info.doLoop.getBody()); + llvm::SmallVector results; + results.push_back(builder->create( + loc, info.doLoop.getInductionVar(), info.doLoop.getStep())); + // Step loopVariable to help optimizations such as vectorization. + // Induction variable elimination will clean up as necessary. + mlir::Value step = builder->createConvert( + loc, info.getLoopVariableType(), info.doLoop.getStep()); + mlir::Value loopVar = + builder->create(loc, info.loopVariable); + results.push_back( + builder->create(loc, loopVar, step)); + builder->create(loc, results); + builder->setInsertionPointAfter(info.doLoop); + // The loop control variable may be used after the loop. + builder->create(loc, info.doLoop.getResult(1), + info.loopVariable); continue; } @@ -1645,6 +1617,7 @@ class FirConverter : public Fortran::lower::AbstractConverter { ¤tIfOp.getElseRegion().front()); } else if (e.isA()) { builder->setInsertionPointAfter(topIfOp); + genFIR(e, /*unstructuredContext=*/false); // may generate branch } else { genFIR(e, /*unstructuredContext=*/false); } @@ -2220,6 +2193,8 @@ class FirConverter : public Fortran::lower::AbstractConverter { getEval().getNestedEvaluations()) { if (auto *selectTypeStmt = eval.getIf()) { + // A genFIR(SelectTypeStmt) call would have unwanted side effects. + maybeStartBlock(eval.block); // Retrieve the selector const auto &s = std::get(selectTypeStmt->t); if (const auto *v = std::get_if(&s.u)) @@ -2280,6 +2255,7 @@ class FirConverter : public Fortran::lower::AbstractConverter { eval.getIf()) { // Map the type guard local symbol for the selector to a more precise // typed entity in the TypeGuardStmt when necessary. + genFIR(eval); const auto &guard = std::get(typeGuardStmt->t); if (hasLocalScope) @@ -2385,11 +2361,13 @@ class FirConverter : public Fortran::lower::AbstractConverter { builder->restoreInsertionPoint(crtInsPt); ++typeGuardIdx; } else if (eval.getIf()) { + genFIR(eval); if (hasLocalScope) localSymbols.popScope(); stmtCtx.finalize(); + } else { + genFIR(eval); } - genFIR(eval); } } @@ -3104,6 +3082,10 @@ class FirConverter : public Fortran::lower::AbstractConverter { } // Nop statements - No code, or code is generated at the construct level. + // But note that the genFIR call immediately below that wraps one of these + // calls does block management, possibly starting a new block, and possibly + // generating a branch to end a block. So these calls may still be required + // for that functionality. void genFIR(const Fortran::parser::AssociateStmt &) {} // nop void genFIR(const Fortran::parser::CaseStmt &) {} // nop void genFIR(const Fortran::parser::ContinueStmt &) {} // nop @@ -3124,36 +3106,42 @@ class FirConverter : public Fortran::lower::AbstractConverter { void genFIR(const Fortran::parser::SelectTypeStmt &) {} // nop void genFIR(const Fortran::parser::TypeGuardStmt &) {} // nop - /// Generate FIR for the Evaluation `eval`. + /// Generate FIR for Evaluation \p eval. void genFIR(Fortran::lower::pft::Evaluation &eval, bool unstructuredContext = true) { - if (unstructuredContext) { - // When transitioning from unstructured to structured code, - // the structured code could be a target that starts a new block. + // Start a new unstructured block when applicable. When transitioning + // from unstructured to structured code, unstructuredContext is true, + // which accounts for the possibility that the structured code could be + // a target that starts a new block. + if (unstructuredContext) maybeStartBlock(eval.isConstruct() && eval.lowerAsStructured() ? eval.getFirstNestedEvaluation().block : eval.block); - } + // Generate evaluation specific code. Even nop calls should usually reach + // here in case they start a new block or require generation of a generic + // end-of-block branch. An alternative is to add special case code + // elsewhere, such as in the genFIR code for a parent construct. setCurrentEval(eval); setCurrentPosition(eval.position); eval.visit([&](const auto &stmt) { genFIR(stmt); }); - if (unstructuredContext && blockIsUnterminated()) { - // Exit from an unstructured IF or SELECT construct block. - Fortran::lower::pft::Evaluation *successor{}; - if (eval.isActionStmt()) - successor = eval.controlSuccessor; - else if (eval.isConstruct() && - eval.getLastNestedEvaluation() - .lexicalSuccessor->isIntermediateConstructStmt()) - successor = eval.constructExit; - else if (eval.isConstructStmt() && - eval.lexicalSuccessor == eval.controlSuccessor) - // empty construct block - successor = eval.parentConstruct->constructExit; - if (successor && successor->block) - genFIRBranch(successor->block); + // Generate an end-of-block branch for several special cases. For + // constructs, this can be done for either the end construct statement, + // or for the construct itself, which will skip this code if the + // end statement was visited first and generated a branch. + Fortran::lower::pft::Evaluation *successor = + eval.isConstruct() ? eval.getLastNestedEvaluation().lexicalSuccessor + : eval.lexicalSuccessor; + if (successor && blockIsUnterminated()) { + if (successor->isIntermediateConstructStmt() && + successor->parentConstruct->lowerAsUnstructured()) + // Exit from an intermediate unstructured IF or SELECT construct block. + genFIRBranch(successor->parentConstruct->constructExit->block); + else if (unstructuredContext && eval.isConstructStmt() && + successor == eval.controlSuccessor) + // Exit from a degenerate, empty construct block. + genFIRBranch(eval.parentConstruct->constructExit->block); } } diff --git a/flang/lib/Lower/PFTBuilder.cpp b/flang/lib/Lower/PFTBuilder.cpp index 5c5b94ce93396..a5f1ed7fb0fda 100644 --- a/flang/lib/Lower/PFTBuilder.cpp +++ b/flang/lib/Lower/PFTBuilder.cpp @@ -824,7 +824,7 @@ class PFTBuilder { lastConstructStmtEvaluation = &eval; }, [&](const parser::EndSelectStmt &) { - eval.nonNopSuccessor().isNewBlock = true; + eval.isNewBlock = true; lastConstructStmtEvaluation = nullptr; }, [&](const parser::ChangeTeamStmt &s) { @@ -924,32 +924,31 @@ class PFTBuilder { }, // Constructs - set (unstructured) construct exit targets - [&](const parser::AssociateConstruct &) { setConstructExit(eval); }, + [&](const parser::AssociateConstruct &) { + eval.constructExit = &eval.evaluationList->back(); + }, [&](const parser::BlockConstruct &) { - // EndBlockStmt may have code. eval.constructExit = &eval.evaluationList->back(); }, [&](const parser::CaseConstruct &) { - setConstructExit(eval); + eval.constructExit = &eval.evaluationList->back(); eval.isUnstructured = true; }, [&](const parser::ChangeTeamConstruct &) { - // EndChangeTeamStmt may have code. eval.constructExit = &eval.evaluationList->back(); }, [&](const parser::CriticalConstruct &) { - // EndCriticalStmt may have code. eval.constructExit = &eval.evaluationList->back(); }, [&](const parser::DoConstruct &) { setConstructExit(eval); }, [&](const parser::ForallConstruct &) { setConstructExit(eval); }, [&](const parser::IfConstruct &) { setConstructExit(eval); }, [&](const parser::SelectRankConstruct &) { - setConstructExit(eval); + eval.constructExit = &eval.evaluationList->back(); eval.isUnstructured = true; }, [&](const parser::SelectTypeConstruct &) { - setConstructExit(eval); + eval.constructExit = &eval.evaluationList->back(); eval.isUnstructured = true; }, [&](const parser::WhereConstruct &) { setConstructExit(eval); }, @@ -971,13 +970,6 @@ class PFTBuilder { if (eval.evaluationList) analyzeBranches(&eval, *eval.evaluationList); - // Set the successor of the last statement in an IF or SELECT block. - if (!eval.controlSuccessor && eval.lexicalSuccessor && - eval.lexicalSuccessor->isIntermediateConstructStmt()) { - eval.controlSuccessor = parentConstruct->constructExit; - eval.lexicalSuccessor->isNewBlock = true; - } - // Propagate isUnstructured flag to enclosing construct. if (parentConstruct && eval.isUnstructured) parentConstruct->isUnstructured = true; diff --git a/flang/test/Lower/select-case-statement.f90 b/flang/test/Lower/select-case-statement.f90 index d62e9e2d05f4d..5db675af0d2c6 100644 --- a/flang/test/Lower/select-case-statement.f90 +++ b/flang/test/Lower/select-case-statement.f90 @@ -168,7 +168,7 @@ subroutine scharacter1(s) ! CHECK: %[[V_8:[0-9]+]] = fir.call @_FortranACharacterCompareScalar1 ! CHECK: %[[V_9:[0-9]+]] = arith.cmpi sge, %[[V_8]], %c0{{.*}} : i32 - ! CHECK: cond_br %[[V_9]], ^bb1, ^bb15 + ! CHECK: cond_br %[[V_9]], ^bb1, ^bb16 ! CHECK: ^bb1: // pred: ^bb0 if (lge(s,'00')) then @@ -245,7 +245,7 @@ subroutine scharacter1(s) ! CHECK: ^bb13: // pred: ^bb12 ! CHECK: ^bb14: // 3 preds: ^bb9, ^bb11, ^bb12 ! CHECK: fir.store %c4{{.*}} to %[[V_1]] : !fir.ref - ! CHECK: ^bb15: // 6 preds: ^bb0, ^bb3, ^bb4, ^bb6, ^bb8, ^bb14 + ! CHECK: ^bb15: // 5 preds: ^bb3, ^bb4, ^bb6, ^bb8, ^bb14 end select end if ! CHECK: %[[V_89:[0-9]+]] = fir.load %[[V_1]] : !fir.ref @@ -347,7 +347,7 @@ subroutine sgoto ! CHECK: fir.select_case %[[selector]] : i32 [#fir.upper, %c2{{.*}}, ^bb3, #fir.lower, %c5{{.*}}, ^bb4, unit, ^bb7] ! CHECK: ^bb3: // pred: ^bb2 ! CHECK: arith.muli %c10{{[^0]}} - ! CHECK: br ^bb9 + ! CHECK: br ^bb8 ! CHECK: ^bb4: // pred: ^bb2 ! CHECK: arith.muli %c1000{{[^0]}} ! CHECK: cond_br {{.*}}, ^bb5, ^bb6 @@ -355,14 +355,14 @@ subroutine sgoto ! CHECK: br ^bb8 ! CHECK: ^bb6: // pred: ^bb4 ! CHECK: arith.muli %c10000{{[^0]}} - ! CHECK: br ^bb9 + ! CHECK: br ^bb8 ! CHECK: ^bb7: // pred: ^bb2 ! CHECK: arith.muli %c100{{[^0]}} ! CHECK: br ^bb8 - ! CHECK: ^bb8: // 2 preds: ^bb5, ^bb7 - ! CHECK: br ^bb9 - ! CHECK: ^bb9: // 3 preds: ^bb3, ^bb6, ^bb8 + ! CHECK: ^bb8: // 4 preds: ^bb3, ^bb5, ^bb6, ^bb7 ! CHECK: fir.call @_FortranAioBeginExternalListOutput + ! CHECK: br ^bb1 + ! CHECK: ^bb9: // pred: ^bb1 select case(i) case (:2) n = i * 10