Re: [PATCH] Fix PR57235

[Richard Biener <rguenther at suse dot de>]

On Mon, 13 May 2013, Richard Biener wrote:

> 
> This fixes a virtual SSA updating problem with sinking clobbers.
> Namely when sinking into a block with multiple predecessors and
> no virtual use we lack a convenient PHI node that serves as a
> merge of the virtual operands from the predecessors.  The following
> patch gives up sinking clobbers in this case (alternatively
> we can remove all clobbers).  Any preference here?  I'm wondering
> when sinking a clobber into a block with multiple preds is good.

After discussion on IRC I re-instantiated the virtual operand
renaming case to recover in this case.  We'll sometimes rename
vops without good reason though as the CFG is not cleaned up
during the sinking and thus SSA form is not up-to-date.

Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.

Richard.

2013-05-13  Richard Biener  <rguenther@suse.de>

	PR middle-end/57235
	* tree-eh.c (sink_clobbers): Give up for successors with
	multiple predecessors and no virtual uses.

	* g++.dg/torture/pr57235.C: New testcase.

Index: gcc/tree-eh.c
===================================================================
--- gcc/tree-eh.c	(revision 198815)
+++ gcc/tree-eh.c	(working copy)
@@ -3312,6 +3312,7 @@ sink_clobbers (basic_block bb)
   gimple_stmt_iterator gsi, dgsi;
   basic_block succbb;
   bool any_clobbers = false;
+  unsigned todo = 0;
 
   /* Only optimize if BB has a single EH successor and
      all predecessor edges are EH too.  */
@@ -3410,9 +3411,22 @@ sink_clobbers (basic_block bb)
 	  SET_USE (PHI_ARG_DEF_PTR_FROM_EDGE (vphi, succe), gimple_vuse (stmt));
 	  SET_USE (gimple_vuse_op (stmt), vuse);
 	}
+      /* If there isn't a single predecessor but no virtual PHI node
+         arrange for virtual operands to be renamed.  */
+      else if (gimple_vuse_op (stmt) != NULL_USE_OPERAND_P
+	       && !single_pred_p (succbb))
+	{
+	  /* In this case there will be no use of the VDEF of this stmt.
+	     ???  Unless this is a secondary opportunity and we have not
+	     removed unreachable blocks yet, so we cannot assert this.
+	     Which also means we will end up renaming too many times.  */
+	  SET_USE (gimple_vuse_op (stmt), gimple_vop (cfun));
+	  mark_virtual_operands_for_renaming (cfun);
+	  todo |= TODO_update_ssa_only_virtuals;
+	}
     }
 
-  return 0;
+  return todo;
 }
 
 /* At the end of inlining, we can lower EH_DISPATCH.  Return true when 
Index: gcc/testsuite/g++.dg/torture/pr57235.C
===================================================================
--- gcc/testsuite/g++.dg/torture/pr57235.C	(revision 0)
+++ gcc/testsuite/g++.dg/torture/pr57235.C	(working copy)
@@ -0,0 +1,156 @@
+// { dg-do compile }
+
+namespace std
+{
+  template < class _Elem > struct char_traits
+  {
+  };
+  struct _Container_base
+  {
+  };
+    template < class _Ty > struct _Allocator_base
+  {
+  };
+    template < class _Ty > class allocator:public _Allocator_base < _Ty >
+  {
+  };
+  class _String_base:public _Container_base
+  {
+  };
+template < class _Ty, class _Alloc > class _String_val:public _String_base
+  {
+  };
+template < class _Elem, class _Traits, class _Ax > class basic_string:public _String_val < _Elem,
+    _Ax
+    >
+  {
+  public:typedef basic_string < _Elem, _Traits, _Ax > _Myt;
+    typedef _String_val < _Elem, _Ax > _Mybase;
+    basic_string (const _Elem * _Ptr):_Mybase ()
+    {
+    }
+  };
+  typedef basic_string < char, char_traits < char >,
+    allocator < char > >string;
+}
+
+
+namespace google
+{
+  namespace protobuf
+  {
+    namespace internal
+    {
+      template < class C > class scoped_ptr
+      {
+      public:typedef C element_type;
+      explicit scoped_ptr (C * p = __null):ptr_ (p)
+	{
+	}
+	 ~scoped_ptr ()
+	{
+	  delete ptr_;
+	}
+	C *get () const
+	{
+	  return ptr_;
+	}
+      private:  C * ptr_;
+      };
+    }
+    using internal::scoped_ptr;
+    enum LogLevel
+    {
+      LOGLEVEL_INFO, LOGLEVEL_WARNING, LOGLEVEL_ERROR, LOGLEVEL_FATAL,
+	LOGLEVEL_DFATAL = LOGLEVEL_ERROR
+    };
+    namespace internal
+    {
+      class LogMessage
+      {
+      public:LogMessage (LogLevel level, const char *filename,
+		    int line);
+	 ~LogMessage ();
+	  LogMessage & operator<< (const std::string & value);
+      };
+      class LogFinisher
+      {
+      public:void operator= (LogMessage & other);
+      };
+    }
+    using namespace std;
+    class Descriptor
+    {
+    };
+    class FieldDescriptor
+    {
+    public:
+      const Descriptor *message_type () const;
+      string DebugString () const;
+    };
+    class MessageLite
+    {
+    };
+    class Message:public MessageLite
+    {
+    public:inline Message ()
+      {
+      }
+      virtual ~ Message ();
+      virtual Message *New () const = 0;
+    };
+    class MessageFactory
+    {
+    };
+    class UnknownFieldSet
+    {
+    };
+    class DynamicMessageFactory:public MessageFactory
+    {
+    public:DynamicMessageFactory ();
+      const Message *GetPrototype (const Descriptor * type);
+    };
+    namespace io
+    {
+      class ErrorCollector
+      {
+      public:inline ErrorCollector ()
+	{
+	}
+	virtual ~ ErrorCollector ();
+      };
+    }
+    class DescriptorBuilder
+    {
+      class OptionInterpreter
+      {
+	bool SetAggregateOption (const FieldDescriptor * option_field,
+				 UnknownFieldSet * unknown_fields);
+	DynamicMessageFactory dynamic_factory_;
+      };
+    };
+    namespace
+    {
+      class AggregateErrorCollector:public io::ErrorCollector
+      {
+      };
+    }
+    bool DescriptorBuilder::OptionInterpreter::
+      SetAggregateOption (const FieldDescriptor * option_field,
+			  UnknownFieldSet * unknown_fields)
+    {
+      const Descriptor *type = option_field->message_type ();
+      scoped_ptr < Message >
+	dynamic (dynamic_factory_.GetPrototype (type)->New ());
+      !(!(dynamic.get () !=
+	  __null)) ? (void) 0 : ::google::protobuf::internal::
+	LogFinisher () =::google::protobuf::internal::LogMessage (::google::
+								  protobuf::
+								  LOGLEVEL_FATAL,
+								  "descriptor.cc",
+								  4396) <<
+	option_field->DebugString ();
+      AggregateErrorCollector collector;
+    }
+  }
+}