diff mbox

Don't combine instructions if they can't be copied

Message ID 4CC51F33.90209@codesourcery.com
State New
Headers show

Commit Message

Jie Zhang Oct. 25, 2010, 6:09 a.m. UTC 
On 10/23/2010 10:56 PM, Eric Botcazou wrote:
>> With this patch, GCC will not combine instructions if they can't be
>> copied. Tested on arm-linux-gnueabi with c,c++,lto. No regressions are
>> found.
>>
>> Is it OK?
>
>  From the above description, as well as the ChangeLog, I would have said no,
> this is too broad.  But the implementation is actually more fine-grained and
> looks sensible to me.  So OK for mainline if (1) you add a comment to the code
> explaining its purpose, e.g. "We are about to copy insns for the case where
> they need to be kept around.  Check that..." and (2) you fix the ChangeLog.
>
> No need to resubmit.
>
Thanks for review. I'm going to commit the attached patch soon with the 
above two points addressed.


Regards,

Comments

Jie Zhang Oct. 25, 2010, 12:14 p.m. UTC | #1 
On 10/25/2010 02:09 PM, Jie Zhang wrote:
> 	testsuite/
> 	g++.dg/opt/combine.c: New test.

I just found a leading "*" was missing in this entry. I have committed a 
fix for it.


Regards,
diff mbox

Patch


	* combine.c (try_combine): If insns need to be kept around,
	check that they can be copied in the merged instruction.

	testsuite/
	g++.dg/opt/combine.c: New test.

Index: testsuite/g++.dg/opt/combine.C
===================================================================
--- testsuite/g++.dg/opt/combine.C	(revision 0)
+++ testsuite/g++.dg/opt/combine.C	(revision 0)
@@ -0,0 +1,72 @@ 
+// { dg-do assemble { target fpic } }
+// { dg-options "-O2 -fweb -fPIC -fvisibility=hidden" }
+
+class QBasicAtomicInt
+{
+public:
+  volatile int _q_value;
+  inline operator int () const {return _q_value;}
+};
+class QVariant;
+class QScriptContext;
+class QScriptEngine;
+class QScriptValue
+{
+public:
+  QVariant toVariant () const;
+};
+class QScriptDebuggerBackendPrivate
+{
+  static QScriptValue trace (QScriptContext *context);
+};
+template <typename T> struct QMetaTypeId { };
+template <typename T> struct QMetaTypeId2
+{
+  static inline int qt_metatype_id ()
+  {
+    return QMetaTypeId<T>::qt_metatype_id () ;
+  }
+};
+template <typename T> inline int qMetaTypeId (T * = 0)
+{
+  return QMetaTypeId2<T>::qt_metatype_id () ;
+}
+class QVariant { };
+template<typename T> inline T qvariant_cast (const QVariant &v)
+{
+  const int vid = qMetaTypeId<T> ((0)) ;
+};
+class QScriptContext
+{
+public: 
+  QScriptValue callee () const;
+};
+class QScriptEngine  
+{
+public:
+  static bool convertV2 (const QScriptValue &value , int type , void *ptr) ;
+};
+inline bool qscriptvalue_cast_helper (const QScriptValue &value , int type , void *ptr)
+{
+  return QScriptEngine::convertV2 (value, type, ptr) ;
+}
+template<typename T> T qscriptvalue_cast (const QScriptValue &value)
+{
+  T t;
+  const int id = qMetaTypeId<T> () ;
+  if ( qscriptvalue_cast_helper (value, id, &t))
+    return qvariant_cast<T> (value.toVariant ()) ;
+}
+template <> struct QMetaTypeId< QScriptDebuggerBackendPrivate* >
+{
+  static int qt_metatype_id ()
+  {
+    static QBasicAtomicInt metatype_id = { (0) };
+    return metatype_id;
+  }
+};
+QScriptValue QScriptDebuggerBackendPrivate::trace (QScriptContext *context)
+{
+  QScriptValue data = context->callee () ;
+  QScriptDebuggerBackendPrivate *self = qscriptvalue_cast<QScriptDebuggerBackendPrivate*> (data) ;
+}
Index: combine.c
===================================================================
--- combine.c	(revision 165910)
+++ combine.c	(working copy)
@@ -2917,6 +2917,18 @@  try_combine (rtx i3, rtx i2, rtx i1, rtx
   else
     added_sets_0 = 0;
 
+  /* We are about to copy insns for the case where they need to be kept
+     around.  Check that they can be copied in the merged instruction.  */
+
+  if (targetm.cannot_copy_insn_p
+      && ((added_sets_2 && targetm.cannot_copy_insn_p (i2))
+	  || (i1 && added_sets_1 && targetm.cannot_copy_insn_p (i1))
+	  || (i0 && added_sets_0 && targetm.cannot_copy_insn_p (i0))))
+    {
+      undo_all ();
+      return 0;
+    }
+
   /* If the set in I2 needs to be kept around, we must make a copy of
      PATTERN (I2), so that when we substitute I1SRC for I1DEST in
      PATTERN (I2), we are only substituting for the original I1DEST, not into