[patch] Tame fix for PR ipa/99122

[patch] Tame fix for PR ipa/99122

[Eric Botcazou]

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

Hi,

as explained in the audit trail, the return part has a major performance 
impact in Ada where variable-sized types are first-class citizens, but it 
turns out that it is not exercized in the testsuite yet.

Tested on x86-64/Linux, OK for mainline and 11 branch?


2021-06-02  Eric Botcazou  <ebotcazou@adacore.com>

	PR ipa/99122
	* tree-inline.c (inline_forbidden_p): Remove test on return type.


2021-06-02  Eric Botcazou  <ebotcazou@adacore.com>

	* gnat.dg/inline22.adb: New test.

-- 
Eric Botcazou

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

diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
index d38e8617e3d..cc7168614c0 100644
--- a/gcc/tree-inline.c
+++ b/gcc/tree-inline.c
@@ -4004,17 +4004,11 @@ inline_forbidden_p (tree fndecl)
   wi.info = (void *) fndecl;
   wi.pset = &visited_nodes;
 
-  /* We cannot inline a function with a VLA typed argument or result since
-     we have no implementation materializing a variable of such type in
-     the caller.  */
-  if (COMPLETE_TYPE_P (TREE_TYPE (TREE_TYPE (fndecl)))
-      && !poly_int_tree_p (TYPE_SIZE (TREE_TYPE (TREE_TYPE (fndecl)))))
-    {
-      inline_forbidden_reason
-	= G_("function %q+F can never be inlined because "
-	     "it has a VLA return argument");
-      return true;
-    }
+  /* We cannot inline a function with a variable-sized parameter because we
+     cannot materialize a temporary of such a type in the caller if need be.
+
+     Note that the return case is not symmetrical because we can guarantee
+     that a temporary is not needed by means of CALL_EXPR_RETURN_SLOT_OPT.  */
   for (tree parm = DECL_ARGUMENTS (fndecl); parm; parm = DECL_CHAIN (parm))
     if (!poly_int_tree_p (DECL_SIZE (parm)))
       {

[-- Attachment #3: inline22.adb --]
[-- Type: text/x-adasrc, Size: 303 bytes --]

-- { dg-compile }

procedure Inline22 (L, U : Integer) is

  type Arr is array (Integer range L .. U) of Boolean;

  function Get_Zero return Arr;
  pragma Inline_Always (Get_Zero);

  function Get_Zero return Arr is
  begin
    return (others => False);
  end;

  A : Arr;

begin
  A := Get_Zero;
end;

Re: [patch] Tame fix for PR ipa/99122

[Richard Biener]

On Wed, Jun 2, 2021 at 2:05 PM Eric Botcazou <botcazou@adacore.com> wrote:
>
> Hi,
>
> as explained in the audit trail, the return part has a major performance
> impact in Ada where variable-sized types are first-class citizens, but it
> turns out that it is not exercized in the testsuite yet.
>
> Tested on x86-64/Linux, OK for mainline and 11 branch?

Not sure whether we know VLA results are always returned by
reference?  In particular does this mean we'll never see a
WITH_SIZE_EXPR on the LHS of a call?  You might have noticed
I've done WITH_SIZE_EXPR "enhancements" recently on trunk.

As for the patch we indeed didn't have a testcase covering the
return case so it should be safe to revert the return part.

I still wonder about the WITH_SIZE_EXPR here ;)

Richard.

>
> 2021-06-02  Eric Botcazou  <ebotcazou@adacore.com>
>
>         PR ipa/99122
>         * tree-inline.c (inline_forbidden_p): Remove test on return type.
>
>
> 2021-06-02  Eric Botcazou  <ebotcazou@adacore.com>
>
>         * gnat.dg/inline22.adb: New test.
>
> --
> Eric Botcazou

Re: [patch] Tame fix for PR ipa/99122

[Eric Botcazou]

> Not sure whether we know VLA results are always returned by
> reference?  In particular does this mean we'll never see a
> WITH_SIZE_EXPR on the LHS of a call?  You might have noticed
> I've done WITH_SIZE_EXPR "enhancements" recently on trunk.

VLAs are always aggregate_value_p so the caller passes the address of the 
return slot, but they are not always DECL_BY_REFERENCE, at least in Ada.

In Ada we use DECL_BY_REFERENCE only for self-referential types, i.e. types 
whose size is not the same for all the objects, because we need to control the 
amount of returned bytes on a case-by-case basis; when the size is uniform, 
this complication is unnecessary.

> As for the patch we indeed didn't have a testcase covering the
> return case so it should be safe to revert the return part.

OK, thanks, patch installed.

-- 
Eric Botcazou