public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug middle-end/110378] New: SRA for destructors
@ 2023-06-23 16:14 hubicka at gcc dot gnu.org
2023-06-23 16:15 ` [Bug middle-end/110378] " hubicka at gcc dot gnu.org
` (9 more replies)
0 siblings, 10 replies; 11+ messages in thread
From: hubicka at gcc dot gnu.org @ 2023-06-23 16:14 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110378
Bug ID: 110378
Summary: SRA for destructors
Product: gcc
Version: 13.1.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: middle-end
Assignee: unassigned at gcc dot gnu.org
Reporter: hubicka at gcc dot gnu.org
Target Milestone: ---
In thestcase from PR109849 built with profile feedback we get the following
cleanup:
<L10>:
D.28322 ={v} {CLOBBER};
cur ={v} {CLOBBER(eol)};
std::vector<std::pair<unsigned int, unsigned int> >::~vector (&stack);
_28 = __builtin_eh_pointer (2);
__builtin_unwind_resume (_28);
which calls destructor of std::vector. This destructor compiles to:
void std::vector<std::pair<unsigned int, unsigned int> >::~vector (struct
vector * const this)
{
struct pair * _6;
struct pair * _7;
long int _8;
long unsigned int _15;
;; basic block 2, loop depth 0, count 10000 (precise)
;; prev block 0, next block 3, flags: (NEW, REACHABLE, VISITED)
;; pred: ENTRY [always] count:10000 (precise) (FALLTHRU,EXECUTABLE)
_6 = MEM[(struct _Vector_base *)this_3(D)]._M_impl.D.25963._M_end_of_storage;
_7 = MEM[(struct _Vector_base *)this_3(D)]._M_impl.D.25963._M_start;
_8 = _6 - _7;
if (_7 != 0B)
goto <bb 3>; [100.00%]
else
goto <bb 4>; [0.00%]
;; succ: 3 [always] count:10000 (precise) (TRUE_VALUE,EXECUTABLE)
;; 4 [never] count:0 (precise) (FALSE_VALUE,EXECUTABLE)
;; basic block 3, loop depth 0, count 10000 (precise)
;; prev block 2, next block 4, flags: (NEW, REACHABLE, VISITED)
;; pred: 2 [always] count:10000 (precise) (TRUE_VALUE,EXECUTABLE)
_15 = (long unsigned int) _8;
operator delete (_7, _15); [tail call]
;; succ: 4 [always] count:10000 (precise) (FALLTHRU,EXECUTABLE)
;; basic block 4, loop depth 0, count 10000 (precise)
;; prev block 3, next block 1, flags: (NEW, REACHABLE, VISITED)
;; pred: 2 [never] count:0 (precise) (FALSE_VALUE,EXECUTABLE)
;; 3 [always] count:10000 (precise) (FALLTHRU,EXECUTABLE)
return;
;; succ: EXIT [always] count:10000 (precise) (EXECUTABLE)
/usr/include/c++/13/bits/stl_vector.h:735:7
}
and it is used twice in code that is considered cold. Since the code itself is
longer than the call of operator delete, it makes sense to not inline it.
Before ipa-cp it is:
Released 8 names, 53.33%, removed 8 holes
void std::vector<std::pair<unsigned int, unsigned int> >::~vector (struct
vector * const this)
{
struct pair * _1;
struct pair * _2;
struct pair * _6;
struct pair * _7;
long int _8;
long int _9;
long unsigned int _10;
struct _Vector_impl * _11;
;; basic block 2, loop depth 0, count 1073741824 (estimated locally), maybe
hot
;; prev block 0, next block 3, flags: (NEW, REACHABLE, VISITED)
;; pred: ENTRY [always] count:1073741824 (estimated locally)
(FALLTHRU,EXECUTABLE)
_1 = this_3(D)->D.26656._M_impl.D.25963._M_finish;
_2 = this_3(D)->D.26656._M_impl.D.25963._M_start;
std::_Destroy<std::pair<unsigned int, unsigned int>*> (_2, _1);
_6 = MEM[(struct _Vector_base *)this_3(D)]._M_impl.D.25963._M_end_of_storage;
_7 = MEM[(struct _Vector_base *)this_3(D)]._M_impl.D.25963._M_start;
_8 = _6 - _7;
_9 = _8 /[ex] 8;
_10 = (long unsigned int) _9;
if (_7 != 0B)
goto <bb 3>; [53.47%]
else
goto <bb 4>; [46.53%]
;; succ: 3 [53.5% (guessed)] count:574129752 (estimated locally)
(TRUE_VALUE,EXECUTABLE)
;; 4 [46.5% (guessed)] count:499612072 (estimated locally)
(FALSE_VALUE,EXECUTABLE)
;; basic block 3, loop depth 0, count 574129753 (estimated locally), maybe
hot
;; prev block 2, next block 4, flags: (NEW, REACHABLE, VISITED)
;; pred: 2 [53.5% (guessed)] count:574129752 (estimated locally)
(TRUE_VALUE,EXECUTABLE)
_11 = &MEM[(struct _Vector_base *)this_3(D)]._M_impl;
std::__new_allocator<std::pair<unsigned int, unsigned int> >::deallocate
(_11, _7, _10);
;; succ: 4 [always] count:574129753 (estimated locally)
(FALLTHRU,EXECUTABLE) /usr/include/c++/13/bits/alloc_traits.h:516:35
;; basic block 4, loop depth 0, count 1073741824 (estimated locally), maybe
hot
;; prev block 3, next block 1, flags: (NEW, REACHABLE, VISITED)
;; pred: 2 [46.5% (guessed)] count:499612072 (estimated locally)
(FALSE_VALUE,EXECUTABLE)
;; 3 [always] count:574129753 (estimated locally)
(FALLTHRU,EXECUTABLE) /usr/include/c++/13/bits/alloc_traits.h:516:35
*this_3(D) ={v} {CLOBBER};
return;
;; succ: EXIT [always] count:1073741824 (estimated locally)
(EXECUTABLE) /usr/include/c++/13/bits/stl_vector.h:735:7
}
I think it is the clobber that prevents ipa-sra :)
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug middle-end/110378] SRA for destructors
2023-06-23 16:14 [Bug middle-end/110378] New: SRA for destructors hubicka at gcc dot gnu.org
@ 2023-06-23 16:15 ` hubicka at gcc dot gnu.org
2023-07-31 15:25 ` [Bug middle-end/110378] IPA-SRA " jamborm at gcc dot gnu.org
` (8 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: hubicka at gcc dot gnu.org @ 2023-06-23 16:15 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110378
Jan Hubicka <hubicka at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Blocks| |109849
--- Comment #1 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
In general we may also learn to handle modifications to parameters that are
clobbered later as read-only.
Referenced Bugs:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109849
[Bug 109849] suboptimal code for vector walking loop
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug middle-end/110378] IPA-SRA for destructors
2023-06-23 16:14 [Bug middle-end/110378] New: SRA for destructors hubicka at gcc dot gnu.org
2023-06-23 16:15 ` [Bug middle-end/110378] " hubicka at gcc dot gnu.org
@ 2023-07-31 15:25 ` jamborm at gcc dot gnu.org
2023-07-31 16:02 ` [Bug ipa/110378] " jamborm at gcc dot gnu.org
` (7 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: jamborm at gcc dot gnu.org @ 2023-07-31 15:25 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110378
--- Comment #2 from Martin Jambor <jamborm at gcc dot gnu.org> ---
Created attachment 55663
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55663&action=edit
Simplest testcase
The PR 109849 testcase behavior changes over time, so I prepared three
specialized for this PR. This one is simplest, just making sure a clobber is
not considered a write for the purposes of IPA-SRA parameter splitting.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug ipa/110378] IPA-SRA for destructors
2023-06-23 16:14 [Bug middle-end/110378] New: SRA for destructors hubicka at gcc dot gnu.org
2023-06-23 16:15 ` [Bug middle-end/110378] " hubicka at gcc dot gnu.org
2023-07-31 15:25 ` [Bug middle-end/110378] IPA-SRA " jamborm at gcc dot gnu.org
@ 2023-07-31 16:02 ` jamborm at gcc dot gnu.org
2023-07-31 16:06 ` jamborm at gcc dot gnu.org
` (6 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: jamborm at gcc dot gnu.org @ 2023-07-31 16:02 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110378
Martin Jambor <jamborm at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |NEW
Last reconfirmed| |2023-07-31
Ever confirmed|0 |1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug ipa/110378] IPA-SRA for destructors
2023-06-23 16:14 [Bug middle-end/110378] New: SRA for destructors hubicka at gcc dot gnu.org
` (2 preceding siblings ...)
2023-07-31 16:02 ` [Bug ipa/110378] " jamborm at gcc dot gnu.org
@ 2023-07-31 16:06 ` jamborm at gcc dot gnu.org
2023-07-31 16:08 ` jamborm at gcc dot gnu.org
` (5 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: jamborm at gcc dot gnu.org @ 2023-07-31 16:06 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110378
--- Comment #3 from Martin Jambor <jamborm at gcc dot gnu.org> ---
Created attachment 55664
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55664&action=edit
Testcase with single inheritance
This testcase is somewhat more difficult and addressing will mean not
just changes in the IPA-SRA analysis but also in the modification.
Both need to deal with the following:
_1 = &this_2(D)->D.2842;
{anonymous}::foo::~foo (_1);
Modification must see _1 before its use to understand it is just
another name for a (split) parameter and must behave accordingly when
processing the call statement (i.e. prepare ground for call
redirection later).
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug ipa/110378] IPA-SRA for destructors
2023-06-23 16:14 [Bug middle-end/110378] New: SRA for destructors hubicka at gcc dot gnu.org
` (3 preceding siblings ...)
2023-07-31 16:06 ` jamborm at gcc dot gnu.org
@ 2023-07-31 16:08 ` jamborm at gcc dot gnu.org
2023-08-01 12:45 ` jamborm at gcc dot gnu.org
` (4 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: jamborm at gcc dot gnu.org @ 2023-07-31 16:08 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110378
--- Comment #4 from Martin Jambor <jamborm at gcc dot gnu.org> ---
Created attachment 55665
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55665&action=edit
Testcase with non-zero offset with pass-through split
This testcase is similar to the previous one but on top of it also deals with
non-zero offsets which is currently not supported by IPA-SRA analysis and will
also require some assumption adjustments on the modification side (see TODOs in
ptr_parm_has_nonarg_uses and in ipa_param_body_adjustments::modify_call_stmt).
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug ipa/110378] IPA-SRA for destructors
2023-06-23 16:14 [Bug middle-end/110378] New: SRA for destructors hubicka at gcc dot gnu.org
` (4 preceding siblings ...)
2023-07-31 16:08 ` jamborm at gcc dot gnu.org
@ 2023-08-01 12:45 ` jamborm at gcc dot gnu.org
2023-08-07 17:15 ` cvs-commit at gcc dot gnu.org
` (3 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: jamborm at gcc dot gnu.org @ 2023-08-01 12:45 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110378
--- Comment #5 from Martin Jambor <jamborm at gcc dot gnu.org> ---
I have proposed a patch addressing the simplest case of the three on the
mailing list:
https://gcc.gnu.org/pipermail/gcc-patches/2023-July/625895.html
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug ipa/110378] IPA-SRA for destructors
2023-06-23 16:14 [Bug middle-end/110378] New: SRA for destructors hubicka at gcc dot gnu.org
` (5 preceding siblings ...)
2023-08-01 12:45 ` jamborm at gcc dot gnu.org
@ 2023-08-07 17:15 ` cvs-commit at gcc dot gnu.org
2023-08-08 16:20 ` clyon at gcc dot gnu.org
` (2 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-08-07 17:15 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110378
--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Martin Jambor <jamborm@gcc.gnu.org>:
https://gcc.gnu.org/g:da1a888b524d620c7a17f368b69c46934b69495c
commit r14-3038-gda1a888b524d620c7a17f368b69c46934b69495c
Author: Martin Jambor <mjambor@suse.cz>
Date: Mon Aug 7 19:13:41 2023 +0200
ipa-sra: Don't consider CLOBBERS as writes preventing splitting
When IPA-SRA detects whether a parameter passed by reference is
written to, it does not special case CLOBBERs which means it often
bails out unnecessarily, especially when dealing with C++ destructors.
Fixed by the obvious continue in the two relevant loops and by adding
a simple function that marks the clobbers in the transformation code
as statements to be removed.
gcc/ChangeLog:
2023-08-04 Martin Jambor <mjambor@suse.cz>
PR ipa/110378
* ipa-param-manipulation.h (class ipa_param_body_adjustments): New
members get_ddef_if_exists_and_is_used and mark_clobbers_dead.
* ipa-sra.cc (isra_track_scalar_value_uses): Ignore clobbers.
(ptr_parm_has_nonarg_uses): Likewise.
* ipa-param-manipulation.cc
(ipa_param_body_adjustments::get_ddef_if_exists_and_is_used): New.
(ipa_param_body_adjustments::mark_dead_statements): Move initial
checks to get_ddef_if_exists_and_is_used.
(ipa_param_body_adjustments::mark_clobbers_dead): New.
(ipa_param_body_adjustments::common_initialization): Call
mark_clobbers_dead when splitting.
gcc/testsuite/ChangeLog:
2023-07-31 Martin Jambor <mjambor@suse.cz>
PR ipa/110378
* g++.dg/ipa/pr110378-1.C: New test.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug ipa/110378] IPA-SRA for destructors
2023-06-23 16:14 [Bug middle-end/110378] New: SRA for destructors hubicka at gcc dot gnu.org
` (6 preceding siblings ...)
2023-08-07 17:15 ` cvs-commit at gcc dot gnu.org
@ 2023-08-08 16:20 ` clyon at gcc dot gnu.org
2023-08-08 16:20 ` clyon at gcc dot gnu.org
2023-10-03 16:53 ` cvs-commit at gcc dot gnu.org
9 siblings, 0 replies; 11+ messages in thread
From: clyon at gcc dot gnu.org @ 2023-08-08 16:20 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110378
Christophe Lyon <clyon at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |clyon at gcc dot gnu.org
--- Comment #7 from Christophe Lyon <clyon at gcc dot gnu.org> ---
The new test fails on arm:
FAIL: g++.dg/ipa/pr110378-1.C -std=gnu++14 scan-ipa-dump sra "Will split
parameter 0"
FAIL: g++.dg/ipa/pr110378-1.C -std=gnu++14 scan-tree-dump-not optimized
"shouldnotexist"
FAIL: g++.dg/ipa/pr110378-1.C -std=gnu++17 scan-ipa-dump sra "Will split
parameter 0"
FAIL: g++.dg/ipa/pr110378-1.C -std=gnu++17 scan-tree-dump-not optimized
"shouldnotexist"
FAIL: g++.dg/ipa/pr110378-1.C -std=gnu++20 scan-ipa-dump sra "Will split
parameter 0"
FAIL: g++.dg/ipa/pr110378-1.C -std=gnu++20 scan-tree-dump-not optimized
"shouldnotexist"
FAIL: g++.dg/ipa/pr110378-1.C -std=gnu++98 scan-ipa-dump sra "Will split
parameter 0"
FAIL: g++.dg/ipa/pr110378-1.C -std=gnu++98 scan-tree-dump-not optimized
"shouldnotexist"
I'm attaching pr110378-1.C.083i.sra
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug ipa/110378] IPA-SRA for destructors
2023-06-23 16:14 [Bug middle-end/110378] New: SRA for destructors hubicka at gcc dot gnu.org
` (7 preceding siblings ...)
2023-08-08 16:20 ` clyon at gcc dot gnu.org
@ 2023-08-08 16:20 ` clyon at gcc dot gnu.org
2023-10-03 16:53 ` cvs-commit at gcc dot gnu.org
9 siblings, 0 replies; 11+ messages in thread
From: clyon at gcc dot gnu.org @ 2023-08-08 16:20 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110378
--- Comment #8 from Christophe Lyon <clyon at gcc dot gnu.org> ---
Created attachment 55707
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55707&action=edit
pr110378-1.C.083i.sra
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug ipa/110378] IPA-SRA for destructors
2023-06-23 16:14 [Bug middle-end/110378] New: SRA for destructors hubicka at gcc dot gnu.org
` (8 preceding siblings ...)
2023-08-08 16:20 ` clyon at gcc dot gnu.org
@ 2023-10-03 16:53 ` cvs-commit at gcc dot gnu.org
9 siblings, 0 replies; 11+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-10-03 16:53 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110378
--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Martin Jambor <jamborm@gcc.gnu.org>:
https://gcc.gnu.org/g:14d0c509898b0361f78284c05556035edde6d1e0
commit r14-4383-g14d0c509898b0361f78284c05556035edde6d1e0
Author: Martin Jambor <mjambor@suse.cz>
Date: Tue Oct 3 18:44:52 2023 +0200
ipa-sra: Allow IPA-SRA in presence of returns which will be removed
Testing on 32bit arm revealed that even the simplest case of PR 110378
was still not resolved there because destructors were rturning this
pointer. Needless to say, the return value of those destructors often
is just not used, which IPA-SRA can already detect in time. Since
such enhancement seems generally useful, here it is.
The patch simply adds two flag to respective summaries to mark down
situations when it encounters either a simple direct use of a defaut
definition SSA_NAME of a paramter, which means that the parameter may
still be split when rturn value is removed, and when any derived use
of it is returned, allowing for complete removal in that case, instead
of discarding it as a candidate for removal or splitting like we do
now. The IPA phase then simply checks that we indeed plan to remove
the return value before allowing any transformation to be considered
in such cases.
gcc/ChangeLog:
2023-08-18 Martin Jambor <mjambor@suse.cz>
PR ipa/110378
* ipa-param-manipulation.cc
(ipa_param_body_adjustments::mark_dead_statements): Verify that any
return uses of PARAM will be removed.
(ipa_param_body_adjustments::mark_clobbers_dead): Likewise.
* ipa-sra.cc (isra_param_desc): New fields
remove_only_when_retval_removed and split_only_when_retval_removed.
(struct gensum_param_desc): Likewise. Fix comment long line.
(ipa_sra_function_summaries::duplicate): Copy the new flags.
(dump_gensum_param_descriptor): Dump the new flags.
(dump_isra_param_descriptor): Likewise.
(isra_track_scalar_value_uses): New parameter desc. Set its flag
remove_only_when_retval_removed when encountering a simple return.
(isra_track_scalar_param_local_uses): Replace parameter call_uses_p
with desc. Pass it to isra_track_scalar_value_uses and set its
call_uses.
(ptr_parm_has_nonarg_uses): Accept parameter descriptor as a
parameter. If there is a direct return use, mark any..
(create_parameter_descriptors): Pass the whole parameter descriptor
to
isra_track_scalar_param_local_uses and ptr_parm_has_nonarg_uses.
(process_scan_results): Copy the new flags.
(isra_write_node_summary): Stream the new flags.
(isra_read_node_info): Likewise.
(adjust_parameter_descriptions): Check that transformations
requring return removal only happen when return value is removed.
Restructure main loop. Adjust dump message.
gcc/testsuite/ChangeLog:
2023-08-18 Martin Jambor <mjambor@suse.cz>
PR ipa/110378
* gcc.dg/ipa/ipa-sra-32.c: New test.
* gcc.dg/ipa/pr110378-4.c: Likewise.
* gcc.dg/ipa/ipa-sra-4.c: Use a return value.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-10-03 16:53 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-23 16:14 [Bug middle-end/110378] New: SRA for destructors hubicka at gcc dot gnu.org
2023-06-23 16:15 ` [Bug middle-end/110378] " hubicka at gcc dot gnu.org
2023-07-31 15:25 ` [Bug middle-end/110378] IPA-SRA " jamborm at gcc dot gnu.org
2023-07-31 16:02 ` [Bug ipa/110378] " jamborm at gcc dot gnu.org
2023-07-31 16:06 ` jamborm at gcc dot gnu.org
2023-07-31 16:08 ` jamborm at gcc dot gnu.org
2023-08-01 12:45 ` jamborm at gcc dot gnu.org
2023-08-07 17:15 ` cvs-commit at gcc dot gnu.org
2023-08-08 16:20 ` clyon at gcc dot gnu.org
2023-08-08 16:20 ` clyon at gcc dot gnu.org
2023-10-03 16:53 ` cvs-commit at gcc dot gnu.org
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).