Invalid code generated for Coldfire target

[Jeff Law]

Meloun Michal wrote:
> Hello all,
> I tracing bug in GCC for Coldfire target, but I end in dead water and
> I need some help from real experts :).
> Both gcc 4.4 and 4.3 have same problem
>
> GCC miscompile this small test case.
>
> //-----------------------------------------------------------------
> //m68k-elf-gcc -save-temps -da -fdump-tree-all -fdump-ipa-all -c test.c -o test.o
> void dummy(char *arg);
>
> static void test1(void)
> {
>   char tmp[2] = "0";
> }
>
> void test2(void)
> {
>   dummy("0");
> }
> //------------------------------------------------------------------
> The file is compiled to:
> #NO_APP
> 	.file	"test.c"
> 	.section	.rodata
> .LC0:
> 	.string	"0"
> 	.text
> 	.align	2
> 	.type	test1, @function
> test1:
> 	link.w %fp,#-4
> 	lea .LC0,%a0
> 	move.w (%a0),-2(%fp)
> 	unlk %fp
> 	rts
> 	.size	test1, .-test1
>
> 	.align	2
> 	.globl	test2
> 	.type	test2, @function
> test2:
> 	link.w %fp,#0
> 	move.l %a0,-(%sp)           <-- note: a0 is used uninitialized here
> 	jsr dummy
> 	addq.l #4,%sp
> 	unlk %fp
> 	rts
> 	.size	test2, .-test2
>
> 	.ident	"GCC: (GNU) 4.4.0 20081031 (experimental)"
>
>
> And relevant part of RTL after expand pass:
>
> ;; Function test1 (test1)
> ;; Generating RTL for gimple basic block 2
> ;; tmp ={v} "0";
> (insn 5 4 0 test.c:8 (set (mem/s/c:HI (plus:SI (reg/f:SI 26 virtual-stack-vars)
>                 (const_int -2 [0xfffffffe])) [0 tmp+0 S2 A16])
>         (mem/s:HI (symbol_ref/f:SI ("*.LC0") [flags 0x2] <string_cst 0x7fdf7700>) [0 S2 A8])) -1 (nil))
>
> ;; Function test2 (test2)
> ;; Generating RTL for gimple basic block 2
> ;; dummy (&"0"[0]);
> <---  !!!! bad insn here -----
> (insn 5 4 6 test.c:15 (set (mem/f/i:SI (pre_dec:SI (reg/f:SI 15 %sp)) [0 S4 A16])
>         (reg:SI 8 %a0)) -1 (nil))
> <------------------------------
> (call_insn 6 5 7 test.c:15 (call (mem:QI (symbol_ref:SI ("dummy") [flags 0x41] <function_decl 0x7fdd4580 dummy>) [0 S1 A8])
>         (const_int 4 [0x4])) -1 (nil)
>     (nil))
>
> (insn 7 6 0 test.c:15 (set (reg/f:SI 15 %sp)
>         (plus:SI (reg/f:SI 15 %sp)
>             (const_int 4 [0x4]))) -1 (nil))
>
> After some debugging,  I  found  cause of this bug, but proper solution is
> not clear for me. When "char tmp[2] = "0";" is  compiled, the function "output_constant_def"
> is called and proper insn is stored into cache:
>
>  (gdb) call debug_rtx(desc->rtl)
>  (mem/s:HI (symbol_ref/f:SI ("*.LC0") [flags 0x2] <string_cst 0x7fdf7700>) [0 S2 A8])
>
>
> Later, in ira pass, this insn is spitted to this (Coldfire has no memory to memory move):
>
> Reloads for insn # 5
> Reload 0: reload_in (SI) = (symbol_ref/f:SI ("*.LC0") [flags 0x2] <string_cst
> 0x7fdf7700>)
>       ADDR_REGS, RELOAD_FOR_INPUT (opnum = 1), inc by 2
>       reload_in_reg: (symbol_ref/f:SI ("*.LC0") [flags 0x2] <string_cst 0x7fdf7700>)
>       reload_reg_rtx: (reg:SI 8 %a0)
> ...
> (note 2 3 14 2 NOTE_INSN_FUNCTION_BEG)
>
> (insn 14 2 5 2 test.c:7 (set (reg:SI 8 %a0)
>         (symbol_ref/f:SI ("*.LC0") [flags 0x2] <string_cst 0x7fdf7700>)) 38 {*movsi_cf} (nil))
>
> (insn 5 14 13 2 test.c:7 (set (mem/s/c:HI (plus:SI (reg/f:SI 14 %a6)
>                 (const_int -2 [0xfffffffe])) [0 tmp+0 S2 A16])
>         (mem/s:HI (reg:SI 8 %a0) [0 S2 A8])) 41 {*m68k.md:906} (nil))
> ;; End of basic block 2 -> ( 1)
> ;; lr  out 	 14 [%a6] 15 [%sp] 24 [%argptr]
>
> The first insn is newly allocated, but second one overwrites original insn.
> From local point of view, this is still OK, but  this corrupt the output_constant_def
> cache (cache holds pointer to overwritten insn. So every
> next access to "0" constat  returns
>  (mem/s:HI (reg:SI 8 %a0) [0 S2 A8])) 41 {*m68k.md:906} (nil))
> and not
>  (symbol_ref/f:SI ("*.LC0") [flags 0x2] <string_cst 0x7fdf7700>)) 38 {*movsi_cf} (nil))
>
> But how to fix this? By my mean, the cache must hold own immutable copy of insn.
> But im not gcc expert, so I need help with proper solution. Is this patch OK? Any other solution?
>
> --- varasm.c.orig       2008-11-14 18:04:27.693643900 +0100
> +++ varasm.c    2008-11-14 17:58:06.522748300 +0100
> @@ -3245,7 +3245,7 @@
>      }
>
>    maybe_output_constant_def_contents (desc, defer);
> -  return desc->rtl;
> +  return copy_rtx (desc->rtl);
>  }
>
>  /* Subroutine of output_constant_def: Decide whether or not we need to
> ---------------------------------------------
>
> Moreover, this can be common problem on more places (at least at  gen_rtx_CONST_INT).
>
>
> Ohh, and sorry for my english.
>
> Many thanks
>
>  Michal Meloun
>   
Can you forward all the debugging dumps?  Clearly there's a structure 
sharing problem here, but I'd like to see the full dumps.

Jeff