From fc5c9b9e469cca348031f6a70c4117a5cd8268d7 Mon Sep 17 00:00:00 2001 From: Sergi Siso Date: Tue, 15 Jul 2025 12:25:55 +0100 Subject: [PATCH 1/3] #3056 Demote OpenMP lowering error to warning --- src/psyclone/psyir/nodes/omp_directives.py | 12 ++++--- .../tests/psyir/nodes/omp_directives_test.py | 34 +++++++++---------- 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/src/psyclone/psyir/nodes/omp_directives.py b/src/psyclone/psyir/nodes/omp_directives.py index 891b620bb6..da55f2de56 100644 --- a/src/psyclone/psyir/nodes/omp_directives.py +++ b/src/psyclone/psyir/nodes/omp_directives.py @@ -46,6 +46,7 @@ import abc import itertools import sympy +import logging from psyclone.configuration import Config from psyclone.core import AccessType @@ -1384,11 +1385,12 @@ def lower_to_language_level(self): clause.children]: break else: - raise GenerationError( - f"Lowering '{type(self).__name__}' does not support " - f"symbols that need synchronisation unless they are " - f"in a depend clause, but found: " - f"'{sym.name}' which is not in a depend clause.") + logger = logging.getLogger(__name__) + logger.warning( + "Lowering '%s' detected a possible race condition for " + "symbol '%s'. Make sure this is a false WaW dependency" + " or the code includes the necessary " + "synchronisations.", type(self).__name__, sym.name) self.addchild(private_clause) self.addchild(fprivate_clause) diff --git a/src/psyclone/tests/psyir/nodes/omp_directives_test.py b/src/psyclone/tests/psyir/nodes/omp_directives_test.py index 8bf6245627..7186a68b6b 100644 --- a/src/psyclone/tests/psyir/nodes/omp_directives_test.py +++ b/src/psyclone/tests/psyir/nodes/omp_directives_test.py @@ -77,7 +77,7 @@ "gocean1p0") -def test_ompparallel_lowering(fortran_reader, monkeypatch): +def test_ompparallel_lowering(fortran_reader, monkeypatch, caplog): ''' Check that lowering an OMP Parallel region leaves it with the appropriate begin_string and clauses for the backend to generate the right code''' @@ -140,11 +140,11 @@ def test_ompparallel_lowering(fortran_reader, monkeypatch): a_sym = Symbol("a") monkeypatch.setattr(pdir, "infer_sharing_attributes", lambda: ({}, {}, {a_sym})) - with pytest.raises(GenerationError) as err: - pdir.lower_to_language_level() - assert ("Lowering 'OMPParallelDirective' does not support symbols that " - "need synchronisation unless they are in a depend clause, but " - "found: 'a' which is not in a depend clause." in str(err.value)) + pdir.lower_to_language_level() + assert ("Lowering 'OMPParallelDirective' detected a possible race " + "condition for symbol 'a'. Make sure this is a false WaW " + "dependency or the code includes the necessary synchronisations." + "\n" in caplog.text) # Also a case which contains the symbol in an input dependency clause. task_dir = OMPTaskDirective() @@ -157,14 +157,14 @@ def test_ompparallel_lowering(fortran_reader, monkeypatch): task_dir.addchild(in_clause) pdir.children[0].addchild(task_dir) - with pytest.raises(GenerationError) as err: - pdir.lower_to_language_level() - assert ("Lowering 'OMPParallelDirective' does not support symbols that " - "need synchronisation unless they are in a depend clause, but " - "found: 'a' which is not in a depend clause." in str(err.value)) + pdir.lower_to_language_level() + assert ("Lowering 'OMPParallelDirective' detected a possible race " + "condition for symbol 'a'. Make sure this is a false WaW " + "dependency or the code includes the necessary synchronisations." + "\n" in caplog.text) -def test_omp_parallel_do_lowering(fortran_reader, monkeypatch): +def test_omp_parallel_do_lowering(fortran_reader, monkeypatch, caplog): ''' Check that lowering an OMP Parallel Do leaves it with the appropriate begin_string and clauses for the backend to generate the right code''' @@ -230,11 +230,11 @@ def test_omp_parallel_do_lowering(fortran_reader, monkeypatch): # Monkeypatch a case with shared variables that need synchronisation monkeypatch.setattr(pdir, "infer_sharing_attributes", lambda: ({}, {}, {Symbol("a")})) - with pytest.raises(GenerationError) as err: - pdir.lower_to_language_level() - assert ("Lowering 'OMPParallelDoDirective' does not support symbols that " - "need synchronisation unless they are in a depend clause, but " - "found: 'a' which is not in a depend clause." in str(err.value)) + pdir.lower_to_language_level() + assert ("Lowering 'OMPParallelDoDirective' detected a possible race " + "condition for symbol 'a'. Make sure this is a false WaW " + "dependency or the code includes the necessary synchronisations." + "\n" in caplog.text) def test_omp_teams_distribute_parallel_do_strings( From a7433a7fcd1865878bcbdd8e9443fdd4f0ac085e Mon Sep 17 00:00:00 2001 From: Sergi Siso Date: Tue, 15 Jul 2025 14:09:03 +0100 Subject: [PATCH 2/3] #3056 Fix caplog test by using a mandatory context manager --- src/psyclone/tests/psyir/nodes/omp_directives_test.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/psyclone/tests/psyir/nodes/omp_directives_test.py b/src/psyclone/tests/psyir/nodes/omp_directives_test.py index 7186a68b6b..0e7aa0ef60 100644 --- a/src/psyclone/tests/psyir/nodes/omp_directives_test.py +++ b/src/psyclone/tests/psyir/nodes/omp_directives_test.py @@ -41,6 +41,7 @@ import os import pytest +import logging from psyclone.errors import UnresolvedDependencyError from psyclone.parse.algorithm import parse from psyclone.psyGen import PSyFactory @@ -140,7 +141,8 @@ def test_ompparallel_lowering(fortran_reader, monkeypatch, caplog): a_sym = Symbol("a") monkeypatch.setattr(pdir, "infer_sharing_attributes", lambda: ({}, {}, {a_sym})) - pdir.lower_to_language_level() + with caplog.at_level(logging.WARNING): + pdir.lower_to_language_level() assert ("Lowering 'OMPParallelDirective' detected a possible race " "condition for symbol 'a'. Make sure this is a false WaW " "dependency or the code includes the necessary synchronisations." @@ -157,7 +159,8 @@ def test_ompparallel_lowering(fortran_reader, monkeypatch, caplog): task_dir.addchild(in_clause) pdir.children[0].addchild(task_dir) - pdir.lower_to_language_level() + with caplog.at_level(logging.WARNING): + pdir.lower_to_language_level() assert ("Lowering 'OMPParallelDirective' detected a possible race " "condition for symbol 'a'. Make sure this is a false WaW " "dependency or the code includes the necessary synchronisations." @@ -230,7 +233,8 @@ def test_omp_parallel_do_lowering(fortran_reader, monkeypatch, caplog): # Monkeypatch a case with shared variables that need synchronisation monkeypatch.setattr(pdir, "infer_sharing_attributes", lambda: ({}, {}, {Symbol("a")})) - pdir.lower_to_language_level() + with caplog.at_level(logging.WARNING): + pdir.lower_to_language_level() assert ("Lowering 'OMPParallelDoDirective' detected a possible race " "condition for symbol 'a'. Make sure this is a false WaW " "dependency or the code includes the necessary synchronisations." From 7476b926734c4886115959702cc1d7e8369cad26 Mon Sep 17 00:00:00 2001 From: LonelyCat124 <3043914+LonelyCat124@users.noreply.github.com.> Date: Wed, 23 Jul 2025 14:40:47 +0100 Subject: [PATCH 3/3] #3056 update changelog --- changelog | 2 ++ 1 file changed, 2 insertions(+) diff --git a/changelog b/changelog index b0a13a4e5b..b2f319dead 100644 --- a/changelog +++ b/changelog @@ -1,3 +1,5 @@ + 58) PR #3057 for 3056. Demote OpenMP lowering error to warning. + 57) PR #3018 for #3017. Fix ArrayType with defined lower bounds, but no upper bound.