public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] ipa-sra: Prevent constructing debug info from wrong argument
@ 2020-07-01 21:57 Martin Jambor
  2020-07-02  8:41 ` Richard Biener
  0 siblings, 1 reply; 2+ messages in thread
From: Martin Jambor @ 2020-07-01 21:57 UTC (permalink / raw)
  To: GCC Patches; +Cc: Jan Hubicka

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

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


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] ipa-sra: Prevent constructing debug info from wrong argument
  2020-07-01 21:57 [PATCH] ipa-sra: Prevent constructing debug info from wrong argument Martin Jambor
@ 2020-07-02  8:41 ` Richard Biener
  0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2020-07-02  8:41 UTC (permalink / raw)
  To: Martin Jambor; +Cc: GCC Patches, Jan Hubicka

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
>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2020-07-02  8:41 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-01 21:57 [PATCH] ipa-sra: Prevent constructing debug info from wrong argument Martin Jambor
2020-07-02  8:41 ` Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).