trippels@CFARM-IUT-TLSE3 ~ % cat mhtml_generation_manager.ii
template <typename> class A;
struct B {
  using pointer = int *;
};
template <typename _CharT, typename = A<_CharT>> class basic_string {
  long _M_string_length;
  enum { _S_local_capacity = 15 } _M_local_buf[_S_local_capacity];
  B::pointer _M_local_data;

public:
  ~basic_string();
};
template <typename _CharT, typename _Traits, typename _Alloc>
int operator<<(_Traits, basic_string<_CharT, _Alloc>);
class C {
  basic_string<A<char>> _M_string;
};
class D {
  C _M_stringbuf;
};
class F {
  int stream;
  D stream_;
};
class G {
public:
  void operator&(int);
};
class H {
public:
  H(unsigned);
  H(H &&);
  bool m_fn1();
};
class I {
  void m_fn2(const int &&);
  static H m_fn3(const int &);
};
template <typename Functor> void Bind(Functor);
class J {
public:
  static basic_string<char> m_fn4();
};
int a;
void I::m_fn2(const int &&) { Bind(m_fn3); }
H I::m_fn3(const int &) {
  !false ? (void)0 : G() & F() << J::m_fn4();
  H b(a);
  if (b.m_fn1())
    F();
}

trippels@CFARM-IUT-TLSE3 ~ % g++ -c -O2 mhtml_generation_manager.ii
mhtml_generation_manager.ii: In static member function ‘static H I::m_fn3(const int&)’:
mhtml_generation_manager.ii:46:3: internal compiler error: in assign_temp, at function.c:961

This is a problem with the gimple call generated in split_function.  It considers functions that return by invisible reference and calls gimple_call_set_return_slot_opt, but doesn't then set the LHS of the call, so we end up allocating a temporary stack slot for it, which is wrong for a TREE_ADDRESSABLE type.

It seems that the LHS doesn't get set because m_fn3 doesn't actually have a return statement, but for some reason return_bb is different from the exit block, so we don't get to the code later in split_function that creates a return statement.

Changing component to IPA.

The assertion started to fail with the merge of the C++ delayed folding branch.
r230365

*** Bug 69273 has been marked as a duplicate of this bug. ***

*** Bug 69438 has been marked as a duplicate of this bug. ***

The dup PR69438 has a smaller testcase.

Most reduced testcase:

  __attribute__((noreturn)) void V(int);
  struct R{R(const R&r){}};
  R f(){V(0);}
  R c(){V(0);}

Slightly more reduced [2 bytes less? ;-)]...

__attribute__((noreturn))void V(int);
struct R{R(const R&){}};
R f(){V(0);}
R c(){V(0);}



This might be the most-reduced-possible form of this test case.  Experimentation shows that:

  * removing the second function definition removes the ICE
  * removing the user-defined copy ctor     removes the ICE
  * removing the int param. from 'V'        removes the ICE

Further-reduced test case [13 bytes shorter: 76 bytes with 1-byte line endings]...



[[noreturn]]void V(int);
struct R{R(const R&){}};
R f(){V(0);}
R c(){V(0);}



Additional notes:

  * removing "__attribute__((noreturn))" from the original reduction removes the ICE

  * removing "[[noreturn]]"              from the above    reduction removes the ICE

  * compiling with "-O0" or "-O1" is OK [no ICE]; "-O2", "-O3", and "-Os" all lead to ICE

Adding either of the following flags to "-O1" causes the compiler to ICE on the most-reduced test case; adding any of the other "-f<...>" flags I tested [39 of them including the following 2] did not enable the ICE:

  -fipa-icf
  -fipa-icf-functions


Using either of those flags results in exactly the same ICE with exactly the same output...



C++11_v1_.cpp: In function ‘R f()’:
C++11_v1_.cpp:3:3: internal compiler error: in assign_temp, at function.c:961
 R f(){V(0);}
   ^

0xa2215c assign_temp(tree_node*, int, int)
        ../../gcc/function.c:961
0x8b2611 expand_call(tree_node*, rtx_def*, int)
        ../../gcc/calls.c:2559
0x9b72d2 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool)
        ../../gcc/expr.c:10575
0x8c350c expand_expr
        ../../gcc/expr.h:256
0x8c350c expand_call_stmt
        ../../gcc/cfgexpand.c:2648
