[Frontend][OpenMP] Reduction modifier must be applied somewhere

[kparzysz]

Detect the case when a reduction modifier ends up not being applied after construct decomposition, treat it as an error.

This fixes a regression in the gfortran test suite after #90098.

[llvmbot]

@llvm/pr-subscribers-flang-fir-hlfir

@llvm/pr-subscribers-flang-openmp

Author: Krzysztof Parzyszek (kparzysz)

Changes

Detect the case when a reduction modifier ends up not being applied after construct decomposition, treat it as an error.

This fixes a regression in the gfortran test suite after PR90098.

---

Full diff: https://github.com/llvm/llvm-project/pull/92160.diff

1 Files Affected:

• (modified) llvm/include/llvm/Frontend/OpenMP/ConstructDecompositionT.h (+2-1)

diff --git a/llvm/include/llvm/Frontend/OpenMP/ConstructDecompositionT.h b/llvm/include/llvm/Frontend/OpenMP/ConstructDecompositionT.h
index 5f12c62b832fc..e5fcc493ea590 100644
--- a/llvm/include/llvm/Frontend/OpenMP/ConstructDecompositionT.h
+++ b/llvm/include/llvm/Frontend/OpenMP/ConstructDecompositionT.h
@@ -930,7 +930,8 @@ bool ConstructDecompositionT<C, H>::applyClause(
       // Apply clause without modifier.
       leaf.clauses.push_back(unmodified);
     }
-    applied = true;
+    // The modifier must be applied to some construct.
+    applied = effectiveApplied;
   }
 
   if (!applied)

[tblah]

If it isn't too time consuming, please could you add a test for this to the flang test suite so that we catch this bug with the regular build bots next time.

[tblah]

Thanks for adding the test

[psteinfeld]

@kparzysz, I'm still getting an assertion in Decomposer.cpp with my internal test even with this latest change. Are you still working on this, or should I work to create a small reproducer?

[kparzysz]

@kparzysz, I'm still getting an assertion in Decomposer.cpp with my internal test even with this latest change. Are you still working on this, or should I work to create a small reproducer?

This test uses an invalid reduction modifier for this construct. Before this fix we just silently dropped the modifier, now we will assert. I will open a follow-up issue to have this detected in the semantic checks and emit a user-friendly error.

Does your code have this specific combination of reduction/construct? IIRC, it asserted before any of the follow up fixes, so it's likely a different problem. Please do create a testcase for it.

Edit: @psteinfeld In case you missed my previous message: please send me a testcase for the crash you're seeing.

[kparzysz]

Created issue #92397, and PR #92406.