ipa-sra: Prevent constructing debug info from wrong argument

Message ID ri6blkynbja.fsf@suse.cz
State New
Headers show
Series
  • ipa-sra: Prevent constructing debug info from wrong argument
Related show

Commit Message

Martin Jambor July 1, 2020, 9:57 p.m.
Hi,

the mechanism generating debug info for removed parameters did not
adjust index of the argument in the call statement to take into
account extra arguments IPA-SRA might have produced when splitting a
strucutre.  This patch addresses that omission and stops gdb from
showing incorrect value for the removed parameter and says "value
optimized out" instead.  The guality testcase will end up as
UNSUPPORTED in the results which is how Richi told me on IRC we deal
with this.

It is possible to generate debug info to actually show the value of the
removed parameter but so far my approaches to do that seem too
controversial
(https://gcc.gnu.org/pipermail/gcc-patches/2020-May/546705.html), so
before I come up with something better I'd like to push this to master
and the gcc-10 branch in time for the GCC 10.2 release.

Bootstrapped and tested on master on x86_64-linux, bootstrap on top of
the gcc-10 branch is underway?  OK for both if it passes?

Thanks,

Martin


gcc/ChangeLog:

2020-07-01  Martin Jambor  <mjambor@suse.cz>

	PR guality/95343
	* ipa-param-manipulation.c (ipa_param_adjustments::modify_call): Adjust
	argument index if necessary.

gcc/testsuite/ChangeLog:

2020-07-01  Martin Jambor  <mjambor@suse.cz>

	PR guality/95343
	* gcc.dg/guality/pr95343.c: New test.
---
 gcc/ipa-param-manipulation.c           |  6 +++-
 gcc/testsuite/gcc.dg/guality/pr95343.c | 45 ++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/guality/pr95343.c

-- 
2.27.0

Comments

Peter Bergner via Gcc-patches July 2, 2020, 8:41 a.m. | #1
On Wed, Jul 1, 2020 at 11:59 PM Martin Jambor <mjambor@suse.cz> wrote:
>

> Hi,

>

> the mechanism generating debug info for removed parameters did not

> adjust index of the argument in the call statement to take into

> account extra arguments IPA-SRA might have produced when splitting a

> strucutre.  This patch addresses that omission and stops gdb from

> showing incorrect value for the removed parameter and says "value

> optimized out" instead.  The guality testcase will end up as

> UNSUPPORTED in the results which is how Richi told me on IRC we deal

> with this.

>

> It is possible to generate debug info to actually show the value of the

> removed parameter but so far my approaches to do that seem too

> controversial

> (https://gcc.gnu.org/pipermail/gcc-patches/2020-May/546705.html), so

> before I come up with something better I'd like to push this to master

> and the gcc-10 branch in time for the GCC 10.2 release.

>

> Bootstrapped and tested on master on x86_64-linux, bootstrap on top of

> the gcc-10 branch is underway?  OK for both if it passes?


OK.

Thanks,
Richard.

> Thanks,

>

> Martin

>

>

> gcc/ChangeLog:

>

> 2020-07-01  Martin Jambor  <mjambor@suse.cz>

>

>         PR guality/95343

>         * ipa-param-manipulation.c (ipa_param_adjustments::modify_call): Adjust

>         argument index if necessary.

>

> gcc/testsuite/ChangeLog:

>

> 2020-07-01  Martin Jambor  <mjambor@suse.cz>

>

>         PR guality/95343

>         * gcc.dg/guality/pr95343.c: New test.

> ---

>  gcc/ipa-param-manipulation.c           |  6 +++-

>  gcc/testsuite/gcc.dg/guality/pr95343.c | 45 ++++++++++++++++++++++++++

>  2 files changed, 50 insertions(+), 1 deletion(-)

>  create mode 100644 gcc/testsuite/gcc.dg/guality/pr95343.c

>

> diff --git a/gcc/ipa-param-manipulation.c b/gcc/ipa-param-manipulation.c

> index 2cc4bc79dc1..5fc0de56556 100644

> --- a/gcc/ipa-param-manipulation.c

> +++ b/gcc/ipa-param-manipulation.c

> @@ -790,7 +790,11 @@ ipa_param_adjustments::modify_call (gcall *stmt,

>           if (!is_gimple_reg (old_parm) || kept[i])

>             continue;

>           tree origin = DECL_ORIGIN (old_parm);

> -         tree arg = gimple_call_arg (stmt, i);

> +         tree arg;

> +         if (transitive_remapping)

> +           arg = gimple_call_arg (stmt, index_map[i]);

> +         else

> +           arg = gimple_call_arg (stmt, i);

>

>           if (!useless_type_conversion_p (TREE_TYPE (origin), TREE_TYPE (arg)))

>             {

> diff --git a/gcc/testsuite/gcc.dg/guality/pr95343.c b/gcc/testsuite/gcc.dg/guality/pr95343.c

> new file mode 100644

> index 00000000000..a3e57decda8

> --- /dev/null

> +++ b/gcc/testsuite/gcc.dg/guality/pr95343.c

> @@ -0,0 +1,45 @@

> +/* { dg-do run } */

> +/* { dg-options "-g -fno-ipa-icf" } */

> +

> +volatile int v;

> +

> +int __attribute__((noipa))

> +get_val0 (void)  {return 0;}

> +int __attribute__((noipa))

> +get_val2 (void)  {return 2;}

> +

> +struct S

> +{

> +  int a, b, c;

> +};

> +

> +static int __attribute__((noinline))

> +bar (struct S s, int x, int y, int z, int i)

> +{

> +  int r;

> +  v = s.a + s.b;               /* { dg-final { gdb-test . "i+1" "3" } } */

> +  return r;

> +}

> +

> +static int __attribute__((noinline))

> +foo (struct S s, int i)

> +{

> +  int r;

> +  r = bar (s, 3, 4, 5, i);

> +  return r;

> +}

> +

> +

> +int

> +main (void)

> +{

> +  struct S s;

> +  int i;

> +  i = get_val2 ();

> +  s.a = get_val0 ();

> +  s.b = get_val0 ();

> +  s.c = get_val0 ();

> +  int r = foo (s, i);

> +  v = r + i;

> +  return 0;

> +}

> --

> 2.27.0

>

Patch

diff --git a/gcc/ipa-param-manipulation.c b/gcc/ipa-param-manipulation.c
index 2cc4bc79dc1..5fc0de56556 100644
--- a/gcc/ipa-param-manipulation.c
+++ b/gcc/ipa-param-manipulation.c
@@ -790,7 +790,11 @@  ipa_param_adjustments::modify_call (gcall *stmt,
 	  if (!is_gimple_reg (old_parm) || kept[i])
 	    continue;
 	  tree origin = DECL_ORIGIN (old_parm);
-	  tree arg = gimple_call_arg (stmt, i);
+	  tree arg;
+	  if (transitive_remapping)
+	    arg = gimple_call_arg (stmt, index_map[i]);
+	  else
+	    arg = gimple_call_arg (stmt, i);
 
 	  if (!useless_type_conversion_p (TREE_TYPE (origin), TREE_TYPE (arg)))
 	    {
diff --git a/gcc/testsuite/gcc.dg/guality/pr95343.c b/gcc/testsuite/gcc.dg/guality/pr95343.c
new file mode 100644
index 00000000000..a3e57decda8
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/guality/pr95343.c
@@ -0,0 +1,45 @@ 
+/* { dg-do run } */
+/* { dg-options "-g -fno-ipa-icf" } */
+
+volatile int v;
+
+int __attribute__((noipa))
+get_val0 (void)  {return 0;}
+int __attribute__((noipa))
+get_val2 (void)  {return 2;}
+
+struct S
+{
+  int a, b, c;
+};
+
+static int __attribute__((noinline))
+bar (struct S s, int x, int y, int z, int i)
+{
+  int r;
+  v = s.a + s.b;		/* { dg-final { gdb-test . "i+1" "3" } } */
+  return r;
+}
+
+static int __attribute__((noinline))
+foo (struct S s, int i)
+{
+  int r;
+  r = bar (s, 3, 4, 5, i);
+  return r;
+}
+
+
+int
+main (void)
+{
+  struct S s;
+  int i;
+  i = get_val2 ();
+  s.a = get_val0 ();
+  s.b = get_val0 ();
+  s.c = get_val0 ();
+  int r = foo (s, i);
+  v = r + i;
+  return 0;
+}