0x8c350c expand_gimple_stmt_1
        ../../gcc/cfgexpand.c:3536
0x8c350c expand_gimple_stmt
        ../../gcc/cfgexpand.c:3702
0x8c5801 expand_gimple_tailcall
        ../../gcc/cfgexpand.c:3749
0x8c5801 expand_gimple_basic_block
        ../../gcc/cfgexpand.c:5685
0x8cac06 execute
        ../../gcc/cfgexpand.c:6323
Please submit a full bug report,
with preprocessed source if appropriate.



Of note, "-fipa-icf-variables" is one of those that does _not_ trigger the ICE when added to "-O1".

More reduced test case, that does not depend on -ipa-icf:

struct R
{
  R (const R&) { }
};

__attribute__ ((noreturn)) R f ();

R
c ()
{
  f ();
}

Untested fix:

diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index ce1e712..e07cd04 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -4830,7 +4830,8 @@ gimplify_modify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
            }
        }
       notice_special_calls (call_stmt);
-      if (!gimple_call_noreturn_p (call_stmt))
+      if (!gimple_call_noreturn_p (call_stmt)
+         || gimple_call_return_slot_opt_p (call_stmt))
        gimple_call_set_lhs (call_stmt, *to_p);
       assign = call_stmt;
     }

(In reply to Patrick Palka from comment #11)
> More reduced test case, that does not depend on -ipa-icf:
> 
> struct R
> {
>   R (const R&) { }
> };
> 
> __attribute__ ((noreturn)) R f ();
> 
> R
> c ()
> {
>   f ();
> }
> 
> Untested fix:
> 
> diff --git a/gcc/gimplify.c b/gcc/gimplify.c
> index ce1e712..e07cd04 100644
> --- a/gcc/gimplify.c
> +++ b/gcc/gimplify.c
> @@ -4830,7 +4830,8 @@ gimplify_modify_expr (tree *expr_p, gimple_seq *pre_p,
> gimple_seq *post_p,
>             }
>         }
>        notice_special_calls (call_stmt);
> -      if (!gimple_call_noreturn_p (call_stmt))
> +      if (!gimple_call_noreturn_p (call_stmt)
> +         || gimple_call_return_slot_opt_p (call_stmt))
>         gimple_call_set_lhs (call_stmt, *to_p);
>        assign = call_stmt;
>      }

Actually in light of #c2 this might be a separate but related issue.

(In reply to Patrick Palka from comment #12)
> (In reply to Patrick Palka from comment #11)
> > More reduced test case, that does not depend on -ipa-icf:
> > 
> > struct R
> > {
> >   R (const R&) { }
> > };
> > 
> > __attribute__ ((noreturn)) R f ();
> > 
> > R
> > c ()
> > {
> >   f ();
> > }
> > 
> > Untested fix:
> > 
> > diff --git a/gcc/gimplify.c b/gcc/gimplify.c
> > index ce1e712..e07cd04 100644
> > --- a/gcc/gimplify.c
> > +++ b/gcc/gimplify.c
> > @@ -4830,7 +4830,8 @@ gimplify_modify_expr (tree *expr_p, gimple_seq *pre_p,
> > gimple_seq *post_p,
> >             }
> >         }
> >        notice_special_calls (call_stmt);
> > -      if (!gimple_call_noreturn_p (call_stmt))
> > +      if (!gimple_call_noreturn_p (call_stmt)
> > +         || gimple_call_return_slot_opt_p (call_stmt))
> >         gimple_call_set_lhs (call_stmt, *to_p);
> >        assign = call_stmt;
> >      }
> 
> Actually in light of #c2 this might be a separate but related issue.

It may be better to check

    TREE_ADDRESSABLE (TREE_TYPE (*to_p))

instead of checking

    gimple_call_return_slot_opt_p (call_stmt)

Because with the latter, we trigger a gimple verification error "LHS in noreturn call" for the following test case, which utilizes the return-slot optimization but whose TREE_TYPE in question is not TREE_ADDRESSABLE.

struct R
{
  int x[100];
};

__attribute__ ((noreturn)) R f ();

void
c ()
{
  f ();
}

*** Bug 69649 has been marked as a duplicate of this bug. ***

*** Bug 69649 has been marked as a duplicate of this bug. ***

