fortran: Correctly evaluate the MASK argument of MINLOC/MAXLOC

Message ID 20240713094818.399842-1-morin-mikael@orange.fr
State Committed
Commit d211100903d4d532d989451243ea00d7fa2e9d5e
Headers
Series fortran: Correctly evaluate the MASK argument of MINLOC/MAXLOC |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Test passed

Commit Message

Mikael Morin July 13, 2024, 9:48 a.m. UTC
  From: Mikael Morin <mikael@gcc.gnu.org>

Hello,

I'm currently testing this on x86_64-linux.
I plan to push to master if all goes well.

Mikael

-- 8< --

Add the preliminary code that the generated expression for MASK may depend
on when generating the inline code to evaluate MINLOC or MAXLOC with a
scalar MASK.

The generated code was only keeping the generated expression but not the
preliminary code, which was sufficient for simple cases such as data
references or simple (scalar) function calls, but was bogus with more
complicated ones.

gcc/fortran/ChangeLog:

	* trans-intrinsic.cc (gfc_conv_intrinsic_minmaxloc): Add the
	preliminary code generated for MASK to the preliminary code of
	MINLOC/MAXLOC.

gcc/testsuite/ChangeLog:

	* gfortran.dg/minmaxloc_17.f90: New test.
---
 gcc/fortran/trans-intrinsic.cc             |  1 +
 gcc/testsuite/gfortran.dg/minmaxloc_17.f90 | 33 ++++++++++++++++++++++
 2 files changed, 34 insertions(+)
 create mode 100644 gcc/testsuite/gfortran.dg/minmaxloc_17.f90
  

Comments

Paul Richard Thomas July 14, 2024, 6:26 a.m. UTC | #1
Hi Mikael,

The fix is blindingly obvious :-) Not only that, the failing testcase runs
correctly.

OK for mainline and please backport to 14-branch before the 14.2 release.

Thanks for the patch

Paul


On Sat, 13 Jul 2024 at 10:48, Mikael Morin <morin-mikael@orange.fr> wrote:

> From: Mikael Morin <mikael@gcc.gnu.org>
>
> Hello,
>
> I'm currently testing this on x86_64-linux.
> I plan to push to master if all goes well.
>
> Mikael
>
> -- 8< --
>
> Add the preliminary code that the generated expression for MASK may depend
> on when generating the inline code to evaluate MINLOC or MAXLOC with a
> scalar MASK.
>
> The generated code was only keeping the generated expression but not the
> preliminary code, which was sufficient for simple cases such as data
> references or simple (scalar) function calls, but was bogus with more
> complicated ones.
>
> gcc/fortran/ChangeLog:
>
>         * trans-intrinsic.cc (gfc_conv_intrinsic_minmaxloc): Add the
>         preliminary code generated for MASK to the preliminary code of
>         MINLOC/MAXLOC.
>
> gcc/testsuite/ChangeLog:
>
>         * gfortran.dg/minmaxloc_17.f90: New test.
> ---
>  gcc/fortran/trans-intrinsic.cc             |  1 +
>  gcc/testsuite/gfortran.dg/minmaxloc_17.f90 | 33 ++++++++++++++++++++++
>  2 files changed, 34 insertions(+)
>  create mode 100644 gcc/testsuite/gfortran.dg/minmaxloc_17.f90
>
> diff --git a/gcc/fortran/trans-intrinsic.cc
> b/gcc/fortran/trans-intrinsic.cc
> index cadbd177452..180d0d7a88c 100644
> --- a/gcc/fortran/trans-intrinsic.cc
> +++ b/gcc/fortran/trans-intrinsic.cc
> @@ -5749,6 +5749,7 @@ gfc_conv_intrinsic_minmaxloc (gfc_se * se, gfc_expr
> * expr, enum tree_code op)
>
>        gfc_init_se (&maskse, NULL);
>        gfc_conv_expr_val (&maskse, maskexpr);
> +      gfc_add_block_to_block (&se->pre, &maskse.pre);
>        gfc_init_block (&block);
>        gfc_add_block_to_block (&block, &loop.pre);
>        gfc_add_block_to_block (&block, &loop.post);
> diff --git a/gcc/testsuite/gfortran.dg/minmaxloc_17.f90
> b/gcc/testsuite/gfortran.dg/minmaxloc_17.f90
> new file mode 100644
> index 00000000000..7e6e586ab03
> --- /dev/null
> +++ b/gcc/testsuite/gfortran.dg/minmaxloc_17.f90
> @@ -0,0 +1,33 @@
> +! { dg-do run }
> +!
> +! Check that the code necessary to evaluate MINLOC's or MAXLOC's MASK
> +! argument is correctly generated.
> +
> +program p
> +  implicit none
> +  integer, parameter :: data10(*) = (/ 2, 5, 2, 0, 6, 5, 3, 6, 0, 1 /)
> +  logical, parameter :: mask10(*) = (/ .false., .true., .false., &
> +                                       .false., .true., .true.,  &
> +                                       .true. , .true., .false., &
> +                                       .false. /)
> +  type bool_wrapper
> +    logical :: l
> +  end type
> +  call check_minloc
> +  call check_maxloc
> +contains
> +  subroutine check_minloc
> +    integer :: a(10)
> +    integer :: r
> +    a = data10
> +    r = minloc(a, dim = 1, mask = sum(a) > 0)
> +    if (r /= 4) stop 11
> +  end subroutine
> +  subroutine check_maxloc
> +    integer :: a(10)
> +    integer :: r
> +    a = data10
> +    r = maxloc(a, dim = 1, mask = sum(a) > 0)
> +    if (r /= 5) stop 18
> +  end subroutine
> +end program
> --
> 2.43.0
>
>
  

