diff mbox series

Fortran/OpenMP: Reject non-scalar 'holds' expr in 'omp assume(s)' [PR107424]

Message ID 6cd99975-646d-a122-d844-c194dce8dbd0@codesourcery.com
State New
Headers show
Series Fortran/OpenMP: Reject non-scalar 'holds' expr in 'omp assume(s)' [PR107424] | expand

Commit Message

Tobias Burnus Jan. 12, 2023, 10:22 a.m. UTC
Rather obvious fix for that ICE.

Comments? If there are none, I will commit it later as obvious.

Tobias
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

Comments

Jakub Jelinek Jan. 12, 2023, 10:39 a.m. UTC | #1
On Thu, Jan 12, 2023 at 11:22:40AM +0100, Tobias Burnus wrote:
> Rather obvious fix for that ICE.
> 
> Comments? If there are none, I will commit it later as obvious.

I think the spec should be clarified, unlike clauses like if, novariants,
nocontext, indirect, final clause operands where we specify the argument
to be expression of logical type and glossary term says that OpenMP logical
expression is scalar expression for C/C++ and scalar logical expression
for Fortran.  But for the holds clause, all we say is that holds clause
isn't inarguable and
"the program guarantees that the listed expression evaluates to true in
the assumption scope. The effect of the clause does not include an
observable evaluation of the expression."
so I think making it clear that holds argument is expression of logical type
would be useful.

That said, the patch is ok, a rank > 1 expression can't be considered to
evaluate to true...

	Jakub
Tobias Burnus Jan. 12, 2023, 12:24 p.m. UTC | #2
First, I messed up the PR number – it should be PR107706.

On 12.01.23 11:39, Jakub Jelinek wrote:
> On Thu, Jan 12, 2023 at 11:22:40AM +0100, Tobias Burnus wrote:
>> Rather obvious fix for that ICE.
>>
>> Comments? If there are none, I will commit it later as obvious.
> I think the spec should be clarified, unlike clauses like if, novariants,
> nocontext, indirect, final clause operands where we specify the argument
> to be expression of logical type and glossary term says that OpenMP logical
> expression [...] But for the holds clause, all we say is that holds clause
> isn't inarguable and [...] that the listed expression evaluates to true in
> the assumption scope. [...]
> so I think making it clear that holds argument is expression of logical type
> would be useful.

Actually, the spec does have (internally) hold-expr = "OpenMP logical
expression" in a JSON file but that does not show up in the generated
PDF. I have now filed an OpenMP spec issue for it (#3453).

> That said, the patch is ok, a rank > 1 expression can't be considered to
> evaluate to true...

Thanks! Committed as r13-5118-g2ce55247a8bf32985a96ed63a7a92d36746723dc
(with the fixed PR number).

Thanks.

-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
diff mbox series

Patch

Fortran/OpenMP: Reject non-scalar 'holds' expr in 'omp assume(s)' [PR107424]

gcc/fortran/ChangeLog:

	PR fortran/107424
	* openmp.cc (gfc_resolve_omp_assumptions): Reject nonscalars.

gcc/testsuite/ChangeLog:

	PR fortran/107424
	* gfortran.dg/gomp/assume-2.f90: Update dg-error.
	* gfortran.dg/gomp/assumes-2.f90: Likewise.
	* gfortran.dg/gomp/assume-5.f90: New test.

 gcc/fortran/openmp.cc                        |  8 +++++---
 gcc/testsuite/gfortran.dg/gomp/assume-2.f90  |  2 +-
 gcc/testsuite/gfortran.dg/gomp/assume-5.f90  | 20 ++++++++++++++++++++
 gcc/testsuite/gfortran.dg/gomp/assumes-2.f90 |  2 +-
 4 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/gcc/fortran/openmp.cc b/gcc/fortran/openmp.cc
index b71ee467c01..916daeb1aa5 100644
--- a/gcc/fortran/openmp.cc
+++ b/gcc/fortran/openmp.cc
@@ -6911,9 +6911,11 @@  void
 gfc_resolve_omp_assumptions (gfc_omp_assumptions *assume)
 {
   for (gfc_expr_list *el = assume->holds; el; el = el->next)
-    if (!gfc_resolve_expr (el->expr) || el->expr->ts.type != BT_LOGICAL)
-	gfc_error ("HOLDS expression at %L must be a logical expression",
-		   &el->expr->where);
+    if (!gfc_resolve_expr (el->expr)
+	|| el->expr->ts.type != BT_LOGICAL
+	|| el->expr->rank != 0)
+      gfc_error ("HOLDS expression at %L must be a scalar logical expression",
+		 &el->expr->where);
 }
 
 
diff --git a/gcc/testsuite/gfortran.dg/gomp/assume-2.f90 b/gcc/testsuite/gfortran.dg/gomp/assume-2.f90
index ca3e04dfe95..dc306a9088a 100644
--- a/gcc/testsuite/gfortran.dg/gomp/assume-2.f90
+++ b/gcc/testsuite/gfortran.dg/gomp/assume-2.f90
@@ -22,6 +22,6 @@  subroutine foo (i, a)
   end if
 !  !$omp end assume  - silence: 'Unexpected !$OMP END ASSUME statement'
 
-  !$omp assume holds (1.0)  ! { dg-error "HOLDS expression at .1. must be a logical expression" }
+  !$omp assume holds (1.0)  ! { dg-error "HOLDS expression at .1. must be a scalar logical expression" }
   !$omp end assume
 end
diff --git a/gcc/testsuite/gfortran.dg/gomp/assume-5.f90 b/gcc/testsuite/gfortran.dg/gomp/assume-5.f90
new file mode 100644
index 00000000000..5c6c00750dd
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/gomp/assume-5.f90
@@ -0,0 +1,20 @@ 
+! PR fortran/107424
+!
+! Contributed by G. Steinmetz
+!
+
+integer function f(i)
+   implicit none
+   !$omp assumes holds(i < g())  ! { dg-error "HOLDS expression at .1. must be a scalar logical expression" }
+   integer, value :: i
+
+   !$omp assume holds(i < g())  ! { dg-error "HOLDS expression at .1. must be a scalar logical expression" }
+   block
+   end block
+   f = 3
+contains
+   function g()
+      integer :: g(2)
+      g = 4
+   end
+end
diff --git a/gcc/testsuite/gfortran.dg/gomp/assumes-2.f90 b/gcc/testsuite/gfortran.dg/gomp/assumes-2.f90
index 729c9737a1c..c8719a86a94 100644
--- a/gcc/testsuite/gfortran.dg/gomp/assumes-2.f90
+++ b/gcc/testsuite/gfortran.dg/gomp/assumes-2.f90
@@ -4,7 +4,7 @@  module m
   !$omp assumes contains(target) holds(x > 0.0)
   !$omp assumes absent(target)
   !$omp assumes holds(0.0)
-! { dg-error "HOLDS expression at .1. must be a logical expression" "" { target *-*-* } .-1 }
+! { dg-error "HOLDS expression at .1. must be a scalar logical expression" "" { target *-*-* } .-1 }
 end module
 
 module m2