Created attachment 37565 [details]
gcc6-pr69241.patch

So, it seems we have at least 3 different issues, the attached patch fixes two of them, there is one remaining somewhere in ipa-split.c.

(In reply to Jakub Jelinek from comment #16)
> Created attachment 37565 [details]
> gcc6-pr69241.patch
> 
> So, it seems we have at least 3 different issues, the attached patch fixes
> two of them, there is one remaining somewhere in ipa-split.c.

Your testcase from PR69649 still fails with the patch.

Here is Jakub's testcase. It it shorter and nicer than that from comment1.

struct A { virtual void m1 (); };
struct C : A { void m1 () { m1 (); } };
template <class T> struct B
{
  T *t;
  B (T *x) : t (x) { if (t) t->m1 (); }
  B (const B &);
};
struct D : public C {};
struct F : public D
{
  virtual B<D> m2 ();
  virtual B<D> m3 ();
  int m4 ();
};
struct G : F
{
  B<D> m2 ();
  B<D> m3 ();
};
B<D> G::m2 ()
{
  if (m4 () == 0) return this;
  return 0;
}
B<D> G::m3 ()
{
  if (m4 () == 0) return this;
  return 0;
}

(In reply to Markus Trippelsdorf from comment #17)
> (In reply to Jakub Jelinek from comment #16)
> > Created attachment 37565 [details]
> > gcc6-pr69241.patch
> > 
> > So, it seems we have at least 3 different issues, the attached patch fixes
> > two of them, there is one remaining somewhere in ipa-split.c.
> 
> Your testcase from PR69649 still fails with the patch.

I know.  But fnsplit is a mess.  I'd think if !split_part_set_retval, we have two different cases.  If the split part really doesn't set the return value in any way and for DECL_BY_REFERENCE RESULT_DECL doesn't even use the corresponding default def SSA_NAME, it might be best to arrange for the *.part.* function to have void return value rather than returning the aggregate, but not sure how hard it would be to convince the cloning infrastructure about that.

And for the case where the SSA_NAME is used, but it is not used inside of MEM_REF (then it is treated as nonssa use and split_part_set_retval is true), then perhaps consider_split should be smarter.
Consider:
struct A { A(); ~A (); };
void bar (A *) throw ();
volatile int v;

static inline A
foo (bool x)
{
  A a;
  if (x)
    {
      v++; v++; v++; v++; v++;
      bar (&a);
    }
  return a;
}

A
bar (bool x)
{
  return foo (x);
}

A
baz (bool x)
{
  return foo (x);
}
where at -O2 we currently give up on the partial inlining, because:
Refused: need to pass non-param values
That is:
  if (num_args != bitmap_count_bits (current->ssa_names_to_pass))
and the difference is the DECL_BY_REFERENCE RESULT_DECL default def SSA_NAME.
Perhaps consider_split should take that into account and force split_part_set_retval in that case?

Ah, as for whether versioning supports changing return type to void, clearly it supports it and even fnsplit uses it for the !split_part_return_p.

So perhaps:

--- ipa-split.c.jj1	2016-01-04 14:55:52.000000000 +0100
+++ ipa-split.c	2016-02-03 13:01:45.905136051 +0100
@@ -1254,7 +1254,7 @@ split_function (basic_block return_bb, s
       else
 	main_part_return_p = true;
     }
-  /* The main part also returns if we we split on a fallthru edge
+  /* The main part also returns if we split on a fallthru edge
      and the split part returns.  */
   if (split_part_return_p)
     FOR_EACH_EDGE (e, ei, split_point->entry_bb->preds)
@@ -1364,8 +1364,9 @@ split_function (basic_block return_bb, s
   /* Now create the actual clone.  */
   cgraph_edge::rebuild_edges ();
   node = cur_node->create_version_clone_with_body
-    (vNULL, NULL, args_to_skip, !split_part_return_p, split_point->split_bbs,
-     split_point->entry_bb, "part");
+    (vNULL, NULL, args_to_skip,
+     !split_part_return_p || !split_point->split_part_set_retval,
+     split_point->split_bbs, split_point->entry_bb, "part");
 
   node->split_part = true;
 
(completely untested, but fixes the testcase).  And as incremental step there could be what I spoke above, handle SSA uses of DECL_BY_REFERENCE RESULT_DECL
as artificial "parameter".

Author: jakub
Date: Wed Feb 10 15:06:20 2016
New Revision: 233271

URL: https://gcc.gnu.org/viewcvs?rev=233271&root=gcc&view=rev
Log:
	PR ipa/69241
	PR c++/69649
	* gimplify.c (gimplify_modify_expr): Set lhs even for noreturn
	calls if the return type is TREE_ADDRESSABLE.
	* cgraphunit.c (cgraph_node::expand_thunk): Likewise.
	* ipa-split.c (split_function): Fix doubled "we" in comment.
	Use void return type for the split part even if
	!split_point->split_part_set_retval.

	* g++.dg/ipa/pr69241-1.C: New test.
	* g++.dg/ipa/pr69241-2.C: New test.
	* g++.dg/ipa/pr69241-3.C: New test.
	* g++.dg/ipa/pr69649.C: New test.

Added:
    trunk/gcc/testsuite/g++.dg/ipa/pr69241-1.C
    trunk/gcc/testsuite/g++.dg/ipa/pr69241-2.C
    trunk/gcc/testsuite/g++.dg/ipa/pr69241-3.C
    trunk/gcc/testsuite/g++.dg/ipa/pr69649.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/cgraphunit.c
    trunk/gcc/gimplify.c
    trunk/gcc/ipa-split.c
    trunk/gcc/testsuite/ChangeLog

Fixed.

(In reply to Jakub Jelinek from comment #22)
> Fixed.

Unfortunately, no.
Chromium and the testcase from comment1 still ICE.

The #c1 testcase doesn't bother to return a value from the function, does Chromium has similar garbage in it?

(In reply to Jakub Jelinek from comment #24)
> The #c1 testcase doesn't bother to return a value from the function, does
> Chromium has similar garbage in it?

Actually, the chromium file, from which the testcase was reduced,
compiles fine now.
I will run a full chromium build and close this bug if it succeeds.

Created attachment 37654 [details]
gcc6-pr69241.patch

Well, we shouldn't ICE even on questionable testcases where -Wreturn-type complains on them.  Untested patch that ought to fix that.

Author: jakub
Date: Fri Feb 12 11:59:00 2016
New Revision: 233375

URL: https://gcc.gnu.org/viewcvs?rev=233375&root=gcc&view=rev
Log:
	PR ipa/69241
	* ipa-split.c (split_function): If split part returns TREE_ADDRESSABLE
	type by reference, force lhs on the call.

	* g++.dg/ipa/pr69241-4.C: New test.

Added:
    trunk/gcc/testsuite/g++.dg/ipa/pr69241-4.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/ipa-split.c
    trunk/gcc/testsuite/ChangeLog

Fixed.

*** Bug 69817 has been marked as a duplicate of this bug. ***

On IRC someone pointed to a new variant (huge Boost testcase):





markus@x4 tmp % cat subprocess.ii
namespace std {
template <typename _Tp, _Tp> struct integral_constant {
  static constexpr _Tp value = 0;
};
template <typename> struct conditional;
template <typename...> struct __or_;
template <typename _B1, typename _B2>
struct __or_<_B1, _B2> : conditional<_B2>::type {};
template <typename> struct is_enum : integral_constant<bool, __is_enum(int)> {};
template <typename> struct remove_reference;
template <typename _Tp> struct remove_reference<_Tp &> { typedef _Tp type; };
template <typename _Tp> struct decay {
  typedef typename remove_reference<_Tp>::type type;
};
template <typename> struct conditional { typedef is_enum<char> type; };
template <typename _Head> struct _Head_base { _Head _M_head_impl; };
template <unsigned long, typename...> struct _Tuple_impl;
template <unsigned long _Idx, typename _Head, typename... _Tail>
struct _Tuple_impl<_Idx, _Head, _Tail...> : _Tuple_impl<1, _Tail...>,
                                            _Head_base<_Head> {
  _Tuple_impl(_Tuple_impl &) = default;
  _Tuple_impl(_Tuple_impl &&__in) : _Tuple_impl(__in) {}
};
template <unsigned long _Idx, typename _Head>
struct _Tuple_impl<_Idx, _Head> {};
template <typename... _Elements> struct tuple : _Tuple_impl<0, _Elements...> {};
template <typename> struct _Bind;
template <typename _Functor, typename... _Bound_args>
struct _Bind<_Functor(_Bound_args...)> {
  tuple<_Bound_args...> _M_bound_args;
};
template <typename _Tp, typename _Tp2 = typename decay<_Tp>::type>
using __is_socketlike = __or_<integral_constant<bool, false>, is_enum<_Tp2>>;
template <int, typename... _BoundArgs> struct _Bind_helper {
  typedef _Bind<int(typename decay<_BoundArgs>::type...)> type;
};
template <typename _Func, typename... _BoundArgs>
typename _Bind_helper<__is_socketlike<_Func>::value, _BoundArgs...>::type
bind(_Func &&, _BoundArgs &&...);
struct _Any_data {
  template <typename _Tp> _Tp _M_access() const;
};
enum _Manager_operation {};
template <typename> struct function;
struct _Function_base {
  template <typename _Functor> struct _Base_manager {
    static bool _M_manager(_Any_data &, const _Any_data &__source,
                           _Manager_operation) {
      _Functor(*__source._M_access<_Functor *>());
    }
  };
  typedef bool (*_Manager_type)(_Any_data &, const _Any_data &,
                                _Manager_operation);
  _Manager_type _M_manager;
};
template <typename, typename> struct _Function_handler;
template <typename _Res, typename _Functor, typename... _ArgTypes>
struct _Function_handler<_Res(_ArgTypes...), _Functor>
    : _Function_base::_Base_manager<_Functor> {};
template <typename _Res, typename... _ArgTypes>
struct function<_Res(_ArgTypes...)> : _Function_base {
  template <typename, typename> using _Requires = int;
  template <typename _Functor, typename = _Requires<int, void>,
            typename = _Requires<int, void>>
  function(_Functor);
  _Res operator()(_ArgTypes...);
};
template <typename _Res, typename... _ArgTypes>
template <typename _Functor, typename, typename>
function<_Res(_ArgTypes...)>::function(_Functor) {
  _M_manager = _Function_handler<_Res(), _Functor>::_M_manager;
}
}
struct Option {
  Option(Option &);
};
using std::bind;
using std::function;
struct Subprocess {};
using InputFileDescriptors = Subprocess;
using OutputFileDescriptors = int;
void defaultClone(Option setup) {
  InputFileDescriptors stdinfds;
  OutputFileDescriptors stdoutfds, envp;
  function<int(function<int()>)> clone = 0;
  int pipes = clone(bind(envp, setup, stdinfds, stdoutfds, pipes));
}

markus@x4 tmp % g++ -c subprocess.ii
subprocess.ii: In copy constructor ‘std::_Tuple_impl<_Idx, _Head, _Tail ...>::_Tuple_impl(std::_Tuple_impl<_Idx, _Head, _Tail ...>&) [with long unsigned int _Idx = 0ul; _Head = Option; _Tail = {Subprocess, int, int}]’:
subprocess.ii:21:3: internal compiler error: in assign_temp, at function.c:961
   _Tuple_impl(_Tuple_impl &) = default;
   ^~~~~~~~~~~
0xa51353 assign_temp(tree_node*, int, int)
        ../../gcc/gcc/function.c:961
0x9e9e25 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool)
        ../../gcc/gcc/expr.c:10408
0x9f9048 expand_normal
        ../../gcc/gcc/expr.h:262
0x9f9048 store_field
        ../../gcc/gcc/expr.c:6679
0x9f4c7d expand_assignment(tree_node*, tree_node*, bool)
        ../../gcc/gcc/expr.c:5021
0x8e79de expand_gimple_stmt_1
        ../../gcc/gcc/cfgexpand.c:3606
0x8e79de expand_gimple_stmt
        ../../gcc/gcc/cfgexpand.c:3702
0x8e9e88 expand_gimple_basic_block
        ../../gcc/gcc/cfgexpand.c:5708
0x8efe96 execute
        ../../gcc/gcc/cfgexpand.c:6323

(In reply to Markus Trippelsdorf from comment #30)
> On IRC someone pointed to a new variant (huge Boost testcase):

That looks unrelated to this PR, it is not about missing lhs in a call that returns non-POD, but it is ICE during expansion of some weirdo assignment statement.  So IMHO it should be tracked separately.

I've opened PR69851.