Patchwork [trans-mem] PR47746: handle OBJ_TYPE_REF correctly

login
register
mail settings
Submitter Aldy Hernandez
Date Feb. 21, 2011, 10:41 p.m.
Message ID <4D62EA2F.7090306@redhat.com>
Download mbox | patch
Permalink /patch/83884/
State New
Headers show

Comments

Aldy Hernandez - Feb. 21, 2011, 10:41 p.m.
While building the call to TM_GETTMCLONE, we discard OBJ_TYPE_REF's if 
we can't fold them:

   /* Discard OBJ_TYPE_REF, since we weren't able to fold it.  */
   if (TREE_CODE (old_fn) == OBJ_TYPE_REF)
     old_fn = OBJ_TYPE_REF_EXPR (old_fn);

However, this discard can get rid of an implicit conversion.  In the 
attached testcase, the inner type of the OBJ_TYPE_REF_EXPR is signed, 
but the type of the OBJ_TYPE_REF_EXPR is unsigned.  This causes an ICE 
while verifying the gimple call.

I have fixed this by adding an appropriate conversion after the 
GETTMCLONE call.

I can make sure the conversion only happens for the OBJ_TYPE_REF, but I 
was lazy.  I don't think it'll hurt.

OK for branch?
PR 47746
	* trans-mem.c (ipa_tm_insert_gettmclone_call): Verify type
	compatibility in call.
Richard Henderson - Feb. 21, 2011, 10:45 p.m.
On 02/21/2011 02:41 PM, Aldy Hernandez wrote:
> 	PR 47746
> 	* trans-mem.c (ipa_tm_insert_gettmclone_call): Verify type
> 	compatibility in call.

Ok.


r~
Richard Guenther - Feb. 22, 2011, 10:07 a.m.
On Mon, Feb 21, 2011 at 11:41 PM, Aldy Hernandez <aldyh@redhat.com> wrote:
> While building the call to TM_GETTMCLONE, we discard OBJ_TYPE_REF's if we
> can't fold them:
>
>  /* Discard OBJ_TYPE_REF, since we weren't able to fold it.  */
>  if (TREE_CODE (old_fn) == OBJ_TYPE_REF)
>    old_fn = OBJ_TYPE_REF_EXPR (old_fn);
>
> However, this discard can get rid of an implicit conversion.  In the
> attached testcase, the inner type of the OBJ_TYPE_REF_EXPR is signed, but
> the type of the OBJ_TYPE_REF_EXPR is unsigned.  This causes an ICE while
> verifying the gimple call.
>
> I have fixed this by adding an appropriate conversion after the GETTMCLONE
> call.
>
> I can make sure the conversion only happens for the OBJ_TYPE_REF, but I was
> lazy.  I don't think it'll hurt.
>
> OK for branch?

You should really merge from trunk ;)

Richard.
Aldy Hernandez - Feb. 22, 2011, 12:46 p.m.
> You should really merge from trunk ;)

Sooo true!  I dropped the ball on this such a long time ago, I'm now 
terrified of doing a merge this late in the release and break everyone's 
tree (the Velox folk's anyhow).

I will wait a few weeks until they get their final release in place, and 
then I'll do a merge (and keep up with subsequent merges :)).

Thanks; I really do need a kick in the pants for this.

Aldy

Patch

Index: testsuite/g++.dg/tm/pr47746.C
===================================================================
--- testsuite/g++.dg/tm/pr47746.C	(revision 0)
+++ testsuite/g++.dg/tm/pr47746.C	(revision 0)
@@ -0,0 +1,27 @@ 
+// { dg-do compile }
+// { dg-options "-fgnu-tm" }
+
+class InputStream
+{
+        public:
+        virtual unsigned int readUint32 () = 0;
+};
+
+class Building
+{
+        public:
+        __attribute__((transaction_safe)) Building (InputStream *stream);
+        __attribute__((transaction_safe)) void freeGradients ();
+        void load (InputStream *stream);
+};
+
+Building::Building (InputStream *stream)
+{
+        load(stream);
+}
+
+void Building::load (InputStream *stream)
+{
+        int j = (int)stream->readUint32 ();
+        freeGradients ();
+}
Index: trans-mem.c
===================================================================
--- trans-mem.c	(revision 170360)
+++ trans-mem.c	(working copy)
@@ -4274,6 +4274,27 @@  ipa_tm_insert_gettmclone_call (struct cg
   gimple_call_set_noinline_p (stmt);
 
   gimple_call_set_fn (stmt, callfn);
+
+  /* Discard OBJ_TYPE_REF above, may produce incompatible LHS and RHS
+     for a call statement.  Fix it.  */
+  {
+    tree lhs = gimple_call_lhs (stmt);
+    tree rettype =  TREE_TYPE (TREE_TYPE (TREE_TYPE (callfn)));
+    if (lhs
+	&& !useless_type_conversion_p (TREE_TYPE (lhs), rettype))
+    {
+      tree temp;
+
+      temp = make_rename_temp (rettype, 0);
+      gimple_call_set_lhs (stmt, temp);
+
+      g2 = gimple_build_assign (lhs,
+				fold_build1 (VIEW_CONVERT_EXPR,
+					     TREE_TYPE (lhs), temp));
+      gsi_insert_after (gsi, g2, GSI_SAME_STMT);
+    }
+  }
+
   update_stmt (stmt);
 
   return true;