Re: [PATCH] Fix miscompilation in output_constant_def

[Maxim Kuvyrkov <maxim at codesourcery dot com>]

Mark Mitchell wrote:

...

> > But you're right, copying should be done sometime later and that place
> > is, I believe, unshare rtl pass. 
>
> > Therefore, if we set this flag in output_constant_def, the RTX will be
> > copied, if still used, in unshare pass.  Still, this is basically the
> > same as copying the rtx in output_constant_def itself.
>
> I like this idea.  That should make copies if and only if actually
> necessary, which seems like a good thing.  If nobody objects within 72
> hours, please go ahead and check this in.  If you can put together a
> test case to go with the patch, that would be a good thing.

Checked in.

I didn't manage to prepare a target-independent testcase as constant 
handling varies depending on architecture, hence I've put the testcase 
into gcc.target/m68k.  The final patch is attached.

Thanks,

--
Maxim

2009-07-09  Maxim Kuvyrkov  <maxim@codesourcery.com>

	* varasm.c (build_constant_desc): Don't share RTL in pool entries.
	
2009-07-09  Maxim Kuvyrkov  <maxim@codesourcery.com>

	* gcc.target/m68k/20090709-1.c: New.

Index: varasm.c
===================================================================
--- varasm.c	(revision 148831)
+++ varasm.c	(working copy)
@@ -3208,6 +3208,10 @@ build_constant_desc (tree exp)
   set_mem_alias_set (rtl, 0);
   set_mem_alias_set (rtl, const_alias_set);
 
+  /* We cannot share RTX'es in pool entries.
+     Mark this piece of RTL as required for unsharing.  */
+  RTX_FLAG (rtl, used) = 1;
+
   /* Set flags or add text to the name to record information, such as
      that it is a local symbol.  If the name is changed, the macro
      ASM_OUTPUT_LABELREF will have to know how to strip this
Index: testsuite/gcc.dg/20090709-1.c
===================================================================
--- testsuite/gcc.dg/20090709-1.c	(revision 0)
+++ testsuite/gcc.dg/20090709-1.c	(revision 0)
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* There should be 3 occurrences of .LC0 in the code:
+   one for the definition of "0",
+   one for use in test1() and
+   one for use in test2().
+   FIXME: At the moment m68k GCC does not optimize test1() to nop
+   for some reason.  */
+/* { dg-final { scan-assembler-times ".LC0" 3 } } */
+
+void dummy(char *arg);
+
+void test1(void)
+{
+  char tmp[2] = "0";
+}
+
+void test2(void)
+{
+  dummy("0");
+}