[Bug target/40419] __attribute__((mips16)) is broken on trunk.

[Bug target/40419] __attribute__((mips16)) is broken on trunk.

[meadori at codesourcery dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40419

Meador Inge <meadori at codesourcery dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |meadori at codesourcery dot
                   |                            |com

--- Comment #5 from Meador Inge <meadori at codesourcery dot com> 2012-01-09 22:14:11 UTC ---
I believe this was fixed by pr47564.

[Bug target/40419] __attribute__((mips16)) is broken on trunk.

[rsandifo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40419

rsandifo@gcc.gnu.org <rsandifo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |FIXED

--- Comment #6 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> 2012-01-18 09:34:48 UTC ---
Yeah, seems to have been working for a while.

[Bug target/40419] New: __attribute__((mips16)) is broken on trunk.

[ramana at gcc dot gnu dot org]

While implementing something like mips16 as a __attribute__((thumb)) for the
ARM port I discovered that the following testcase stripped down from a testcase
in the gcc testsuite causes a segmentation fault in the compiler for the MIPS
port as well.


static double __attribute__((mips16)) time_giop_encode (unsigned long);
int
main(int ac, char *av[])
{
  time_giop_encode (0);
}


static
double  __attribute__ ((mips16))
time_giop_encode  (unsigned long l)
{
  giop_encode_ulong (l);
}


A run through a valgrind showed that there was a write to an invalid address - 


==7998== Invalid write of size 4
==7998==    at 0x8234A0E: gen_reg_rtx (emit-rtl.c:912)
==7998==    by 0x830CD7B: expand_function_start (function.c:4416)
==7998==    by 0x88BBF81: gimple_expand_cfg (cfgexpand.c:2512)
==7998==    by 0x83D220C: execute_one_pass (passes.c:1289)
==7998==    by 0x83D248B: execute_pass_list (passes.c:1338)
==7998==    by 0x854C81F: tree_rest_of_compilation (tree-optimize.c:394)
==7998==    by 0x86C524B: cgraph_expand_function (cgraphunit.c:1097)
==7998==    by 0x86C7A4C: cgraph_optimize (cgraphunit.c:1156)
==7998==    by 0x80BE9BA: c_write_global_declarations (c-decl.c:8593)
==7998==    by 0x84F4255: toplev_main (toplev.c:1044)
==7998==    by 0x8144A21: main (main.c:35)
==7998==  Address 0x46811f0 is not stack'd, malloc'd or (recently) free'd
==7998== 
==7998== Invalid write of size 4
==7998==    at 0x8234A0E: gen_reg_rtx (emit-rtl.c:912)
==7998==    by 0x8304E0B: assign_parm_setup_reg (function.c:2780)
==7998==    by 0x830AB61: assign_parms (function.c:3172)
==7998==    by 0x830C938: expand_function_start (function.c:4432)
==7998==    by 0x88BBF81: gimple_expand_cfg (cfgexpand.c:2512)
==7998==    by 0x83D220C: execute_one_pass (passes.c:1289)
==7998==    by 0x83D248B: execute_pass_list (passes.c:1338)
==7998==    by 0x854C81F: tree_rest_of_compilation (tree-optimize.c:394)
==7998==    by 0x86C524B: cgraph_expand_function (cgraphunit.c:1097)
==7998==    by 0x86C7A4C: cgraph_optimize (cgraphunit.c:1156)
==7998==    by 0x80BE9BA: c_write_global_declarations (c-decl.c:8593)
==7998==    by 0x84F4255: toplev_main (toplev.c:1044)
==7998==  Address 0x46811f4 is not stack'd, malloc'd or (recently) free'd
==7998== 
==7998== Invalid write of size 1
==7998==    at 0x82351A2: mark_reg_pointer (emit-rtl.c:1120)
==7998==    by 0x830522D: assign_parm_setup_reg (function.c:2945)
==7998==    by 0x830AB61: assign_parms (function.c:3172)
==7998==    by 0x830C938: expand_function_start (function.c:4432)
==7998==    by 0x88BBF81: gimple_expand_cfg (cfgexpand.c:2512)
==7998==    by 0x83D220C: execute_one_pass (passes.c:1289)
==7998==    by 0x83D248B: execute_pass_list (passes.c:1338)
==7998==    by 0x854C81F: tree_rest_of_compilation (tree-optimize.c:394)
==7998==    by 0x86C524B: cgraph_expand_function (cgraphunit.c:1097)
==7998==    by 0x86C7A4C: cgraph_optimize (cgraphunit.c:1156)
==7998==    by 0x80BE9BA: c_write_global_declarations (c-decl.c:8593)
==7998==    by 0x84F4255: toplev_main (toplev.c:1044)
==7998==  Address 0x479423a is 1 bytes after a block of size 1 alloc'd
==7998==    at 0x4026FDE: malloc (vg_replace_malloc.c:207)
==7998==    by 0x89D8DC7: xrealloc (xmalloc.c:177)
==7998==    by 0x8234A5E: gen_reg_rtx (emit-rtl.c:900)
==7998==    by 0x830CD7B: expand_function_start (function.c:4416)
==7998==    by 0x88BBF81: gimple_expand_cfg (cfgexpand.c:2512)
==7998==    by 0x83D220C: execute_one_pass (passes.c:1289)
==7998==    by 0x83D248B: execute_pass_list (passes.c:1338)
==7998==    by 0x854C81F: tree_rest_of_compilation (tree-optimize.c:394)
==7998==    by 0x86C524B: cgraph_expand_function (cgraphunit.c:1097)
==7998==    by 0x86C7A4C: cgraph_optimize (cgraphunit.c:1156)

The segfault is because reg_rtx_no is 0 and rtx_reg_no are uninitialized. It
looks like the infrastructure of target_reinit is broken because these are not
reinitialized correctly after inlining does its bit with setting and resetting
this .  init_function_start from tree_rest_of_compilation initializes these but
the inlining bits call set_cfun and do a target_reinit which sets all these
values to zero. expand_function_start gets called later and by this time since
regno_reg_rtx is set to 0 you have gen_reg_rtx returning a hard register.


-- 
           Summary: __attribute__((mips16)) is broken on trunk.
           Product: gcc
           Version: 4.5.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: ramana at gcc dot gnu dot org
 GCC build triplet: i686-linux-gnu
  GCC host triplet: i686-linux-gnu
GCC target triplet: mips-elf


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40419

[Bug target/40419] __attribute__((mips16)) is broken on trunk.

[ramana at gcc dot gnu dot org]

------- Comment #1 from ramana at gcc dot gnu dot org  2009-06-11 21:29 -------
Adding Richard and Sandra to the CC list.


-- 

ramana at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |sandra at gcc dot gnu dot
                   |                            |org, rsandifo at gcc dot gnu
                   |                            |dot org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40419

[Bug target/40419] __attribute__((mips16)) is broken on trunk.

[zadeck at naturalbridge dot com]

------- Comment #2 from zadeck at naturalbridge dot com  2010-06-04 17:18 -------
I would just like to say that i think that target_reinit should be removed.  
It is nothing but trouble.   We tried to use it on our private port and it was
very slow and most of the time ended up crashing.   Furthermore, when we got
down to the end, we discovered that in fact it does not end up calling
everything that was needed to reinitialize ira.   

Our platform has a register architecture like a sparc but it has a variable
number of ins and outs.   The exact number of ins depends on the function
signature and the number of outs depends on the signatures of the callees.   
So we tried using target_reinit to reset the register allocator after we had
seen the caller to the function and all of the callees.   

in the end we added a special pass that does an reinit_regs () before ira.  
This sped up our compiler by about 4x.  (i.e target_reinit is not just buggy,
it is slow.)

Note that target_reinit does not do a reinit_regs and so it actually does not
really reinitialize the target, at least as far as ira is concerned.  


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40419

[Bug target/40419] __attribute__((mips16)) is broken on trunk.

[rsandifo at gcc dot gnu dot org]

------- Comment #3 from rsandifo at gcc dot gnu dot org  2010-06-05 07:27 -------
Kenny, I too don't like target_reinit, and FWIW
I fought the same battle before the patch was submitted.
The argument then, and I suspect the argument now,
is that although the function is horribly inefficient,
it still performs a useful function.  The best way of
making it efficient (the argument goes) is to gradually
wean GCC off using global variables for target-specific
stuff.  And as that happens, target_reinit would get
quicker for all targets.

As I remember it, even those supporting target_reinit
didn't really _like_ the thing, but simply accepted
it as the best we could do without major restructuring.

As far as the IRA thing goes: as we talked about elsewhere,
that's simply a bug.  target_reinit should call ira_init.
(Remember that target_reinit predates IRA, so this certainly
isn't the fault of anyone on the "target_reinit side".
It's just one of those things that happens, and needs
to be fixed when the problem shows up.)

What you're doing is probably fine for your port,
but other targets (like MIPS for the MIPS16 switch)
need to do more than modify registers.  Things like
rtx costs change too, and those costs are cached
in various global variables.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40419

[Bug target/40419] __attribute__((mips16)) is broken on trunk.

[zadeck at naturalbridge dot com]

------- Comment #4 from zadeck at naturalbridge dot com  2010-06-05 11:44 -------
richard, 

the reason that i went into such details about my port in (2) was to get the
reinit_regs issue out in a place so that if someone decided to take on this
beast, they had all of the issues in front of them.

kenny


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40419