[patch, fortran] Introduce -finline-pack

[patch, fortran] Introduce -finline-pack

[Thomas Koenig]

[-- Attachment #1: Type: text/plain, Size: 1177 bytes --]

Hello world,

the attached patch introduces a new option, -finline-pack.

Since the fix for PR88821, we now do inline packing of
arguments (if required) via the scalarizer, instead of
using _gfortran_internal_[un]pack when optimizing, but
not when optimizing for size.

This introduces (really) large performance gains for some test
cases because now the middle end can see through the packing.
On the other hand, for test cases which do a _lot_ of this,
compile time and code size can increase by quite a bit.

So, this patch introduces an option to control that behavior,
so that people can turn it off on a by-file basis if they
don't want it.

OK for trunk?

Regards

	Thomas

Introduce -finline-pack.

2019-12-07  Thomas Koenig  <tkoenig@gcc.gnu.org>

	PR middle-end/91512
	PR fortran/92738
	* invoke.texi: Document -finline-pack.
	* lang.opt: Add -finline-pack.
	* options.c (gfc_post_options): Handle -finline-pack.
	* trans-array.c (gfc_conv_array_parameter): Use flag_inline_pack
	instead of checking for optimize and optimize_size.

2019-12-07  Thomas Koenig  <tkoenig@gcc.gnu.org>

	PR middle-end/91512
	PR fortran/92738
	* gfortran.dg/inline_pack_25.f90: New test.

[-- Attachment #2: p1.diff --]
[-- Type: text/x-patch, Size: 4014 bytes --]

Index: invoke.texi
===================================================================
--- invoke.texi	(Revision 279064)
+++ invoke.texi	(Arbeitskopie)
@@ -192,8 +192,9 @@ and warnings}.
 -ffrontend-loop-interchange -ffrontend-optimize @gol
 -finit-character=@var{n} -finit-integer=@var{n} -finit-local-zero @gol
 -finit-derived -finit-logical=@var{<true|false>} @gol
--finit-real=@var{<zero|inf|-inf|nan|snan>} @gol
--finline-matmul-limit=@var{n} -fmax-array-constructor=@var{n} @gol
+-finit-real=@var{<zero|inf|-inf|nan|snan>}
+-finline-matmul-limit=@var{n} @gol
+-finline-pack -fmax-array-constructor=@var{n} @gol
 -fmax-stack-var-size=@var{n} -fno-align-commons -fno-automatic @gol
 -fno-protect-parens -fno-underscoring -fsecond-underscore @gol
 -fpack-derived -frealloc-lhs -frecursive -frepack-arrays @gol
@@ -1779,6 +1780,34 @@ compiled with the @option{-fshort-enums} option.
 GNU Fortran choose the smallest @code{INTEGER} kind a given
 enumerator set will fit in, and give all its enumerators this kind.
 
+@item -finline-pack
+@opindex @code{finline-pack}
+When passing an assumed-shape argument of a procedure as actual
+argument to an assumed-size or explicit size or as argument to a
+procedure that does not have an explicit interface, the argument may
+have to be packed, that is put into contiguous memory. An example is
+the call to @code{foo} in
+@smallexample
+  subroutine foo(a)
+     real, dimension(*) :: a
+  end subroutine foo
+  subroutine bar(b)
+     real, dimension(:) :: b
+     call foo(b)
+  end subroutine bar
+@end smallexample
+
+When @option{-finline-pack} is in effect, this packing will be
+performed by inline code. This allows for more optimization while
+increasing code size.
+
+@option{-finlie-pack} is implied by any of the @option{-O} options
+except when optimizing for size via @option{-Os}.  If the code
+contains a very large number of argument that have to be packed, code
+size and also compilation time may become excessive.  If that is the
+case, it may be better to disable this option.  Instances of packing
+can be found by using by using @option{-Warray-temporaries}.
+
 @item -fexternal-blas
 @opindex @code{fexternal-blas}
 This option will make @command{gfortran} generate calls to BLAS functions
Index: lang.opt
===================================================================
--- lang.opt	(Revision 279064)
+++ lang.opt	(Arbeitskopie)
@@ -647,6 +647,10 @@ Enum(gfc_init_local_real) String(inf) Value(GFC_IN
 EnumValue
 Enum(gfc_init_local_real) String(-inf) Value(GFC_INIT_REAL_NEG_INF)
 
+finline-pack
+Fortran  Var(flag_inline_pack) Init(-1)
+-finline-pack	Perform argument packing inline
+
 finline-matmul-limit=
 Fortran RejectNegative Joined UInteger Var(flag_inline_matmul_limit) Init(-1)
 -finline-matmul-limit=<n>	Specify the size of the largest matrix for which matmul will be inlined.
Index: options.c
===================================================================
--- options.c	(Revision 279064)
+++ options.c	(Arbeitskopie)
@@ -467,6 +467,11 @@ gfc_post_options (const char **pfilename)
   if (flag_frontend_loop_interchange == -1)
     flag_frontend_loop_interchange = optimize;
 
+  /* Do inline packing by default if optimizing, but not if
+     optimizing for size.  */
+  if (flag_inline_pack == -1)
+    flag_inline_pack = optimize && !optimize_size;
+
   if (flag_max_array_constructor < 65535)
     flag_max_array_constructor = 65535;
 
Index: trans-array.c
===================================================================
--- trans-array.c	(Revision 279064)
+++ trans-array.c	(Arbeitskopie)
@@ -8139,7 +8139,7 @@ gfc_conv_array_parameter (gfc_se * se, gfc_expr *
 	 making the packing and unpacking operation visible to the
 	 optimizers.  */
 
-      if (g77 && optimize && !optimize_size && expr->expr_type == EXPR_VARIABLE
+      if (g77 && flag_inline_pack && expr->expr_type == EXPR_VARIABLE
 	  && !is_pointer (expr) && ! gfc_has_dimen_vector_ref (expr)
 	  && (fsym == NULL || fsym->ts.type != BT_ASSUMED))
 	{

[-- Attachment #3: internal_pack_25.f90 --]
[-- Type: text/x-fortran, Size: 614 bytes --]

! { dg-do compile }
! { dg-options "-fno-inline-pack -O -fdump-tree-original" }
! PR fortran/92738, middle-end/91512
! Check that -fno-inline-pack does indeed suppress inline packing.
module x
  implicit none
contains
  subroutine foo(x)
    real, dimension(:), intent(inout) :: x
    call bar (x, size(x))
  end subroutine foo
  subroutine bar (x, n)
    integer, intent(in) :: n
    real, dimension(n) :: x
    x = -x
  end subroutine bar
end module x
! { dg-final { scan-tree-dump-times "_gfortran_internal_pack" 1 "original" } }
! { dg-final { scan-tree-dump-times "_gfortran_internal_unpack" 1 "original" } }

Re: [patch, fortran] Introduce -finline-pack

[Richard Biener]

On Sat, Dec 7, 2019 at 2:53 PM Thomas Koenig <tkoenig@netcologne.de> wrote:
>
> Hello world,
>
> the attached patch introduces a new option, -finline-pack.
>
> Since the fix for PR88821, we now do inline packing of
> arguments (if required) via the scalarizer, instead of
> using _gfortran_internal_[un]pack when optimizing, but
> not when optimizing for size.
>
> This introduces (really) large performance gains for some test
> cases because now the middle end can see through the packing.
> On the other hand, for test cases which do a _lot_ of this,
> compile time and code size can increase by quite a bit.
>
> So, this patch introduces an option to control that behavior,
> so that people can turn it off on a by-file basis if they
> don't want it.
>
> OK for trunk?

Just as a suggestion, maybe we'd want to extend this
to other intrinsics in future so a -fno-inline-intrinsic=pack[,...]
is more future proof? (I'd inline all intrinsics by default thus
only provide the negative form).  You can avoid the extra
option parsing complexity by only literally adding
-fno-inline-intrinsic=pack for now.

Richard.

> Regards
>
>         Thomas
>
> Introduce -finline-pack.
>
> 2019-12-07  Thomas Koenig  <tkoenig@gcc.gnu.org>
>
>         PR middle-end/91512
>         PR fortran/92738
>         * invoke.texi: Document -finline-pack.
>         * lang.opt: Add -finline-pack.
>         * options.c (gfc_post_options): Handle -finline-pack.
>         * trans-array.c (gfc_conv_array_parameter): Use flag_inline_pack
>         instead of checking for optimize and optimize_size.
>
> 2019-12-07  Thomas Koenig  <tkoenig@gcc.gnu.org>
>
>         PR middle-end/91512
>         PR fortran/92738
>         * gfortran.dg/inline_pack_25.f90: New test.

Re: [patch, fortran] Introduce -finline-pack

[Thomas Koenig]

Hi Richard,

> Just as a suggestion, maybe we'd want to extend this
> to other intrinsics in future so a -fno-inline-intrinsic=pack[,...]
> is more future proof? (I'd inline all intrinsics by default thus
> only provide the negative form).  You can avoid the extra
> option parsing complexity by only literally adding
> -fno-inline-intrinsic=pack for now.

I agree that such an option would make sense, I think this is
something we should consider for gcc 11.

In this instance, your reply shows that the option is poorly named,
because it is actually not about the PACK intrinsic, but the internal
packing that happens for arguments.

Maybe -finline-repack would be a better name? -finline-internal-pack?

Regards

	Thomas

Re: [patch, fortran] Introduce -finline-pack

[Thomas Koenig]

Am 09.12.19 um 17:30 schrieb Thomas Koenig:
> Maybe -finline-repack would be a better name? -finline-internal-pack?

Steve made an excellent suggestion: -finline-arg-packing .

So, OK with that change?

Regards

	Thomas

*Ping* Introduce -finline-arg-packing

[Thomas Koenig]

Am 10.12.19 um 22:22 schrieb Thomas Koenig:
> Steve made an excellent suggestion: -finline-arg-packing .
> 
> So, OK with that change?

In other words, is

https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00485.html

OK with renaming the option to -finline-arg-packing ?

Regards

	Thomas

Re: *Ping* Introduce -finline-arg-packing

[Steve Kargl]

On Sun, Dec 15, 2019 at 07:11:25PM +0100, Thomas Koenig wrote:
> Am 10.12.19 um 22:22 schrieb Thomas Koenig:
> > Steve made an excellent suggestion: -finline-arg-packing .
> > 
> > So, OK with that change?
> 
> In other words, is
> 
> https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00485.html
> 
> OK with renaming the option to -finline-arg-packing ?
> 

I think it's ok.

-- 
Steve

Re: *Ping* Introduce -finline-arg-packing

[Jakub Jelinek]

On Sun, Dec 15, 2019 at 07:11:25PM +0100, Thomas Koenig wrote:
> Am 10.12.19 um 22:22 schrieb Thomas Koenig:
> > Steve made an excellent suggestion: -finline-arg-packing .
> > 
> > So, OK with that change?
> 
> In other words, is
> 
> https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00485.html
> 
> OK with renaming the option to -finline-arg-packing ?

This patch broke:
+FAIL: compiler driver --help=fortran option(s): "^ +-.*[^:.]\$" absent from output: "  -finline-arg-packing        Perform argument packing inline"
That test verifies that the help texts of all options are terminated
with dot or semicolon.

Fixed thusly, tested with make check-gcc RUNTESTFLAGS=help.exp.
I've additionally noticed you've swapped the two ChangeLog entries,
testsuite/ one went into fortran/ and vice versa, swapped that too
(and that is the reason why I'm posting exactly what I've committed
as obvious, rather than ChangeLog entry before patch + patch).

--- fortran/lang.opt	(revision 279686)
+++ fortran/lang.opt	(revision 279687)
@@ -649,7 +649,7 @@ Enum(gfc_init_local_real) String(-inf) V
 
 finline-arg-packing
 Fortran  Var(flag_inline_arg_packing) Init(-1)
--finline-arg-packing	Perform argument packing inline
+-finline-arg-packing	Perform argument packing inline.
 
 finline-matmul-limit=
 Fortran RejectNegative Joined UInteger Var(flag_inline_matmul_limit) Init(-1)
--- testsuite/ChangeLog	(revision 279686)
+++ testsuite/ChangeLog	(revision 279687)
@@ -51,12 +51,7 @@
 
 	PR middle-end/91512
 	PR fortran/92738
-	* invoke.texi: Document -finline-arg-packing.
-	* lang.opt: Add -finline-arg-packing.
-	* options.c (gfc_post_options): Handle -finline-arg-packing.
-	* trans-array.c (gfc_conv_array_parameter): Use
-	flag_inline_arg_packing instead of checking for optimize and
-	optimize_size.
+	* gfortran.dg/inline_pack_25.f90: New test.
 
 2019-12-20  Tobias Burnus  <tobias@codesourcery.com>
 
--- fortran/ChangeLog	(revision 279686)
+++ fortran/ChangeLog	(revision 279687)
@@ -1,8 +1,19 @@
+2019-12-20  Jakub Jelinek  <jakub@redhat.com>
+
+	PR middle-end/91512
+	PR fortran/92738
+	* lang.opt (-finline-arg-packing): Add trailing dot to help text.
+
 2019-12-20  Thomas Koenig  <tkoenig@gcc.gnu.org>
 
 	PR middle-end/91512
 	PR fortran/92738
-	* gfortran.dg/inline_pack_25.f90: New test.
+	* invoke.texi: Document -finline-arg-packing.
+	* lang.opt: Add -finline-arg-packing.
+	* options.c (gfc_post_options): Handle -finline-arg-packing.
+	* trans-array.c (gfc_conv_array_parameter): Use
+	flag_inline_arg_packing instead of checking for optimize and
+	optimize_size.
 
 2019-12-20  Tobias Burnus  <tobias@codesourcery.com>
 


	Jakub

Re: *Ping* Introduce -finline-arg-packing

[Thomas Koenig]

Hi Jakub,

> This patch broke:
> +FAIL: compiler driver --help=fortran option(s): "^ +-.*[^:.]\$" absent from output: "  -finline-arg-packing        Perform argument packing inline"
> That test verifies that the help texts of all options are terminated
> with dot or semicolon.

Thanks, and sorry for the breakage.

Note to self: Try not to forget that dot.

This was not caught with "make check-fortran" from the gcc
build directory. Maybe it would be a good idea to add that
test to check-fortran too.  Does anybody have an idea
how to do that?

Regards

	Thomas