From f15c43f47ae09ad54ce9e9fe9ef94014cd5a3bc0 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Mon, 24 Nov 2025 08:15:14 -0600 Subject: [PATCH 1/2] [flang][OpenMP] Tolerate compiler directives in loop constructs PR168884 flagged compiler directives (!dir$ ...) inside OpenMP loop constructs as errors. This caused some customer applications to fail to compile (issue 169229). Downgrade the error to a warning, and gracefully ignore compiler directives when lowering loop constructs to MLIR. Fixes https://github.com/llvm/llvm-project/issues/169229 --- flang/lib/Lower/OpenMP/Utils.cpp | 32 ++++++++++++++++--- flang/lib/Semantics/check-omp-loop.cpp | 2 +- flang/lib/Semantics/resolve-directives.cpp | 7 ---- .../OpenMP/compiler-directives-loop.f90 | 21 ++++++++++++ .../Semantics/OpenMP/loop-association.f90 | 3 +- 5 files changed, 50 insertions(+), 15 deletions(-) create mode 100644 flang/test/Semantics/OpenMP/compiler-directives-loop.f90 diff --git a/flang/lib/Lower/OpenMP/Utils.cpp b/flang/lib/Lower/OpenMP/Utils.cpp index 7d7a4869ab3a6..4af6fb3e88f84 100644 --- a/flang/lib/Lower/OpenMP/Utils.cpp +++ b/flang/lib/Lower/OpenMP/Utils.cpp @@ -796,6 +796,28 @@ static void processTileSizesFromOpenMPConstruct( } } +static pft::Evaluation *getNestedDoConstruct(pft::Evaluation &eval) { + for (pft::Evaluation &nested : eval.getNestedEvaluations()) { + // In an OpenMPConstruct there can be compiler directives: + // 1 <> + // 2 CompilerDirective: !unroll + // <> -> 8 + if (nested.getIf()) + continue; + // Within a DoConstruct, there can be compiler directives, plus + // there is a DoStmt before the body: + // <> -> 8 + // 3 NonLabelDoStmt -> 7: do i = 1, n + // <> -> 7 + if (nested.getIf()) + continue; + assert(nested.getIf() && + "Unexpected construct in the nested evaluations"); + return &nested; + } + llvm_unreachable("Expected do loop to be in the nested evaluations"); +} + /// Populates the sizes vector with values if the given OpenMPConstruct /// contains a loop construct with an inner tiling construct. void collectTileSizesFromOpenMPConstruct( @@ -818,7 +840,7 @@ int64_t collectLoopRelatedInfo( int64_t numCollapse = 1; // Collect the loops to collapse. - lower::pft::Evaluation *doConstructEval = &eval.getFirstNestedEvaluation(); + lower::pft::Evaluation *doConstructEval = getNestedDoConstruct(eval); if (doConstructEval->getIf()->IsDoConcurrent()) { TODO(currentLocation, "Do Concurrent in Worksharing loop construct"); } @@ -844,7 +866,8 @@ void collectLoopRelatedInfo( fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder(); // Collect the loops to collapse. - lower::pft::Evaluation *doConstructEval = &eval.getFirstNestedEvaluation(); + lower::pft::Evaluation *doConstructEval = getNestedDoConstruct(eval); + assert(doConstructEval && "Expected do loop to be in the nested evaluation"); if (doConstructEval->getIf()->IsDoConcurrent()) { TODO(currentLocation, "Do Concurrent in Worksharing loop construct"); } @@ -885,9 +908,8 @@ void collectLoopRelatedInfo( iv.push_back(bounds->name.thing.symbol); loopVarTypeSize = std::max(loopVarTypeSize, bounds->name.thing.symbol->GetUltimate().size()); - collapseValue--; - doConstructEval = - &*std::next(doConstructEval->getNestedEvaluations().begin()); + if (--collapseValue) + doConstructEval = getNestedDoConstruct(*doConstructEval); } while (collapseValue > 0); convertLoopBounds(converter, currentLocation, result, loopVarTypeSize); diff --git a/flang/lib/Semantics/check-omp-loop.cpp b/flang/lib/Semantics/check-omp-loop.cpp index 6d83ee62c3b73..ef237a01d0f7a 100644 --- a/flang/lib/Semantics/check-omp-loop.cpp +++ b/flang/lib/Semantics/check-omp-loop.cpp @@ -267,7 +267,7 @@ void OmpStructureChecker::CheckNestedBlock(const parser::OpenMPLoopConstruct &x, for (auto &stmt : body) { if (auto *dir{parser::Unwrap(stmt)}) { context_.Say(dir->source, - "Compiler directives are not allowed inside OpenMP loop constructs"_err_en_US); + "Compiler directives are not allowed inside OpenMP loop constructs"_warn_en_US); } else if (parser::Unwrap(stmt)) { ++nestedCount; } else if (auto *omp{parser::Unwrap(stmt)}) { diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp index 673ea4ef10e08..b992a4125ffcb 100644 --- a/flang/lib/Semantics/resolve-directives.cpp +++ b/flang/lib/Semantics/resolve-directives.cpp @@ -2419,13 +2419,6 @@ void OmpAttributeVisitor::PrivatizeAssociatedLoopIndexAndCheckLoopLevel( } } CheckAssocLoopLevel(level, GetAssociatedClause()); - } else { - unsigned version{context_.langOptions().OpenMPVersion}; - context_.Say(GetContext().directiveSource, - "A DO loop must follow the %s directive"_err_en_US, - parser::ToUpperCaseLetters( - llvm::omp::getOpenMPDirectiveName(GetContext().directive, version) - .str())); } } } diff --git a/flang/test/Semantics/OpenMP/compiler-directives-loop.f90 b/flang/test/Semantics/OpenMP/compiler-directives-loop.f90 new file mode 100644 index 0000000000000..48b9529c95dc2 --- /dev/null +++ b/flang/test/Semantics/OpenMP/compiler-directives-loop.f90 @@ -0,0 +1,21 @@ +!RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=60 %s -o - | FileCheck %s + +! Check that this compiles successfully, but not rely on any specific output. + +!CHECK: omp.parallel + +program omp_cdir_crash + implicit none + integer, parameter :: n = 10 + real :: a(n) + integer :: i + +!$omp parallel do +!dir$ unroll + do i = 1, n + a(i) = real(i) + end do +!$omp end parallel do + + print *, 'a(1)=', a(1), ' a(n)=', a(n) +end program omp_cdir_crash diff --git a/flang/test/Semantics/OpenMP/loop-association.f90 b/flang/test/Semantics/OpenMP/loop-association.f90 index 9c79a91429fdf..4e63cafb3fda1 100644 --- a/flang/test/Semantics/OpenMP/loop-association.f90 +++ b/flang/test/Semantics/OpenMP/loop-association.f90 @@ -33,10 +33,9 @@ END DO outer ! Accept directives between parallel do and actual loop. - !ERROR: A DO loop must follow the PARALLEL DO directive !$OMP PARALLEL DO !WARNING: Unrecognized compiler directive was ignored [-Wignored-directive] - !ERROR: Compiler directives are not allowed inside OpenMP loop constructs + !WARNING: Compiler directives are not allowed inside OpenMP loop constructs !DIR$ VECTOR ALIGNED DO 20 i=1,N a = a + 0.5 From 1c3f18fa4619af8f58695f944046fcf8ef94d72d Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Mon, 24 Nov 2025 09:56:53 -0600 Subject: [PATCH 2/2] Remove leftover assertion --- flang/lib/Lower/OpenMP/Utils.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/flang/lib/Lower/OpenMP/Utils.cpp b/flang/lib/Lower/OpenMP/Utils.cpp index 4af6fb3e88f84..fed84eb4df071 100644 --- a/flang/lib/Lower/OpenMP/Utils.cpp +++ b/flang/lib/Lower/OpenMP/Utils.cpp @@ -867,7 +867,6 @@ void collectLoopRelatedInfo( // Collect the loops to collapse. lower::pft::Evaluation *doConstructEval = getNestedDoConstruct(eval); - assert(doConstructEval && "Expected do loop to be in the nested evaluation"); if (doConstructEval->getIf()->IsDoConcurrent()) { TODO(currentLocation, "Do Concurrent in Worksharing loop construct"); }