diff mbox

Fix calls.c for a _complex type (PR ipa/80104).

Message ID 5040cf25-9cfb-995c-2442-c4c705533733@suse.cz
State New
Headers show

Commit Message

Martin Liška March 24, 2017, 9:25 a.m. UTC 
Hello.

Briefly described in the PR, running ICF (without any optimization level) can create a thunk call
that does not use an SSA_NAME which is a default def of an argument of the caller:

c (complex float b)
{
  complex float arg.1;
  float retval.0;

  <bb 2> [100.00%]:
  arg.1_2 = b;
  retval.0_4 = a (arg.1_2); [tail call]
  return retval.0_4;

}

The "arg" variable creation was introduced by Jason in r207301.
As complex type is passed as invisible reference, we need to find address of the argument.
Thus I'm suggesting to find it alternatively via SSA_NAME_DEF_STMT.

Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. And fixes the ICE
on s390x cross compiler.

Ready to be installed?
Martin

Comments

Richard Biener March 24, 2017, 11:49 a.m. UTC | #1 
On Fri, Mar 24, 2017 at 10:25 AM, Martin Liška <mliska@suse.cz> wrote:
> Hello.
>
> Briefly described in the PR, running ICF (without any optimization level) can create a thunk call
> that does not use an SSA_NAME which is a default def of an argument of the caller:
>
> c (complex float b)
> {
>   complex float arg.1;
>   float retval.0;
>
>   <bb 2> [100.00%]:
>   arg.1_2 = b;
>   retval.0_4 = a (arg.1_2); [tail call]
>
>   return retval.0_4;
>
> }
>
> The "arg" variable creation was introduced by Jason in r207301.
> As complex type is passed as invisible reference, we need to find address of the argument.
> Thus I'm suggesting to find it alternatively via SSA_NAME_DEF_STMT.
>
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. And fixes the ICE
> on s390x cross compiler.
>
> Ready to be installed?

Ick, that looks like fragile code ...  looks like the
call_from_thunk_p case expects
the default def or the param-decl.  So why not create it that way?  Or
simply not do
this "optimization"?

Richard.

> Martin
Martin Liška March 24, 2017, 1:23 p.m. UTC | #2 
On 03/24/2017 12:49 PM, Richard Biener wrote:
> On Fri, Mar 24, 2017 at 10:25 AM, Martin Liška <mliska@suse.cz> wrote:
>> Hello.
>>
>> Briefly described in the PR, running ICF (without any optimization level) can create a thunk call
>> that does not use an SSA_NAME which is a default def of an argument of the caller:
>>
>> c (complex float b)
>> {
>>   complex float arg.1;
>>   float retval.0;
>>
>>   <bb 2> [100.00%]:
>>   arg.1_2 = b;
>>   retval.0_4 = a (arg.1_2); [tail call]
>>
>>   return retval.0_4;
>>
>> }
>>
>> The "arg" variable creation was introduced by Jason in r207301.
>> As complex type is passed as invisible reference, we need to find address of the argument.
>> Thus I'm suggesting to find it alternatively via SSA_NAME_DEF_STMT.
>>
>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. And fixes the ICE
>> on s390x cross compiler.
>>
>> Ready to be installed?
> 
> Ick, that looks like fragile code ...  looks like the
> call_from_thunk_p case expects
> the default def or the param-decl.  So why not create it that way?  Or

Not creating the "arg" and directly passing the PARM_DECL will break verifier:

/home/marxin/Programming/testcases/pr80104.c:10:1: error: invalid argument to gimple call
 }
 ^
b
# .MEM_2 = VDEF <.MEM_1(D)>
retval.0_3 = a (b); [tail call]
/home/marxin/Programming/testcases/pr80104.c:10:1: internal compiler error: verify_gimple failed

> simply not do
> this "optimization"?

You mean understanding when an argument of a wrapper (thunk) will need invisible reference and
if so, then ICF should not do the optimization?

Martin

> 
> Richard.
> 
>> Martin
diff mbox

Patch

From 6624f7b292d174b5c0c1a276c1ed919c9930dca4 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Thu, 23 Mar 2017 16:03:04 +0100
Subject: [PATCH] Fix calls.c for a _complex type (PR ipa/80104).

gcc/ChangeLog:

2017-03-23  Martin Liska  <mliska@suse.cz>

	PR ipa/80104
	* calls.c (initialize_argument_information): Find real argument
	of a gimple call (of thunk) in order to find address of the
	argument.

gcc/testsuite/ChangeLog:

2017-03-23  Martin Liska  <mliska@suse.cz>

	* gcc.dg/ipa/pr80104.c: New test.
---
 gcc/calls.c                        | 11 +++++++++--
 gcc/testsuite/gcc.dg/ipa/pr80104.c | 15 +++++++++++++++
 2 files changed, 24 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/ipa/pr80104.c

diff --git a/gcc/calls.c b/gcc/calls.c
index 61caf4ca752..0ee21d9e951 100644
--- a/gcc/calls.c
+++ b/gcc/calls.c
@@ -1745,8 +1745,15 @@  initialize_argument_information (int num_actuals ATTRIBUTE_UNUSED,
 		 address.  */
 	      if (TREE_CODE (args[i].tree_value) == SSA_NAME)
 		{
-		  gcc_assert (SSA_NAME_IS_DEFAULT_DEF (args[i].tree_value));
-		  args[i].tree_value = SSA_NAME_VAR (args[i].tree_value);
+		  if (SSA_NAME_IS_DEFAULT_DEF (args[i].tree_value))
+		    args[i].tree_value = SSA_NAME_VAR (args[i].tree_value);
+		  else
+		  {
+		    gimple *g = SSA_NAME_DEF_STMT (args[i].tree_value);
+		    if (gimple_assign_single_p (g))
+		      args[i].tree_value = gimple_assign_rhs1 (g);
+		  }
+
 		  gcc_assert (TREE_CODE (args[i].tree_value) == PARM_DECL);
 		}
 	      /* Argument setup code may have copied the value to register.  We
diff --git a/gcc/testsuite/gcc.dg/ipa/pr80104.c b/gcc/testsuite/gcc.dg/ipa/pr80104.c
new file mode 100644
index 00000000000..7e75c9907e7
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/pr80104.c
@@ -0,0 +1,15 @@ 
+/* PR ipa/80104 */
+/* { dg-do compile } */
+/* { dg-options "-fipa-icf" } */
+
+float
+a (_Complex float b)
+{
+  return *&b;
+}
+
+float
+c (_Complex float b)
+{
+  return (&b)[0];
+}
-- 
2.12.0