[Bug debug/96354] [10/11 Regression] ICE in maybe_canonicalize_mem_ref_addr, at gimple-fold.c:4903 since r10-2271-gd81ab49d0586fca0
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354 Richard Biener changed: What|Removed |Added Resolution|--- |FIXED Status|NEW |RESOLVED Known to fail||10.2.0 Known to work||10.2.1 --- Comment #25 from Richard Biener --- Fixed.
[Bug debug/96354] [10/11 Regression] ICE in maybe_canonicalize_mem_ref_addr, at gimple-fold.c:4903 since r10-2271-gd81ab49d0586fca0
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354 --- Comment #24 from CVS Commits --- The releases/gcc-10 branch has been updated by Jakub Jelinek : https://gcc.gnu.org/g:49643954ffa71ddeec8e979e05f145ea688027f9 commit r10-8569-g49643954ffa71ddeec8e979e05f145ea688027f9 Author: Jakub Jelinek Date: Tue Aug 4 11:31:44 2020 +0200 gimple-fold: Fix ICE in maybe_canonicalize_mem_ref_addr on debug stmt [PR96354] In debug stmts, we are less strict about what is and what is not accepted there, so this patch just punts on optimization of a debug stmt rather than ICEing. 2020-08-04 Jakub Jelinek PR debug/96354 * gimple-fold.c (maybe_canonicalize_mem_ref_addr): Add IS_DEBUG argument. Return false instead of gcc_unreachable if it is true and get_addr_base_and_unit_offset returns NULL. (fold_stmt_1) : Adjust caller. * g++.dg/opt/pr96354.C: New test. (cherry picked from commit fabe0ede9db9fa95832b2329d3d6156711905e20)
[Bug debug/96354] [10/11 Regression] ICE in maybe_canonicalize_mem_ref_addr, at gimple-fold.c:4903 since r10-2271-gd81ab49d0586fca0
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354 --- Comment #23 from CVS Commits --- The master branch has been updated by Jakub Jelinek : https://gcc.gnu.org/g:fabe0ede9db9fa95832b2329d3d6156711905e20 commit r11-2539-gfabe0ede9db9fa95832b2329d3d6156711905e20 Author: Jakub Jelinek Date: Tue Aug 4 11:31:44 2020 +0200 gimple-fold: Fix ICE in maybe_canonicalize_mem_ref_addr on debug stmt [PR96354] In debug stmts, we are less strict about what is and what is not accepted there, so this patch just punts on optimization of a debug stmt rather than ICEing. 2020-08-04 Jakub Jelinek PR debug/96354 * gimple-fold.c (maybe_canonicalize_mem_ref_addr): Add IS_DEBUG argument. Return false instead of gcc_unreachable if it is true and get_addr_base_and_unit_offset returns NULL. (fold_stmt_1) : Adjust caller. * g++.dg/opt/pr96354.C: New test.
[Bug debug/96354] [10/11 Regression] ICE in maybe_canonicalize_mem_ref_addr, at gimple-fold.c:4903 since r10-2271-gd81ab49d0586fca0
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354 --- Comment #22 from Richard Biener --- (In reply to Jakub Jelinek from comment #21) > Created attachment 48987 [details] > gcc11-pr96354.patch > > So like this? yes.
[Bug debug/96354] [10/11 Regression] ICE in maybe_canonicalize_mem_ref_addr, at gimple-fold.c:4903 since r10-2271-gd81ab49d0586fca0
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354 --- Comment #21 from Jakub Jelinek --- Created attachment 48987 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48987&action=edit gcc11-pr96354.patch So like this?
[Bug debug/96354] [10/11 Regression] ICE in maybe_canonicalize_mem_ref_addr, at gimple-fold.c:4903 since r10-2271-gd81ab49d0586fca0
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354 --- Comment #20 from rguenther at suse dot de --- On Wed, 29 Jul 2020, jakub at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354 > > --- Comment #19 from Jakub Jelinek --- > But doing that would mean pretty much the same amount of code at the expense > of > making the IL larger even when not needed (and perhaps risking some > optimization opportunities). > Yet another possibility would be instead of doing that in this function clear > id->regimplify at the start of copy_debug_stmt, and if it is set after > processing the value, do something like regimplification for the debug stmts, > i.e. walk_tree which would handle the cases we consider invalid, perhaps for > now just the MEM_REF case with non-is_gimple_mem_ref_addr/DEBUG_EXPR_DECL as > first operand and in the future perhaps something else too. Meh. I guess let's try not ICEing during maybe_canonicalize_mem_ref_addr instead, thus make it "best effort" for GIMPLE_DEBUG stmts. Or not fold it at all. But I guess that we do and only do maybe_canonicalize_mem_ref_addr means we've run into problems before... like PR45056?
[Bug debug/96354] [10/11 Regression] ICE in maybe_canonicalize_mem_ref_addr, at gimple-fold.c:4903 since r10-2271-gd81ab49d0586fca0
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354 --- Comment #19 from Jakub Jelinek --- But doing that would mean pretty much the same amount of code at the expense of making the IL larger even when not needed (and perhaps risking some optimization opportunities). Yet another possibility would be instead of doing that in this function clear id->regimplify at the start of copy_debug_stmt, and if it is set after processing the value, do something like regimplification for the debug stmts, i.e. walk_tree which would handle the cases we consider invalid, perhaps for now just the MEM_REF case with non-is_gimple_mem_ref_addr/DEBUG_EXPR_DECL as first operand and in the future perhaps something else too.
[Bug debug/96354] [10/11 Regression] ICE in maybe_canonicalize_mem_ref_addr, at gimple-fold.c:4903 since r10-2271-gd81ab49d0586fca0
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354 --- Comment #18 from rguenther at suse dot de --- On Wed, 29 Jul 2020, jakub at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354 > > --- Comment #17 from Jakub Jelinek --- > But we need to do the regimplification for several other reasons, so what > would > be the advantage of doing it that way? The advantage is removing the special casing for debug stmts and fixing the bug?
[Bug debug/96354] [10/11 Regression] ICE in maybe_canonicalize_mem_ref_addr, at gimple-fold.c:4903 since r10-2271-gd81ab49d0586fca0
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354 --- Comment #17 from Jakub Jelinek --- But we need to do the regimplification for several other reasons, so what would be the advantage of doing it that way?
[Bug debug/96354] [10/11 Regression] ICE in maybe_canonicalize_mem_ref_addr, at gimple-fold.c:4903 since r10-2271-gd81ab49d0586fca0
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354
--- Comment #16 from rguenther at suse dot de ---
On Wed, 29 Jul 2020, jakub at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354
>
> --- Comment #15 from Jakub Jelinek ---
> We have indeed:
> # DEBUG D#2 => MEM[(double *)&]
> and on the caller side:
> D.2566[_9] = foo<3, 3> (D.2559, D.2572); [return slot optimization]
> So, that is why to & &D.2566[_9] is propagated.
> Now, if I add to foo a call to some function template that takes &t as
> argument, this is handled correctly because id->regimplify is set to true and
> the
> qux > (&D.2572[_9]);
> call is fixed up by gimple_regimplify_operands. That function really isn't
> called on debug stmts though (and not prepared to be called for them).
> If I use
> template C foo (D, C) { C t; double d = 1.25;
> __builtin_memcpy (&t, &d, sizeof (double)); return t; }
> instead so that before that inlining we get
> MEM [(char * {ref-all})&] = _4;
> then it is indeed again gimple_regimplify_operands that fixes up the
> MEM [(char * {ref-all})&D.2569[_9]] = 4608308318706860032;
> into:
> _21 = &D.2569[_9];
> MEM [(char * {ref-all})_21] = 4608308318706860032;
So we could avoid (some) regimplification if we'd dealt with this
gimplification step during return value setup. We then only might
not re-propagate things and so
.b;
might be forever
tem_1 = &D.123[j_3];
MEM[tem_1].b;
instead of
D.123[j_3].b;
doing the re-gimplification when we replace things from the
decl map might be possible as well of course (and we then
would have "context" and could do special things when in
ADDR_EXPR context). Not sure how ugly that is.
Not sure how bad the non-propagation above is either and how
often it triggers.
[Bug debug/96354] [10/11 Regression] ICE in maybe_canonicalize_mem_ref_addr, at gimple-fold.c:4903 since r10-2271-gd81ab49d0586fca0
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354
--- Comment #15 from Jakub Jelinek ---
We have indeed:
# DEBUG D#2 => MEM[(double *)&]
and on the caller side:
D.2566[_9] = foo<3, 3> (D.2559, D.2572); [return slot optimization]
So, that is why to & &D.2566[_9] is propagated.
Now, if I add to foo a call to some function template that takes &t as
argument, this is handled correctly because id->regimplify is set to true and
the
qux > (&D.2572[_9]);
call is fixed up by gimple_regimplify_operands. That function really isn't
called on debug stmts though (and not prepared to be called for them).
If I use
template C foo (D, C) { C t; double d = 1.25;
__builtin_memcpy (&t, &d, sizeof (double)); return t; }
instead so that before that inlining we get
MEM [(char * {ref-all})&] = _4;
then it is indeed again gimple_regimplify_operands that fixes up the
MEM [(char * {ref-all})&D.2569[_9]] = 4608308318706860032;
into:
_21 = &D.2569[_9];
MEM [(char * {ref-all})_21] = 4608308318706860032;
[Bug debug/96354] [10/11 Regression] ICE in maybe_canonicalize_mem_ref_addr, at gimple-fold.c:4903 since r10-2271-gd81ab49d0586fca0
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354
--- Comment #14 from Richard Biener ---
OK, looks like inlining C<3>::C into foo<3, 3> produces this debug stmt.
foo<3, 3> (struct D D.2506, struct C D.2507)
{
:
C<3>::C (&);
return ;
and inlined from
C<3>::C (struct C * const this)
{
[local count: 1073741824]:
*this_2(D) ={v} {CLOBBER};
# DEBUG D#1 => &this_2(D)->d
# DEBUG this => D#1
# DEBUG INLINE_ENTRY baz
# DEBUG this => NULL
# DEBUG D#2 => MEM[(double *)this_2(D)]
# DEBUG c => D#2
return;
so if the MEM wouldn't be dead we'd face the same situation in regular
stmts I guess. And we do. But then the inliner sees
_10 = &D.2565[_9];
C<3>::C (_10);
_12 = &D.2564[_9];
C<3>::C (_12);
so value-substitution shouldn't happen here anyway... which means
it is eventually a over-eager mapping registered due to debug for
unused decls or so?
So we should fix this more "upstream" I bet. In declare_return_variable
we're doing insert_decl_map (id, result, var) with var == D.2567[_9]
which is problematic. We might want to gimplify this there into
ptr_42 = &D.2567[_9] and map to MEM[ptr_42] instead?
That might simplify code elsewhere but eventually affects code generation.
Note this will only happen for return slots I think.
[Bug debug/96354] [10/11 Regression] ICE in maybe_canonicalize_mem_ref_addr, at gimple-fold.c:4903 since r10-2271-gd81ab49d0586fca0
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354 --- Comment #13 from Richard Biener --- (In reply to Jakub Jelinek from comment #12) > Created attachment 48949 [details] > gcc11-pr96354.patch > > Untested fix. So I wonder where exactly we get this non-invariant address from and what makes substitution into regular stmts fixed up? Looks like the substitution is for & which we deal with separately for regular stmts. I guess we do not share the ADDR_EXPR tree so we can't put a remap of that here. But we may be able to detect this case up in copy_debug_stmt itself? The debug stmt seems to be # DEBUG D#2 => MEM[(double *)&] and retval maps to D.2566[_9]. And I guess in regular code we never end up with & anywhere. So not sure how we ended up with that DEBUG stmt in the first place...
[Bug debug/96354] [10/11 Regression] ICE in maybe_canonicalize_mem_ref_addr, at gimple-fold.c:4903 since r10-2271-gd81ab49d0586fca0
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354 --- Comment #12 from Jakub Jelinek --- Created attachment 48949 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48949&action=edit gcc11-pr96354.patch Untested fix.
