[flang-commits] [flang] 038261f - [flang][OpenMP] Fix subtle bug in GetAffectedNestDepthWithReason (#190645)
via flang-commits
flang-commits at lists.llvm.org
Tue Apr 7 06:03:39 PDT 2026
Author: Krzysztof Parzyszek
Date: 2026-04-07T08:03:33-05:00
New Revision: 038261fa44ced36d05df146451c4db75766bf0fd
URL: https://github.com/llvm/llvm-project/commit/038261fa44ced36d05df146451c4db75766bf0fd
DIFF: https://github.com/llvm/llvm-project/commit/038261fa44ced36d05df146451c4db75766bf0fd.diff
LOG: [flang][OpenMP] Fix subtle bug in GetAffectedNestDepthWithReason (#190645)
For constructs that allow COLLAPSE or ORDERED clauses, the function
would return an empty value for the affected depth if none of these
clauses were actually present. What should happen is that the return
value should be 1 without a specific reason.
This bug was not detectable with any source program, since the empty
value caused depth checks to be skipped. Detecting the problem would
require a loop nest with a lower depth than needed that the bug would
cause not to be diagnosed. Since the correct value was 1, such a loop
would need to have a depth of 0 and such a nest cannot be constructed.
Issue: https://github.com/llvm/llvm-project/issues/185287
Added:
flang/unittests/Semantics/CMakeLists.txt
flang/unittests/Semantics/OpenMPUtils.cpp
Modified:
flang/lib/Semantics/openmp-utils.cpp
flang/unittests/CMakeLists.txt
Removed:
################################################################################
diff --git a/flang/lib/Semantics/openmp-utils.cpp b/flang/lib/Semantics/openmp-utils.cpp
index 33707f9d0d95b..1d9de49e6cd8e 100644
--- a/flang/lib/Semantics/openmp-utils.cpp
+++ b/flang/lib/Semantics/openmp-utils.cpp
@@ -850,17 +850,24 @@ std::pair<WithReason<int64_t>, bool> GetAffectedNestDepthWithReason(
dir, llvm::omp::Clause::OMPC_ordered, version)};
if (allowsCollapse || allowsOrdered) {
- auto [count, reason]{GetArgumentValueWithReason(
+ auto [ccount, creason]{GetArgumentValueWithReason(
spec, llvm::omp::Clause::OMPC_collapse, version)};
- auto [vo, ro]{GetArgumentValueWithReason(
+ auto [ocount, oreason]{GetArgumentValueWithReason(
spec, llvm::omp::Clause::OMPC_ordered, version)};
- if (vo) {
- if (!count || *count < *vo) {
- count = vo;
- reason = std::move(ro);
- }
+ // Ignore invalid arguments.
+ if (ccount <= 0) {
+ ccount = std::nullopt;
+ creason = Reason();
+ }
+ if (ocount <= 0) {
+ ocount = std::nullopt;
+ oreason = Reason();
+ }
+ if (ccount < ocount) {
+ // `ocount` cannot be std::nullopt here (C++ std guarantee).
+ return {{ocount.value_or(1), std::move(oreason)}, true};
}
- return {{count, std::move(reason)}, true};
+ return {{ccount.value_or(1), std::move(creason)}, true};
}
if (IsLoopTransforming(dir)) {
diff --git a/flang/unittests/CMakeLists.txt b/flang/unittests/CMakeLists.txt
index 2d612e58dae24..64cd5c0446474 100644
--- a/flang/unittests/CMakeLists.txt
+++ b/flang/unittests/CMakeLists.txt
@@ -60,3 +60,4 @@ add_subdirectory(Common)
add_subdirectory(Decimal)
add_subdirectory(Evaluate)
add_subdirectory(Frontend)
+add_subdirectory(Semantics)
diff --git a/flang/unittests/Semantics/CMakeLists.txt b/flang/unittests/Semantics/CMakeLists.txt
new file mode 100644
index 0000000000000..ea090b887d717
--- /dev/null
+++ b/flang/unittests/Semantics/CMakeLists.txt
@@ -0,0 +1,32 @@
+set(LLVM_LINK_COMPONENTS
+ ${LLVM_TARGETS_TO_BUILD}
+ Core
+ FrontendOpenACC
+ FrontendOpenMP
+ TargetParser
+)
+
+add_flang_unittest(OpenMPUtilsTests
+ OpenMPUtils.cpp
+)
+
+target_link_libraries(OpenMPUtilsTests
+ PRIVATE
+ flangFrontend
+ flangFrontendTool
+ FortranLower
+ FortranParser
+ FortranSemantics
+ FortranSupport
+ FortranEvaluate
+)
+
+clang_target_link_libraries(OpenMPUtilsTests
+ PRIVATE
+ clangBasic
+)
+
+mlir_target_link_libraries(OpenMPUtilsTests
+ PRIVATE
+ MLIRIR
+)
diff --git a/flang/unittests/Semantics/OpenMPUtils.cpp b/flang/unittests/Semantics/OpenMPUtils.cpp
new file mode 100644
index 0000000000000..32664388ef4d0
--- /dev/null
+++ b/flang/unittests/Semantics/OpenMPUtils.cpp
@@ -0,0 +1,259 @@
+//===- unittests/Semantics/OpenMPUtils.cpp OpenMP utilities tests --------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "flang/Frontend/CompilerInstance.h"
+#include "flang/Frontend/CompilerInvocation.h"
+#include "flang/Frontend/FrontendOptions.h"
+#include "flang/FrontendTool/Utils.h"
+#include "flang/Parser/parse-tree.h"
+#include "flang/Parser/parsing.h"
+#include "flang/Semantics/openmp-utils.h"
+#include "flang/Semantics/semantics.h"
+
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/TargetSelect.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/TargetParser/Host.h"
+#include "llvm/TargetParser/Triple.h"
+
+#include "gtest/gtest.h"
+
+using namespace Fortran;
+using namespace Fortran::frontend;
+
+namespace {
+
+// This is a copy of FrontendActionTest.
+
+class OpenMPUtilsTest : public ::testing::Test {
+protected:
+ // AllSources (which is used to manage files inside every compiler
+ // instance), works with paths. So we need a filename and a path for the
+ // input file.
+ // TODO: We could use `-` for inputFilePath, but then we'd need a way to
+ // write to stdin that's then read by AllSources. Ideally, AllSources should
+ // be capable of reading from any stream.
+ std::string inputFileName;
+ std::string inputFilePath;
+ // The output stream for the input file. Use this to populate the input.
+ std::unique_ptr<llvm::raw_fd_ostream> inputFileOs;
+
+ std::error_code ec;
+
+ CompilerInstance compInst;
+ std::shared_ptr<CompilerInvocation> invoc;
+
+ void SetUp() override {
+ // Generate a unique test file name.
+ const testing::TestInfo *const testInfo =
+ testing::UnitTest::GetInstance()->current_test_info();
+ inputFileName = std::string(testInfo->name()) + "_test-file.f90";
+
+ // Create the input file stream. Note that this stream is populated
+ // separately in every test (i.e. the input is test specific).
+ inputFileOs = std::make_unique<llvm::raw_fd_ostream>(
+ inputFileName, ec, llvm::sys::fs::OF_None);
+ if (ec)
+ FAIL() << "Failed to create the input file";
+
+ // Get the path of the input file.
+ llvm::SmallString<256> cwd;
+ if (std::error_code ec = llvm::sys::fs::current_path(cwd))
+ FAIL() << "Failed to obtain the current working directory";
+ inputFilePath = cwd.c_str();
+ inputFilePath += "/" + inputFileName;
+
+ // Prepare the compiler (CompilerInvocation + CompilerInstance)
+ compInst.createDiagnostics();
+ invoc = std::make_shared<CompilerInvocation>();
+
+ // Set-up default target triple and initialize LLVM Targets so that the
+ // target data layout can be passed to the frontend.
+ invoc->getTargetOpts().triple =
+ llvm::Triple::normalize(llvm::sys::getDefaultTargetTriple());
+ invoc->getLangOpts().OpenMPVersion = 60;
+ llvm::InitializeAllTargets();
+ llvm::InitializeAllTargetMCs();
+
+ compInst.setInvocation(std::move(invoc));
+ compInst.getFrontendOpts().inputs.push_back(
+ FrontendInputFile(inputFilePath, Language::Fortran));
+ compInst.getFrontendOpts().features.Enable(common::LanguageFeature::OpenMP);
+ }
+
+ void TearDown() override {
+ // Clear the input file.
+ llvm::sys::fs::remove(inputFileName);
+
+ // Clear the output files.
+ // Note that these tests use an output buffer (as opposed to an output
+ // file), hence there are no physical output files to delete and
+ // `EraseFiles` is set to `false`. Also, some actions (e.g.
+ // `ParseSyntaxOnly`) don't generated output. In such cases there's no
+ // output to clear and `ClearOutputFile` returns immediately.
+ compInst.clearOutputFiles(/*EraseFiles=*/false);
+ }
+};
+
+TEST_F(OpenMPUtilsTest, AffectedNestDepthNoClauses) {
+ // Populate the input file with the pre-defined input and flush it.
+ *inputFileOs << R"(
+ integer :: i
+ !$omp do
+ do i = 1, 10
+ end do
+ end
+ )";
+ inputFileOs.reset();
+
+ // Set-up the action kind.
+ compInst.getInvocation().getFrontendOpts().programAction = ParseSyntaxOnly;
+
+ // Set-up the output stream for the semantic diagnostics.
+ llvm::SmallVector<char, 256> outputDiagBuffer;
+ std::unique_ptr<llvm::raw_pwrite_stream> outputStream(
+ new llvm::raw_svector_ostream(outputDiagBuffer));
+ compInst.setSemaOutputStream(std::move(outputStream));
+
+ // Execute the action.
+ bool success = executeCompilerInvocation(&compInst);
+
+ std::optional<parser::Program> &parseTree{compInst.getParsing().parseTree()};
+ EXPECT_TRUE(parseTree.has_value());
+
+ if (parseTree) {
+ // clang-format off
+ // The AST for the test program is
+ // Program -> ProgramUnit -> MainProgram
+ // | SpecificationPart
+ // | | ImplicitPart ->
+ // | ExecutionPart -> Block
+ // | | ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPLoopConstruct
+ // | | | OmpBeginLoopDirective
+ // | | | | OmpDirectiveName -> llvm::omp::Directive = do
+ // | | | | OmpClauseList ->
+ // | | | | Flags = {}
+ // | | | Block
+ // | | | | ExecutionPartConstruct -> ExecutableConstruct -> DoConstruct
+ // | | | | | NonLabelDoStmt
+ // | | | | | | LoopControl -> LoopBounds
+ // | | | | | | | Scalar -> Name = 'i'
+ // | | | | | | | Scalar -> Expr -> LiteralConstant -> IntLiteralConstant = '1'
+ // | | | | | | | Scalar -> Expr -> LiteralConstant -> IntLiteralConstant = '10'
+ // | | | | | Block
+ // | | | | | EndDoStmt ->
+ // | EndProgramStmt ->
+ // clang-format on
+ auto &mainProgram =
+ parser::UnwrapRef<parser::MainProgram>(parseTree->v.front());
+ auto &body = std::get<parser::ExecutionPart>(mainProgram.t).v;
+ auto &omp = parser::UnwrapRef<parser::OpenMPLoopConstruct>(body.front());
+ auto [depth, mustBePerfect] =
+ semantics::omp::GetAffectedNestDepthWithReason(omp.BeginDir(), 60);
+ EXPECT_TRUE(depth.value.has_value());
+ if (depth) {
+ EXPECT_EQ(*depth.value, 1);
+ }
+ }
+ // Validate the expected output.
+ EXPECT_TRUE(success);
+}
+
+TEST_F(OpenMPUtilsTest, AffectedNestDepthCollapse) {
+ // Populate the input file with the pre-defined input and flush it.
+ *inputFileOs << R"(
+ integer :: i, j
+ !$omp do collapse(2)
+ do i = 1, 10
+ do j = 1, 10
+ end do
+ end do
+ end
+ )";
+ inputFileOs.reset();
+
+ // Set-up the action kind.
+ compInst.getInvocation().getFrontendOpts().programAction = ParseSyntaxOnly;
+
+ // Set-up the output stream for the semantic diagnostics.
+ llvm::SmallVector<char, 256> outputDiagBuffer;
+ std::unique_ptr<llvm::raw_pwrite_stream> outputStream(
+ new llvm::raw_svector_ostream(outputDiagBuffer));
+ compInst.setSemaOutputStream(std::move(outputStream));
+
+ // Execute the action.
+ bool success = executeCompilerInvocation(&compInst);
+
+ std::optional<parser::Program> &parseTree{compInst.getParsing().parseTree()};
+ EXPECT_TRUE(parseTree.has_value());
+
+ if (parseTree) {
+ auto &mainProgram =
+ parser::UnwrapRef<parser::MainProgram>(parseTree->v.front());
+ auto &body = std::get<parser::ExecutionPart>(mainProgram.t).v;
+ auto &omp = parser::UnwrapRef<parser::OpenMPLoopConstruct>(body.front());
+ auto [depth, mustBePerfect] =
+ semantics::omp::GetAffectedNestDepthWithReason(omp.BeginDir(), 60);
+ EXPECT_TRUE(depth.value.has_value());
+ if (depth) {
+ EXPECT_EQ(*depth.value, 2);
+ }
+ }
+ // Validate the expected output.
+ EXPECT_TRUE(success);
+}
+
+TEST_F(OpenMPUtilsTest, AffectedNestDepthCollapseOrdered) {
+ // Populate the input file with the pre-defined input and flush it.
+ *inputFileOs << R"(
+ integer :: i, j, k, m
+ !$omp do collapse(2) ordered(3)
+ do i = 1, 10
+ do j = 1, 10
+ do k = 1, 10
+ do m = 1, 10
+ end do
+ end do
+ end do
+ end do
+ end
+ )";
+ inputFileOs.reset();
+
+ // Set-up the action kind.
+ compInst.getInvocation().getFrontendOpts().programAction = ParseSyntaxOnly;
+
+ // Set-up the output stream for the semantic diagnostics.
+ llvm::SmallVector<char, 256> outputDiagBuffer;
+ std::unique_ptr<llvm::raw_pwrite_stream> outputStream(
+ new llvm::raw_svector_ostream(outputDiagBuffer));
+ compInst.setSemaOutputStream(std::move(outputStream));
+
+ // Execute the action.
+ bool success = executeCompilerInvocation(&compInst);
+
+ std::optional<parser::Program> &parseTree{compInst.getParsing().parseTree()};
+ EXPECT_TRUE(parseTree.has_value());
+
+ if (parseTree) {
+ auto &mainProgram =
+ parser::UnwrapRef<parser::MainProgram>(parseTree->v.front());
+ auto &body = std::get<parser::ExecutionPart>(mainProgram.t).v;
+ auto &omp = parser::UnwrapRef<parser::OpenMPLoopConstruct>(body.front());
+ auto [depth, mustBePerfect] =
+ semantics::omp::GetAffectedNestDepthWithReason(omp.BeginDir(), 60);
+ EXPECT_TRUE(depth.value.has_value());
+ if (depth) {
+ EXPECT_EQ(*depth.value, 3);
+ }
+ }
+ // Validate the expected output.
+ EXPECT_TRUE(success);
+}
+
+} // namespace
More information about the flang-commits
mailing list