* [PATCH] debug/104337 - avoid messing with the abstract origin chain in NRV
@ 2022-02-03 10:27 Richard Biener
2022-02-03 14:15 ` Jason Merrill
0 siblings, 1 reply; 2+ messages in thread
From: Richard Biener @ 2022-02-03 10:27 UTC (permalink / raw)
To: gcc-patches
The following avoids NRV from massaging DECL_ABSTRACT_ORIGIN after
variable creation since NRV runs _after_ the function was inlined and thus
affects the inlined variables copy indirectly. We may adjust the abstract
origin of a variable only at the point we create it, not further along the
path since otherwise the (new) invariant that the abstract origin is always
the ultimate origin cannot be maintained.
The intent of what NRV does is OK I guess and it may improve the debug
experience. But I also notice we do
SET_DECL_VALUE_EXPR (found, result);
DECL_HAS_VALUE_EXPR_P (found) = 1;
the code is there since the merge from tree-ssa which added tree-nrv.c.
Jakub added the DECL_VALUE_EXPR in g:938650d8fddb878f623e315f0b7fd94b217efa96
and Jason added the abstract origin setting conditional in g:7716876bbd3a
The follwoing takes the radical approach and remove the attempt
to "optimize" the debug info.
The gdb testsuites show no regressions.
Bootstrapped and tested on x86_64-unknown-linux-gnu.
OK?
Thanks,
Richard.
2022-02-03 Richard Biener <rguenther@suse.de>
PR debug/104337
* tree-nrv.cc (pass_nrv::execute): Remove tieing result and found
together via DECL_ABSTRACT_ORIGIN.
* gcc.dg/debug/pr104337.c: New testcase.
---
gcc/testsuite/gcc.dg/debug/pr104337.c | 15 +++++++++++++++
gcc/tree-nrv.cc | 13 -------------
2 files changed, 15 insertions(+), 13 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/debug/pr104337.c
diff --git a/gcc/testsuite/gcc.dg/debug/pr104337.c b/gcc/testsuite/gcc.dg/debug/pr104337.c
new file mode 100644
index 00000000000..d15680fbf1a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/debug/pr104337.c
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+
+struct a {
+ unsigned b : 7;
+};
+inline __attribute__((optimize(3))) __attribute__((always_inline)) struct a
+c() {
+ struct a d;
+ return d;
+}
+void e() {
+ for (;;)
+ c();
+}
+int main() {}
diff --git a/gcc/tree-nrv.cc b/gcc/tree-nrv.cc
index a67736dae5c..b96d5b86f27 100644
--- a/gcc/tree-nrv.cc
+++ b/gcc/tree-nrv.cc
@@ -236,19 +236,6 @@ pass_nrv::execute (function *fun)
fprintf (dump_file, "\n");
}
- /* At this point we know that all the return statements return the
- same local which has suitable attributes for NRV. Copy debugging
- information from FOUND to RESULT if it will be useful. But don't set
- DECL_ABSTRACT_ORIGIN to point at another function. */
- if (!DECL_IGNORED_P (found)
- && !(DECL_ABSTRACT_ORIGIN (found)
- && DECL_CONTEXT (DECL_ABSTRACT_ORIGIN (found)) != current_function_decl))
- {
- DECL_NAME (result) = DECL_NAME (found);
- DECL_SOURCE_LOCATION (result) = DECL_SOURCE_LOCATION (found);
- DECL_ABSTRACT_ORIGIN (result) = DECL_ABSTRACT_ORIGIN (found);
- }
-
TREE_ADDRESSABLE (result) |= TREE_ADDRESSABLE (found);
/* Now walk through the function changing all references to VAR to be
--
2.34.1
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] debug/104337 - avoid messing with the abstract origin chain in NRV
2022-02-03 10:27 [PATCH] debug/104337 - avoid messing with the abstract origin chain in NRV Richard Biener
@ 2022-02-03 14:15 ` Jason Merrill
0 siblings, 0 replies; 2+ messages in thread
From: Jason Merrill @ 2022-02-03 14:15 UTC (permalink / raw)
To: Richard Biener, gcc-patches
On 2/3/22 05:27, Richard Biener wrote:
> The following avoids NRV from massaging DECL_ABSTRACT_ORIGIN after
> variable creation since NRV runs _after_ the function was inlined and thus
> affects the inlined variables copy indirectly. We may adjust the abstract
> origin of a variable only at the point we create it, not further along the
> path since otherwise the (new) invariant that the abstract origin is always
> the ultimate origin cannot be maintained.
>
> The intent of what NRV does is OK I guess and it may improve the debug
> experience. But I also notice we do
>
> SET_DECL_VALUE_EXPR (found, result);
> DECL_HAS_VALUE_EXPR_P (found) = 1;
>
> the code is there since the merge from tree-ssa which added tree-nrv.c.
>
> Jakub added the DECL_VALUE_EXPR in g:938650d8fddb878f623e315f0b7fd94b217efa96
> and Jason added the abstract origin setting conditional in g:7716876bbd3a
>
> The follwoing takes the radical approach and remove the attempt
> to "optimize" the debug info.
>
> The gdb testsuites show no regressions.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu.
>
> OK?
OK. It makes sense that the DECL_VALUE_EXPR is enough for lookup in the
debugger to find 'd', and the debug output when I change c to noinline
seems the same.
> Thanks,
> Richard.
>
> 2022-02-03 Richard Biener <rguenther@suse.de>
>
> PR debug/104337
> * tree-nrv.cc (pass_nrv::execute): Remove tieing result and found
> together via DECL_ABSTRACT_ORIGIN.
>
> * gcc.dg/debug/pr104337.c: New testcase.
> ---
> gcc/testsuite/gcc.dg/debug/pr104337.c | 15 +++++++++++++++
> gcc/tree-nrv.cc | 13 -------------
> 2 files changed, 15 insertions(+), 13 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/debug/pr104337.c
>
> diff --git a/gcc/testsuite/gcc.dg/debug/pr104337.c b/gcc/testsuite/gcc.dg/debug/pr104337.c
> new file mode 100644
> index 00000000000..d15680fbf1a
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/debug/pr104337.c
> @@ -0,0 +1,15 @@
> +/* { dg-do compile } */
> +
> +struct a {
> + unsigned b : 7;
> +};
> +inline __attribute__((optimize(3))) __attribute__((always_inline)) struct a
> +c() {
> + struct a d;
> + return d;
> +}
> +void e() {
> + for (;;)
> + c();
> +}
> +int main() {}
> diff --git a/gcc/tree-nrv.cc b/gcc/tree-nrv.cc
> index a67736dae5c..b96d5b86f27 100644
> --- a/gcc/tree-nrv.cc
> +++ b/gcc/tree-nrv.cc
> @@ -236,19 +236,6 @@ pass_nrv::execute (function *fun)
> fprintf (dump_file, "\n");
> }
>
> - /* At this point we know that all the return statements return the
> - same local which has suitable attributes for NRV. Copy debugging
> - information from FOUND to RESULT if it will be useful. But don't set
> - DECL_ABSTRACT_ORIGIN to point at another function. */
> - if (!DECL_IGNORED_P (found)
> - && !(DECL_ABSTRACT_ORIGIN (found)
> - && DECL_CONTEXT (DECL_ABSTRACT_ORIGIN (found)) != current_function_decl))
> - {
> - DECL_NAME (result) = DECL_NAME (found);
> - DECL_SOURCE_LOCATION (result) = DECL_SOURCE_LOCATION (found);
> - DECL_ABSTRACT_ORIGIN (result) = DECL_ABSTRACT_ORIGIN (found);
> - }
> -
> TREE_ADDRESSABLE (result) |= TREE_ADDRESSABLE (found);
>
> /* Now walk through the function changing all references to VAR to be
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-02-03 14:15 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-03 10:27 [PATCH] debug/104337 - avoid messing with the abstract origin chain in NRV Richard Biener
2022-02-03 14:15 ` Jason Merrill
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).