[Bug c++/96354] New: [10 regression] ICE in maybe_canonicalize_mem_ref_addr, at gimple-fold.c:4903

[Bug c++/96354] New: [10 regression] ICE in maybe_canonicalize_mem_ref_addr, at gimple-fold.c:4903

[tm@dev-zero.ch]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354

            Bug ID: 96354
           Summary: [10 regression] ICE in
                    maybe_canonicalize_mem_ref_addr, at gimple-fold.c:4903
           Product: gcc
           Version: 10.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: tm@dev-zero.ch
  Target Milestone: ---

Unfortunately I wasn't able to isolate it (yet), but what I see with the
plumed-2.6.1 package from https://github.com/plumed/plumed2/releases with
gcc-10.2.0 (which should include the fix for issue #94216 already) is this:

during IPA pass: inline
RMSD.cpp: In member function ‘void
PLMD::RMSDCoreData::doCoreCalcWithCloseStructure(bool, bool, PLMD::Tensor&,
PLMD::Tensor&, std::array<std::array<PLMD::TensorGeneric<3, 3>, 3>, 3>&)’:
RMSD.cpp:1134:6: internal compiler error: in maybe_canonicalize_mem_ref_addr,
at gimple-fold.c:4903
 1134 | void RMSDCoreData::doCoreCalcWithCloseStructure(bool safe,bool alEqDis,
Tensor & rotationPosClose, Tensor & rotationRefClose,
std::array<std::array<Tensor,3>,3> & drotationPosCloseDrr01) {
      |      ^~~~~~~~~~~~
0xa1bdac maybe_canonicalize_mem_ref_addr
    /data/tiziano/cp2k/tools/toolchain/build/gcc-10.2.0/gcc/gimple-fold.c:4903
0xa20553 fold_stmt_1
    /data/tiziano/cp2k/tools/toolchain/build/gcc-10.2.0/gcc/gimple-fold.c:5058
0xd26380 fold_marked_statements
    /data/tiziano/cp2k/tools/toolchain/build/gcc-10.2.0/gcc/tree-inline.c:5342
0xd332b5 optimize_inline_calls(tree_node*)
    /data/tiziano/cp2k/tools/toolchain/build/gcc-10.2.0/gcc/tree-inline.c:5448
0xac9f9e inline_transform(cgraph_node*)
   
/data/tiziano/cp2k/tools/toolchain/build/gcc-10.2.0/gcc/ipa-inline-transform.c:736
0xbf187e execute_one_ipa_transform_pass
    /data/tiziano/cp2k/tools/toolchain/build/gcc-10.2.0/gcc/passes.c:2233
0xbf187e execute_all_ipa_transforms(bool)
    /data/tiziano/cp2k/tools/toolchain/build/gcc-10.2.0/gcc/passes.c:2272
0x8dfdc4 cgraph_node::expand()
    /data/tiziano/cp2k/tools/toolchain/build/gcc-10.2.0/gcc/cgraphunit.c:2293
0x8e117d expand_all_functions
    /data/tiziano/cp2k/tools/toolchain/build/gcc-10.2.0/gcc/cgraphunit.c:2471
0x8e117d symbol_table::compile()
    /data/tiziano/cp2k/tools/toolchain/build/gcc-10.2.0/gcc/cgraphunit.c:2822
0x8e325c symbol_table::finalize_compilation_unit()
    /data/tiziano/cp2k/tools/toolchain/build/gcc-10.2.0/gcc/cgraphunit.c:3002
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.

The function would be this one here:
https://github.com/plumed/plumed2/blob/57fdb1ad1bcd4794190b97c22f86fec51d2a38be/src/tools/RMSD.cpp#L1133-L1203

[Bug c++/96354] [10 regression] ICE in maybe_canonicalize_mem_ref_addr, at gimple-fold.c:4903

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |marxin at gcc dot gnu.org
             Status|UNCONFIRMED                 |WAITING
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2020-07-28

--- Comment #1 from Martin Liška <marxin at gcc dot gnu.org> ---
Can you please provide steps how to build the project? I can reproduce it
locally then.

[Bug c++/96354] [10 regression] ICE in maybe_canonicalize_mem_ref_addr, at gimple-fold.c:4903

[tm@dev-zero.ch]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354

--- Comment #2 from Tiziano Müller <tm@dev-zero.ch> ---
(In reply to Martin Liška from comment #1)
> Can you please provide steps how to build the project? I can reproduce it
> locally then.

Of course! :)

After unpacking you don't even have to configure it, simply doing the following
is enough. Note: it is only with the combination '-O2', '-g', '-fopenmp',
leaving away any one of them makes it build.

$ cd plumed-2.6.1/src/tools
$ c++ --version
c++ (GCC) 10.2.0
Copyright (C) 2020 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
$ c++ -I.. -c -O2 -g -std=c++11 -fopenmp RMSD.cpp -o RMSD.o
during IPA pass: inline
RMSD.cpp: In member function ‘void
PLMD::RMSDCoreData::doCoreCalcWithCloseStructure(bool, bool, PLMD::Tensor&,
PLMD::Tensor&, std::array<std::array<PLMD::TensorGeneric<3, 3>, 3>, 3>&)’:
RMSD.cpp:1134:6: internal compiler error: in maybe_canonicalize_mem_ref_addr,
at gimple-fold.c:4903
 1134 | void RMSDCoreData::doCoreCalcWithCloseStructure(bool safe,bool alEqDis,
Tensor & rotationPosClose, Tensor & rotationRefClose,
std::array<std::array<Tensor,3>,3> & drotationPosCloseDrr01) {
      |      ^~~~~~~~~~~~
0xa1bdac maybe_canonicalize_mem_ref_addr
       
/data/tiziano/cp2k/tools/toolchain/build/gcc-10.2.0/gcc/gimple-fold.c:4903
0xa20553 fold_stmt_1
       
/data/tiziano/cp2k/tools/toolchain/build/gcc-10.2.0/gcc/gimple-fold.c:5058
0xd26380 fold_marked_statements
       
/data/tiziano/cp2k/tools/toolchain/build/gcc-10.2.0/gcc/tree-inline.c:5342
0xd332b5 optimize_inline_calls(tree_node*)
       
/data/tiziano/cp2k/tools/toolchain/build/gcc-10.2.0/gcc/tree-inline.c:5448
0xac9f9e inline_transform(cgraph_node*)
       
/data/tiziano/cp2k/tools/toolchain/build/gcc-10.2.0/gcc/ipa-inline-transform.c:736
0xbf187e execute_one_ipa_transform_pass
        /data/tiziano/cp2k/tools/toolchain/build/gcc-10.2.0/gcc/passes.c:2233
0xbf187e execute_all_ipa_transforms(bool)
        /data/tiziano/cp2k/tools/toolchain/build/gcc-10.2.0/gcc/passes.c:2272
0x8dfdc4 cgraph_node::expand()
       
/data/tiziano/cp2k/tools/toolchain/build/gcc-10.2.0/gcc/cgraphunit.c:2293
0x8e117d expand_all_functions
       
/data/tiziano/cp2k/tools/toolchain/build/gcc-10.2.0/gcc/cgraphunit.c:2471
0x8e117d symbol_table::compile()
       
/data/tiziano/cp2k/tools/toolchain/build/gcc-10.2.0/gcc/cgraphunit.c:2822
0x8e325c symbol_table::finalize_compilation_unit()
       
/data/tiziano/cp2k/tools/toolchain/build/gcc-10.2.0/gcc/cgraphunit.c:3002
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.

[Bug c++/96354] [10 regression] ICE in maybe_canonicalize_mem_ref_addr, at gimple-fold.c:4903

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rguenth at gcc dot gnu.org
   Target Milestone|---                         |10.3

[Bug c++/96354] [10 regression] ICE in maybe_canonicalize_mem_ref_addr, at gimple-fold.c:4903

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |marxin at gcc dot gnu.org

--- Comment #3 from Martin Liška <marxin at gcc dot gnu.org> ---
Great, I was able to reproduce that. Working on that..

[Bug c++/96354] [10/11 Regression] ICE in maybe_canonicalize_mem_ref_addr, at gimple-fold.c:4903 since r10-2271-gd81ab49d0586fca0

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org
            Summary|[10 regression] ICE in      |[10/11 Regression] ICE in
                   |maybe_canonicalize_mem_ref_ |maybe_canonicalize_mem_ref_
                   |addr, at gimple-fold.c:4903 |addr, at gimple-fold.c:4903
                   |                            |since
                   |                            |r10-2271-gd81ab49d0586fca0
           Keywords|                            |ice-on-valid-code, openmp

--- Comment #4 from Martin Liška <marxin at gcc dot gnu.org> ---
Started with r10-2271-gd81ab49d0586fca0, I'm reducing that.

[Bug c++/96354] [10/11 Regression] ICE in maybe_canonicalize_mem_ref_addr, at gimple-fold.c:4903 since r10-2271-gd81ab49d0586fca0

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |NEW

--- Comment #5 from Martin Liška <marxin at gcc dot gnu.org> ---
Reduced test-case:

cat /tmp/RMSD.ii
template <int _Nm> struct __array_traits { typedef double _Type[_Nm]; };
template <int _Nm> struct array {
  typename __array_traits<_Nm>::_Type _M_elems;
  double *data() { return _M_elems; }
};
template <unsigned n> class VectorGeneric {
  array<n> d;
public:
  VectorGeneric();
};
template <unsigned n> VectorGeneric<n>::VectorGeneric() {
  double __trans_tmp_3 = *d.data();
}
template <unsigned n>
void operator-(VectorGeneric<n>, const VectorGeneric<n> &);
template <unsigned> class TensorGeneric {};
template <unsigned n, unsigned m>
VectorGeneric<n> matmul(TensorGeneric<n>, VectorGeneric<m>) {
  VectorGeneric<n> t;
  return t;
}
int doCoreCalc_n;
class RMSDCoreData {
  TensorGeneric<3> rotation;
  void doCoreCalc();
};
void RMSDCoreData::doCoreCalc() {
  for (unsigned iat;;)
#pragma omp simd
    for (iat = 0; iat < doCoreCalc_n; iat++) {
      VectorGeneric<3> __trans_tmp_2, __trans_tmp_4;
      __trans_tmp_4 - matmul(rotation, __trans_tmp_2);
    }
}

@Jakub: Can you please take a look?

[Bug c++/96354] [10/11 Regression] ICE in maybe_canonicalize_mem_ref_addr, at gimple-fold.c:4903 since r10-2271-gd81ab49d0586fca0

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Sure.

[Bug c++/96354] [10/11 Regression] ICE in maybe_canonicalize_mem_ref_addr, at gimple-fold.c:4903 since r10-2271-gd81ab49d0586fca0

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
So we have:
  # DEBUG this => &<retval>
  <retval> ={v} {CLOBBER};
  # DEBUG D#1 => &<retval>.d
  # DEBUG D#2 => MEM[(double *)&<retval>]
  # DEBUG __trans_tmp_3 => D#2
in matmul during inlining, and inline it as:
  # DEBUG this => &D.2697[_13]
  _24 = &D.2697[_13];
  *_24 ={v} {CLOBBER};
  # DEBUG D#1 => &D.2697[_13].d
  # DEBUG D#2 => MEM[(double *)&D.2697[_13]]
  # DEBUG __trans_tmp_3 => D#2
  # DEBUG this => NULL
Generally, we are more lax about IL restrictions we have in debug stmts
compared to other stmts, e.g. verify_gimple_debug always returns false.
So, do we want to allow this case?
I believe cfgexpand.c handles that:
    case MEM_REF:
      if (!is_gimple_mem_ref_addr (TREE_OPERAND (exp, 0)))
        {
          tree newexp = fold_binary (MEM_REF, TREE_TYPE (exp),
                                     TREE_OPERAND (exp, 0),
                                     TREE_OPERAND (exp, 1));
          if (newexp)
            return expand_debug_expr (newexp);
        }
      /* FALLTHROUGH */
    case INDIRECT_REF:
or punts if the folding doesn't come up with anything.
So, we could e.g. add bool is_debug argument to
maybe_canonicalize_mem_ref_addr, pass true only when in GIMPLE_DEBUG and
instead of failing the assertion there return false (what about the other
assertions in there?).
Or instead of bool is_debug take bool *is_debug and if non-NULL (only for
GIMPLE_DEBUG) set it to *false if we'd otherwise fail and tell that way to the
caller that it should reset the debug stmt?
richi, your thoughts on this?

[Bug c++/96354] [10/11 Regression] ICE in maybe_canonicalize_mem_ref_addr, at gimple-fold.c:4903 since r10-2271-gd81ab49d0586fca0

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354

--- Comment #8 from rguenther at suse dot de <rguenther at suse dot de> ---
On July 28, 2020 4:45:59 PM GMT+02:00, "jakub at gcc dot gnu.org"
<gcc-bugzilla@gcc.gnu.org> wrote:
>https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354
>
>--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
>So we have:
>  # DEBUG this => &<retval>
>  <retval> ={v} {CLOBBER};
>  # DEBUG D#1 => &<retval>.d
>  # DEBUG D#2 => MEM[(double *)&<retval>]
>  # DEBUG __trans_tmp_3 => D#2
>in matmul during inlining, and inline it as:
>  # DEBUG this => &D.2697[_13]
>  _24 = &D.2697[_13];
>  *_24 ={v} {CLOBBER};
>  # DEBUG D#1 => &D.2697[_13].d
>  # DEBUG D#2 => MEM[(double *)&D.2697[_13]]
>  # DEBUG __trans_tmp_3 => D#2
>  # DEBUG this => NULL
>Generally, we are more lax about IL restrictions we have in debug stmts
>compared to other stmts, e.g. verify_gimple_debug always returns false.
>So, do we want to allow this case?
>I believe cfgexpand.c handles that:
>    case MEM_REF:
>      if (!is_gimple_mem_ref_addr (TREE_OPERAND (exp, 0)))
>        {
>          tree newexp = fold_binary (MEM_REF, TREE_TYPE (exp),
>                                     TREE_OPERAND (exp, 0),
>                                     TREE_OPERAND (exp, 1));
>          if (newexp)
>            return expand_debug_expr (newexp);
>        }
>      /* FALLTHROUGH */
>    case INDIRECT_REF:
>or punts if the folding doesn't come up with anything.
>So, we could e.g. add bool is_debug argument to
>maybe_canonicalize_mem_ref_addr, pass true only when in GIMPLE_DEBUG
>and
>instead of failing the assertion there return false (what about the
>other
>assertions in there?).
>Or instead of bool is_debug take bool *is_debug and if non-NULL (only
>for
>GIMPLE_DEBUG) set it to *false if we'd otherwise fail and tell that way
>to the
>caller that it should reset the debug stmt?
>richi, your thoughts on this?

We could allow this in debug stmts but pretty much any (or very many) workers
on MEM_REFs will be quite unhappy about this. Of course we're not likely to run
them on debug stmts. But then, we do have debug Temps for this...

[Bug c++/96354] [10/11 Regression] ICE in maybe_canonicalize_mem_ref_addr, at gimple-fold.c:4903 since r10-2271-gd81ab49d0586fca0

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to rguenther@suse.de from comment #8)
> We could allow this in debug stmts but pretty much any (or very many)
> workers on MEM_REFs will be quite unhappy about this. Of course we're not
> likely to run them on debug stmts. But then, we do have debug Temps for
> this...

I was wondering if we are ok with even MEM_REF with DEBUG_EXPR_DECL as the
address, but seems at least this routine is ok with that one.
The question is what we would do with that at cfgexpand.c time.
But I guess I can look at where this is folded into it and see if I can break
it out into a debug expr decl and then see what expansion does for it.

[Bug c++/96354] [10/11 Regression] ICE in maybe_canonicalize_mem_ref_addr, at gimple-fold.c:4903 since r10-2271-gd81ab49d0586fca0

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|marxin at gcc dot gnu.org          |jakub at gcc dot gnu.org

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Slightly adjusted testcase:

template <int N> struct A { typedef double T[N]; };
template <int N> struct B { typename A<N>::T b; double *baz () { return b; } };
template <int N> struct C { B<N> d; C (); };
template <int N> C<N>::C () { double c = *d.baz (); }
template <int N> void operator- (C<N>, const C<N> &);
template <int> struct D {};
template <int N, int M> C<N> foo (D<N>, C<M>) { C<N> t; return t; }
int e;
struct E { D<3> d; void bar (); };

void
E::bar ()
{
#pragma omp simd
  for (int i = 0; i < e; i++)
    {
      C<3> f, g;
      g - foo (d, f);
    }
}

[Bug c++/96354] [10/11 Regression] ICE in maybe_canonicalize_mem_ref_addr, at gimple-fold.c:4903 since r10-2271-gd81ab49d0586fca0

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354

--- Comment #11 from rguenther at suse dot de <rguenther at suse dot de> ---
On Tue, 28 Jul 2020, jakub at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354
> 
> --- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> (In reply to rguenther@suse.de from comment #8)
> > We could allow this in debug stmts but pretty much any (or very many)
> > workers on MEM_REFs will be quite unhappy about this. Of course we're not
> > likely to run them on debug stmts. But then, we do have debug Temps for
> > this...
> 
> I was wondering if we are ok with even MEM_REF with DEBUG_EXPR_DECL as the
> address, but seems at least this routine is ok with that one.
> The question is what we would do with that at cfgexpand.c time.
> But I guess I can look at where this is folded into it and see if I can break
> it out into a debug expr decl and then see what expansion does for it.

It wouldn't be an address of a DEBUG_EXPR_DECL but the DEBUG_EXPR
defining the DEBUG_EXPR_DECL would be the address itself, thus

# DEBUG D.12 = &a[i_3];
# DEBUG a = MEM_REF[D.12];

[Bug debug/96354] [10/11 Regression] ICE in maybe_canonicalize_mem_ref_addr, at gimple-fold.c:4903 since r10-2271-gd81ab49d0586fca0

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 48949
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48949&action=edit
gcc11-pr96354.patch

Untested fix.

[Bug debug/96354] [10/11 Regression] ICE in maybe_canonicalize_mem_ref_addr, at gimple-fold.c:4903 since r10-2271-gd81ab49d0586fca0

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354

--- Comment #13 from Richard Biener <rguenth at gcc dot gnu.org> ---
(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 &<retval> 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 *)&<retval>]

and retval maps to D.2566[_9].  And I guess in regular code we never end
up with &<retval> 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

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354

--- Comment #14 from Richard Biener <rguenth at gcc dot gnu.org> ---
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)
{
  <bb 2> :
  C<3>::C (&<retval>);
  return <retval>;

and inlined from

C<3>::C (struct C * const this)
{
  <bb 2> [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 <result> 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

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354

--- Comment #15 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
We have indeed:
  # DEBUG D#2 => MEM[(double *)&<retval>]
and on the caller side:
  D.2566[_9] = foo<3, 3> (D.2559, D.2572); [return slot optimization]
So, that is why to &<retval> &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<C<3> > (&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 <int N, int M> C<N> foo (D<N>, C<M>) { C<N> t; double d = 1.25;
__builtin_memcpy (&t, &d, sizeof (double)); return t; }
instead so that before that inlining we get
  MEM <long unsigned int> [(char * {ref-all})&<retval>] = _4;
then it is indeed again gimple_regimplify_operands that fixes up the
MEM <long unsigned int> [(char * {ref-all})&D.2569[_9]] = 4608308318706860032;
into:
  _21 = &D.2569[_9];
  MEM <long unsigned int> [(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

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354

--- Comment #16 from rguenther at suse dot de <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 <jakub at gcc dot gnu.org> ---
> We have indeed:
>   # DEBUG D#2 => MEM[(double *)&<retval>]
> and on the caller side:
>   D.2566[_9] = foo<3, 3> (D.2559, D.2572); [return slot optimization]
> So, that is why to &<retval> &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<C<3> > (&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 <int N, int M> C<N> foo (D<N>, C<M>) { C<N> t; double d = 1.25;
> __builtin_memcpy (&t, &d, sizeof (double)); return t; }
> instead so that before that inlining we get
>   MEM <long unsigned int> [(char * {ref-all})&<retval>] = _4;
> then it is indeed again gimple_regimplify_operands that fixes up the
> MEM <long unsigned int> [(char * {ref-all})&D.2569[_9]] = 4608308318706860032;
> into:
>   _21 = &D.2569[_9];
>   MEM <long unsigned int> [(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

 <retval>.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

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354

--- Comment #17 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
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

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354

--- Comment #18 from rguenther at suse dot de <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 <jakub at gcc dot gnu.org> ---
> 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

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354

--- Comment #19 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
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

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354

--- Comment #20 from rguenther at suse dot de <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 <jakub at gcc dot gnu.org> ---
> 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

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354

--- Comment #21 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
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

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354

--- Comment #22 from Richard Biener <rguenth at gcc dot gnu.org> ---
(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

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354

--- Comment #23 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:fabe0ede9db9fa95832b2329d3d6156711905e20

commit r11-2539-gfabe0ede9db9fa95832b2329d3d6156711905e20
Author: Jakub Jelinek <jakub@redhat.com>
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  <jakub@redhat.com>

            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) <case GIMPLE_DEBUG>: 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

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354

--- Comment #24 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:49643954ffa71ddeec8e979e05f145ea688027f9

commit r10-8569-g49643954ffa71ddeec8e979e05f145ea688027f9
Author: Jakub Jelinek <jakub@redhat.com>
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  <jakub@redhat.com>

            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) <case GIMPLE_DEBUG>: 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

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96354

Richard Biener <rguenth at gcc dot gnu.org> 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 <rguenth at gcc dot gnu.org> ---
Fixed.