https://github.com/ergawy updated https://github.com/llvm/llvm-project/pull/217220
>From 9d9da73b9d4a29b0b3ae832d3b0efcbb14bd96e1 Mon Sep 17 00:00:00 2001 From: ergawy <[email protected]> Date: Tue, 18 Aug 2026 23:21:17 -0700 Subject: [PATCH 1/2] [flang] Do not branch to a FORMAT statement from an assigned GO TO Label analysis already classifies which labeled statements may be named by a statement that branches. Record the positions of those statements in the semantics context and consult it when lowering records the targets of an assigned GO TO, so that a FORMAT statement is not given a target block. A GO TO whose variable holds only a format label now reaches the run-time error instead of branching into the FORMAT statement. The label analysis is built once and shared: AnalyzeLabels() runs the constraint checks first and records the branch targets only if they pass, since a program with fatal errors is not lowered. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> --- flang/include/flang/Semantics/semantics.h | 14 ++++ flang/lib/Lower/PFTBuilder.cpp | 19 ++++- flang/lib/Semantics/resolve-labels.cpp | 28 ++++++- flang/lib/Semantics/resolve-labels.h | 6 +- flang/lib/Semantics/semantics.cpp | 2 +- .../Lower/assigned-goto-format-target.f90 | 75 +++++++++++++++++++ flang/test/Semantics/assign07.f90 | 6 ++ 7 files changed, 141 insertions(+), 9 deletions(-) create mode 100644 flang/test/Lower/assigned-goto-format-target.f90 diff --git a/flang/include/flang/Semantics/semantics.h b/flang/include/flang/Semantics/semantics.h index 28e5ccd9ad409..c41da2302e85b 100644 --- a/flang/include/flang/Semantics/semantics.h +++ b/flang/include/flang/Semantics/semantics.h @@ -377,6 +377,19 @@ class SemanticsContext { // Top-level ProgramTrees are owned by the SemanticsContext for persistence. ProgramTree &SaveProgramTree(ProgramTree &&); + // Label analysis classifies every labeled statement, and only some of those + // classifications may be named by a statement that branches. Lowering needs + // the same distinction when it records the targets of a branch, so the + // positions of the statements that may be branched to are kept here rather + // than being derived a second time from the parse tree. + void RecordBranchTarget(parser::CharBlock statementPosition) { + branchTargets_.insert(statementPosition); + } + + bool IsRecordedBranchTarget(parser::CharBlock statementPosition) const { + return branchTargets_.find(statementPosition) != branchTargets_.end(); + } + private: struct ScopeIndexComparator { bool operator()(parser::CharBlock, parser::CharBlock) const; @@ -389,6 +402,7 @@ class SemanticsContext { const parser::CharBlock &, const Symbol &, parser::MessageFixedText &&); void CheckError(const Symbol &); + std::set<parser::CharBlock, ScopeIndexComparator> branchTargets_; const common::IntrinsicTypeDefaultKinds &defaultKinds_; const common::LanguageFeatureControl &languageFeatures_; const common::LangOptions &langOpts_; diff --git a/flang/lib/Lower/PFTBuilder.cpp b/flang/lib/Lower/PFTBuilder.cpp index c4a16fcc12285..7cce0308742f7 100644 --- a/flang/lib/Lower/PFTBuilder.cpp +++ b/flang/lib/Lower/PFTBuilder.cpp @@ -1051,11 +1051,26 @@ class PFTBuilder { // Mark every possible target of the assigned GO TO so that // wrappability analyses can see any escape from an enclosing // construct. + auto markIfBranchTarget = [&](parser::Label label) { + if (!label) + return; + + auto iter{labelEvaluationMap->find(label)}; + if (iter == labelEvaluationMap->end()) + return; + + lower::pft::Evaluation *target{iter->second}; + if (!target) + return; + + if (semanticsContext.IsRecordedBranchTarget(target->position)) + markBranchTarget(eval, *target); + }; const auto &labelList = std::get<std::list<parser::Label>>(s.t); if (!labelList.empty()) { // Explicit target list: `go to v, (l1, l2, ...)`. for (const auto &label : labelList) - markBranchTarget(eval, label); + markIfBranchTarget(label); } else { // No explicit list (`go to v`): fall back to the set of labels // that have been previously ASSIGN'd to v. @@ -1067,7 +1082,7 @@ class PFTBuilder { auto iter = assignSymbolLabelMap->find(*sym); if (iter != assignSymbolLabelMap->end()) for (auto label : iter->second) - markBranchTarget(eval, label); + markIfBranchTarget(label); } } eval.isUnstructured = true; diff --git a/flang/lib/Semantics/resolve-labels.cpp b/flang/lib/Semantics/resolve-labels.cpp index 8cb7b6417ca4a..e6212a17b1a3a 100644 --- a/flang/lib/Semantics/resolve-labels.cpp +++ b/flang/lib/Semantics/resolve-labels.cpp @@ -1221,8 +1221,8 @@ void CheckAssignConstraints(const SourceStmtList &assigns, CheckAssignTargetConstraints(assigns, labels, context); } -bool CheckConstraints(ParseTreeAnalyzer &&parseTreeAnalysis) { - auto &context{parseTreeAnalysis.ErrorHandler()}; +bool CheckConstraints( + const ParseTreeAnalyzer &parseTreeAnalysis, SemanticsContext &context) { for (const auto &programUnit : parseTreeAnalysis.ProgramUnits()) { const auto &dos{programUnit.doStmtSources}; const auto &branches{programUnit.otherStmtSources}; @@ -1238,7 +1238,27 @@ bool CheckConstraints(ParseTreeAnalyzer &&parseTreeAnalysis) { return !context.AnyFatalError(); } -bool ValidateLabels(SemanticsContext &context, const parser::Program &program) { - return CheckConstraints(LabelAnalysis(context, program)); +// Record the statements that a branch may name, for lowering to consult when +// it records the targets of a branch. Statements are identified by source +// position because a label is only unique within one program unit. +static void RecordBranchTargets( + const ParseTreeAnalyzer &analysis, SemanticsContext &context) { + for (const auto &programUnit : analysis.ProgramUnits()) { + for (const auto &[label, info] : programUnit.targetStmts) { + if (info.labeledStmtClassificationSet.test(TargetStatementEnum::Branch)) { + context.RecordBranchTarget(info.parserCharBlock); + } + } + } +} + +bool AnalyzeLabels(SemanticsContext &context, const parser::Program &program) { + ParseTreeAnalyzer analysis{LabelAnalysis(context, program)}; + if (!CheckConstraints(analysis, context)) { + // The program will not be lowered, so there is nothing to record for. + return false; + } + RecordBranchTargets(analysis, context); + return true; } } // namespace Fortran::semantics diff --git a/flang/lib/Semantics/resolve-labels.h b/flang/lib/Semantics/resolve-labels.h index 296a90305a41e..e56d2e8c04df7 100644 --- a/flang/lib/Semantics/resolve-labels.h +++ b/flang/lib/Semantics/resolve-labels.h @@ -16,10 +16,12 @@ struct Program; namespace Fortran::semantics { class SemanticsContext; -/// \brief Validate the labels in the program +/// \brief Analyze the labels in the program: check them, and record the +/// statements that a branch may name so that lowering can consult the same +/// classification rather than deriving it again. /// \param context semantic context for errors /// \param program the parse tree of the program /// \return true, iff the program's labels pass semantics checks -bool ValidateLabels(SemanticsContext &context, const parser::Program &program); +bool AnalyzeLabels(SemanticsContext &context, const parser::Program &program); } // namespace Fortran::semantics #endif // FORTRAN_SEMANTICS_RESOLVE_LABELS_H_ diff --git a/flang/lib/Semantics/semantics.cpp b/flang/lib/Semantics/semantics.cpp index 33c54c81f8abd..26ec590b84a16 100644 --- a/flang/lib/Semantics/semantics.cpp +++ b/flang/lib/Semantics/semantics.cpp @@ -702,7 +702,7 @@ bool Semantics::Perform() { } } } - if (!(ValidateLabels(context_, program_) && + if (!(AnalyzeLabels(context_, program_) && parser::CanonicalizeDo(program_) && // force line break CanonicalizeAcc(context_.messages(), program_) && CanonicalizeOmp(context_, program_) && CanonicalizeCUDA(program_) && diff --git a/flang/test/Lower/assigned-goto-format-target.f90 b/flang/test/Lower/assigned-goto-format-target.f90 new file mode 100644 index 0000000000000..4d502738f9830 --- /dev/null +++ b/flang/test/Lower/assigned-goto-format-target.f90 @@ -0,0 +1,75 @@ +! RUN: bbc -emit-fir -o - %s | FileCheck %s + +! A FORMAT statement is not a branch target. Branching to a label that was +! ASSIGN'd from one is not conforming, and the program is meant to reach the +! run-time error rather than jump into the FORMAT statement. + +! The only label assigned to j is a FORMAT, so no target survives and no +! branch is generated at all. +! CHECK-LABEL: func.func @_QPfmt_only( +! CHECK: %[[J:.*]] = fir.declare %arg0 +! CHECK: fir.store %c1{{.*}} to %[[J]] +! CHECK-NOT: fir.select +! CHECK-NOT: ^bb +! CHECK: fir.call @_FortranAReportFatalUserError +! CHECK-NEXT: fir.unreachable +subroutine fmt_only(j) + integer :: j + assign 1 to j + go to j +1 format("fmt") +end subroutine + +! Both labels are assigned to j, but only 20 is a branch target. The select +! carries exactly one case, for 20; label 1 does not appear. +! CHECK-LABEL: func.func @_QPfmt_and_real( +! CHECK: %[[J:.*]] = fir.declare %arg0 +! CHECK: fir.store %c1{{.*}} to %[[J]] +! CHECK: fir.store %c20{{.*}} to %[[J]] +! CHECK: %[[V:.*]] = fir.load %[[J]] +! CHECK: fir.select %[[V]] : i32 [20, ^bb[[TGT:[0-9]+]], unit, ^bb[[ERR:[0-9]+]]] +! +! The default destination reports the error and terminates. +! CHECK: ^bb[[ERR]]: +! CHECK: fir.call @_FortranAReportFatalUserError +! CHECK-NEXT: fir.unreachable +! +! The one real target is the PRINT at label 20, which returns normally. +! CHECK: ^bb[[TGT]]: +! CHECK: fir.call @_FortranAioBeginExternalListOutput +! CHECK: return +subroutine fmt_and_real(j) + integer :: j + assign 1 to j + assign 20 to j + go to j +1 format("fmt") +20 print *, "twenty" +end subroutine + +! FORMAT is the only labelled statement that can be assigned and then reach the +! GO TO without being a branch target; every other kind is rejected by semantic +! analysis at the ASSIGN. The labels below are branch targets of four different +! kinds -- an action statement, the statement that begins an IF construct, the +! statement that begins a DO construct, and the END statement of the subroutine +! -- so every one of them survives and appears as a case of the select. Each is +! branched to from the same inclusive scope, and the two construct labels name +! the statement that begins the construct rather than the one that ends it, so +! control enters the construct normally instead of jumping into its interior. +! CHECK-LABEL: func.func @_QPbranch_target_kinds( +! CHECK: fir.select %{{.*}} : i32 [10, ^bb{{[0-9]+}}, 20, ^bb{{[0-9]+}}, 30, ^bb{{[0-9]+}}, 40, ^bb{{[0-9]+}}, unit, ^bb{{[0-9]+}}] +subroutine branch_target_kinds(n) + integer :: n, j + assign 10 to j + assign 20 to j + assign 30 to j + assign 40 to j + go to j +10 continue +20 if (n == 1) then + print *, "a" + end if +30 do while (n > 0) + n = n - 1 + end do +40 end subroutine diff --git a/flang/test/Semantics/assign07.f90 b/flang/test/Semantics/assign07.f90 index 41a99c1d5d0a7..5b522d170b37c 100644 --- a/flang/test/Semantics/assign07.f90 +++ b/flang/test/Semantics/assign07.f90 @@ -26,10 +26,16 @@ subroutine test(n) if (n==1) goto lab(1,666) !ERROR: Label '2' was not found if (n==1) goto lab(1,2) + ! Label 3 is a FORMAT statement in this scope. It can be assigned and + ! used as a format, but naming it in the label list of an assigned GOTO + ! is an error: a FORMAT statement is not a branch target. The diagnostic + ! is reported on the FORMAT statement itself, below. + if (n==1) goto lab(1,3) assign 3 to lab write(*,fmt=lab) ! ok write(*,fmt=implicitlab3) ! ok 1 continue + !ERROR: Label '3' is not a branch target 3 format('yes') end subroutine test end program >From b7afc28365c0fba859dfe9cc53ad7b2e4ef491d1 Mon Sep 17 00:00:00 2001 From: ergawy <[email protected]> Date: Thu, 20 Aug 2026 09:23:08 -0700 Subject: [PATCH 2/2] PR review fixes --- flang/lib/Lower/PFTBuilder.cpp | 23 ++++---- flang/lib/Semantics/resolve-labels.cpp | 2 +- .../Lower/assigned-goto-format-target.f90 | 5 +- .../test/Lower/assigned-goto-labeled-end.f90 | 55 +++++++++++++++++++ 4 files changed, 71 insertions(+), 14 deletions(-) create mode 100644 flang/test/Lower/assigned-goto-labeled-end.f90 diff --git a/flang/lib/Lower/PFTBuilder.cpp b/flang/lib/Lower/PFTBuilder.cpp index 7cce0308742f7..a967073c0e8a9 100644 --- a/flang/lib/Lower/PFTBuilder.cpp +++ b/flang/lib/Lower/PFTBuilder.cpp @@ -1023,10 +1023,14 @@ class PFTBuilder { auto &label = std::get<parser::Label>(s.t); const auto *sym = std::get<parser::Name>(s.t).symbol; assert(sym && "missing AssignStmt symbol"); - lower::pft::Evaluation *target{ - labelEvaluationMap->find(label)->second}; + auto labelIter{labelEvaluationMap->find(label)}; + assert(labelIter != labelEvaluationMap->end() && + "assigned label has no evaluation"); + lower::pft::Evaluation *target{labelIter->second}; assert(target && "missing branch target evaluation"); - if (!target->isA<parser::FormatStmt>()) { + // Consult the same classification the assigned GO TO uses, so the + // two agree on which statements may be branched to. + if (semanticsContext.IsRecordedBranchTarget(target->position)) { target->isNewBlock = true; for (lower::pft::Evaluation *parent = target->parentConstruct; parent; parent = parent->parentConstruct) { @@ -1052,17 +1056,12 @@ class PFTBuilder { // wrappability analyses can see any escape from an enclosing // construct. auto markIfBranchTarget = [&](parser::Label label) { - if (!label) - return; - + assert(label && "missing branch target label"); auto iter{labelEvaluationMap->find(label)}; - if (iter == labelEvaluationMap->end()) - return; - + assert(iter != labelEvaluationMap->end() && + "branch target label has no evaluation"); lower::pft::Evaluation *target{iter->second}; - if (!target) - return; - + assert(target && "missing branch target evaluation"); if (semanticsContext.IsRecordedBranchTarget(target->position)) markBranchTarget(eval, *target); }; diff --git a/flang/lib/Semantics/resolve-labels.cpp b/flang/lib/Semantics/resolve-labels.cpp index e6212a17b1a3a..d9b9d12a8f3b5 100644 --- a/flang/lib/Semantics/resolve-labels.cpp +++ b/flang/lib/Semantics/resolve-labels.cpp @@ -1255,7 +1255,7 @@ static void RecordBranchTargets( bool AnalyzeLabels(SemanticsContext &context, const parser::Program &program) { ParseTreeAnalyzer analysis{LabelAnalysis(context, program)}; if (!CheckConstraints(analysis, context)) { - // The program will not be lowered, so there is nothing to record for. + // The program will not be lowered, so there is nothing to record. return false; } RecordBranchTargets(analysis, context); diff --git a/flang/test/Lower/assigned-goto-format-target.f90 b/flang/test/Lower/assigned-goto-format-target.f90 index 4d502738f9830..4b6895e43266f 100644 --- a/flang/test/Lower/assigned-goto-format-target.f90 +++ b/flang/test/Lower/assigned-goto-format-target.f90 @@ -57,7 +57,10 @@ subroutine fmt_and_real(j) ! the statement that begins the construct rather than the one that ends it, so ! control enters the construct normally instead of jumping into its interior. ! CHECK-LABEL: func.func @_QPbranch_target_kinds( -! CHECK: fir.select %{{.*}} : i32 [10, ^bb{{[0-9]+}}, 20, ^bb{{[0-9]+}}, 30, ^bb{{[0-9]+}}, 40, ^bb{{[0-9]+}}, unit, ^bb{{[0-9]+}}] +! CHECK: fir.select %{{.*}} : i32 [ +! CHECK-SAME: 10, ^bb{{[0-9]+}}, 20, ^bb{{[0-9]+}}, +! CHECK-SAME: 30, ^bb{{[0-9]+}}, 40, ^bb{{[0-9]+}}, +! CHECK-SAME: unit, ^bb{{[0-9]+}}] subroutine branch_target_kinds(n) integer :: n, j assign 10 to j diff --git a/flang/test/Lower/assigned-goto-labeled-end.f90 b/flang/test/Lower/assigned-goto-labeled-end.f90 new file mode 100644 index 0000000000000..1642560ccea0c --- /dev/null +++ b/flang/test/Lower/assigned-goto-labeled-end.f90 @@ -0,0 +1,55 @@ +! RUN: bbc -pft-test -o - %s | FileCheck %s + +! The END statement of a program unit may carry a label and is a branch target +! (F2023 11.2.1 p1 lists end-subroutine-stmt, end-function-stmt and +! end-program-stmt). Label analysis visits that statement in advance, before +! the statement visitor reaches it, so its recorded position has to be supplied +! explicitly; otherwise lowering fails to recognise it and the assigned GO TO +! silently loses the edge. These checks pin the behaviour at the PFT layer, +! where the target is marked -- the generated select alone cannot see it, +! because ASSIGN also marks blocks for the labels it names. + +! Two units in one file, each with a labeled END. The second unit's END must +! not be recorded under the first unit's position. +! CHECK-LABEL: Subroutine two_first: +! CHECK: AssignedGotoStmt! -> [[E1:[0-9]+]]: go to j +! CHECK: [[E1]] ^EndSubroutineStmt: 41 end subroutine +subroutine two_first(j) + integer :: j + assign 41 to j + go to j +41 end subroutine + +! CHECK-LABEL: Subroutine two_second: +! CHECK: AssignedGotoStmt! -> [[E2:[0-9]+]]: go to j +! CHECK: [[E2]] ^EndSubroutineStmt: 42 end subroutine +subroutine two_second(j) + integer :: j + assign 42 to j + go to j +42 end subroutine + +! An explicit label list naming a labeled END that is never ASSIGN'd. Nothing +! marks the target except the GO TO itself, so this shape depends only on the +! recorded classification. +! CHECK-LABEL: Subroutine listed_end: +! CHECK: AssignedGotoStmt! -> [[E3:[0-9]+]]: go to j,(43) +! CHECK: [[E3]] ^EndSubroutineStmt: 43 end subroutine +subroutine listed_end(j) + integer :: j + go to j, (43) +43 end subroutine + +! A labeled END of an internal subprogram, reached through the host's CONTAINS. +! CHECK-LABEL: Subroutine inner: +! CHECK: AssignedGotoStmt! -> [[E4:[0-9]+]]: go to j +! CHECK: [[E4]] ^EndSubroutineStmt: 44 end subroutine +program host + call inner(1) +contains + subroutine inner(j) + integer :: j + assign 44 to j + go to j +44 end subroutine +end program _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