Patch

diff --git a/gcc/fortran/trans-intrinsic.cc b/gcc/fortran/trans-intrinsic.cc
index cadbd177452..180d0d7a88c 100644
--- a/gcc/fortran/trans-intrinsic.cc
+++ b/gcc/fortran/trans-intrinsic.cc
@@ -5749,6 +5749,7 @@  gfc_conv_intrinsic_minmaxloc (gfc_se * se, gfc_expr * expr, enum tree_code op)
 
       gfc_init_se (&maskse, NULL);
       gfc_conv_expr_val (&maskse, maskexpr);
+      gfc_add_block_to_block (&se->pre, &maskse.pre);
       gfc_init_block (&block);
       gfc_add_block_to_block (&block, &loop.pre);
       gfc_add_block_to_block (&block, &loop.post);
diff --git a/gcc/testsuite/gfortran.dg/minmaxloc_17.f90 b/gcc/testsuite/gfortran.dg/minmaxloc_17.f90
new file mode 100644
index 00000000000..7e6e586ab03
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/minmaxloc_17.f90
@@ -0,0 +1,33 @@ 
+! { dg-do run }
+!
+! Check that the code necessary to evaluate MINLOC's or MAXLOC's MASK
+! argument is correctly generated.
+
+program p
+  implicit none
+  integer, parameter :: data10(*) = (/ 2, 5, 2, 0, 6, 5, 3, 6, 0, 1 /)
+  logical, parameter :: mask10(*) = (/ .false., .true., .false., &
+                                       .false., .true., .true.,  &
+                                       .true. , .true., .false., &
+                                       .false. /)
+  type bool_wrapper
+    logical :: l
+  end type
+  call check_minloc
+  call check_maxloc
+contains
+  subroutine check_minloc
+    integer :: a(10)
+    integer :: r
+    a = data10
+    r = minloc(a, dim = 1, mask = sum(a) > 0)
+    if (r /= 4) stop 11
+  end subroutine
+  subroutine check_maxloc
+    integer :: a(10)
+    integer :: r
+    a = data10
+    r = maxloc(a, dim = 1, mask = sum(a) > 0)
+    if (r /= 5) stop 18
+  end subroutine
+end program