[Bug middle-end/55279] New: New pseudo registers aren't supported in CSE

[Bug middle-end/55279] New: New pseudo registers aren't supported in CSE

[hjl.tools at gmail dot com]

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

             Bug #: 55279
           Summary: New pseudo registers aren't supported in CSE
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: hjl.tools@gmail.com


Since CSE allocates register table in init_cse_reg_info,
if a pseudo register is created to convert memory address,
we get

==23925== 
==23925== Debugger has detached.  Valgrind regains control.  We continue.
==23925== Invalid write of size 4
==23925==    at 0xF813E1: get_cse_reg_info_1(unsigned int) (cse.c:826)
==23925==    by 0xF81473: get_cse_reg_info(unsigned int) (cse.c:841)
==23925==    by 0xF84792: hash_rtx_cb(rtx_def const*, machine_mode, int*, int*,
bool, int (*)(rtx_def const*, machine_mode, rtx_def**, machine_mode*))
(cse.c:2312)
==23925==    by 0xF85097: hash_rtx(rtx_def const*, machine_mode, int*, int*,
bool) (cse.c:2560)
==23925==    by 0xF850C7: canon_hash(rtx_def*, machine_mode) (cse.c:2571)
==23925==    by 0xF899F7: cse_insn(rtx_def*) (cse.c:4703)
==23925==    by 0xF8EA8A: cse_extended_basic_block(cse_basic_block_data*)
(cse.c:6408)
==23925==    by 0xF8F024: cse_main(rtx_def*, int) (cse.c:6585)
==23925==    by 0xF90D40: rest_of_handle_cse() (cse.c:7435)
==23925==    by 0x936857: execute_one_pass(opt_pass*) (passes.c:2337)
==23925==    by 0x936ACE: execute_pass_list(opt_pass*) (passes.c:2398)
==23925==    by 0x936AFF: execute_pass_list(opt_pass*) (passes.c:2399)
==23925==  Address 0xb7611fc is 12 bytes after a block of size 2,560 alloc'd
==23925==    at 0x4A0881C: malloc (vg_replace_malloc.c:270)
==23925==    by 0x10E4F74: xmalloc (xmalloc.c:147)
==23925==    by 0xF812FF: init_cse_reg_info(unsigned int) (cse.c:793)
==23925==    by 0xF8EE57: cse_main(rtx_def*, int) (cse.c:6528)
==23925==    by 0xF90D40: rest_of_handle_cse() (cse.c:7435)
==23925==    by 0x936857: execute_one_pass(opt_pass*) (passes.c:2337)
==23925==    by 0x936ACE: execute_pass_list(opt_pass*) (passes.c:2398)
==23925==    by 0x936AFF: execute_pass_list(opt_pass*) (passes.c:2399)
==23925==    by 0x666D1D: expand_function(cgraph_node*) (cgraphunit.c:1643)
==23925==    by 0x6671D8: expand_all_functions() (cgraphunit.c:1747)
==23925==    by 0x667C62: compile() (cgraphunit.c:2045)
==23925==    by 0x667DE1: finalize_compilation_unit() (cgraphunit.c:2122)
==23925== 

get_cse_reg_info_1 doesn't check if regno < cse_reg_info_table_size.

[Bug middle-end/55279] New pseudo registers aren't supported in CSE

[pinskia at gcc dot gnu.org]

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> 2012-11-12 03:49:22 UTC ---
Can you explain how you hit this with what patch and what testcase on what
target?

[Bug middle-end/55279] New pseudo registers aren't supported in CSE

[hjl.tools at gmail dot com]

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

--- Comment #2 from H.J. Lu <hjl.tools at gmail dot com> 2012-11-12 07:25:27 UTC ---
If we revert the explow.c change:

http://gcc.gnu.org/viewcvs?view=revision&revision=177592

gcc.dg/Warray-bounds.c will cause valgrind error with

-fno-diagnostics-show-caret -O2 -Warray-bounds -S -mx32 -maddress-mode=long

[Bug middle-end/55279] New pseudo registers aren't supported in CSE

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |ice-on-valid-code

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> 2012-12-09 01:23:00 UTC ---
I think combine was changed for the similar reason to support psedudos but I
cannot find the patch right now.

[Bug middle-end/55279] New pseudo registers aren't supported in CSE

[steven at gcc dot gnu.org]

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

Steven Bosscher <steven at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2012-12-09
                 CC|                            |jakub at gcc dot gnu.org
     Ever Confirmed|0                           |1

--- Comment #4 from Steven Bosscher <steven at gcc dot gnu.org> 2012-12-09 10:58:34 UTC ---
Potentially silent wrong code -> RM?

[Bug middle-end/55279] New pseudo registers aren't supported in CSE

[hjl.tools at gmail dot com]

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

--- Comment #5 from H.J. Lu <hjl.tools at gmail dot com> 2012-12-09 15:33:53 UTC ---
Any passes which allocate a table for maximum number of registers
can't deal with new pseudo registers.  But there is nothing to
check and enforce it.

[Bug middle-end/55279] New pseudo registers aren't supported in CSE

[steven at gcc dot gnu.org]

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

--- Comment #6 from Steven Bosscher <steven at gcc dot gnu.org> 2012-12-09 16:09:04 UTC ---
(In reply to comment #5)

There shouldn't be any such tables. Before the DF merge, there was 
no_new_pseudos, but it had its problems.

Why can't cse_reg_info_table() be modified to intercept this?

[Bug middle-end/55279] New pseudo registers aren't supported in CSE

[steven at gcc dot gnu.org]

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

Steven Bosscher <steven at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |steven at gcc dot gnu.org

--- Comment #7 from Steven Bosscher <steven at gcc dot gnu.org> 2012-12-09 16:09:44 UTC ---
(In reply to comment #5)
> Why can't cse_reg_info_table() be modified to intercept this?
Correction: get_cse_reg_info()

[Bug middle-end/55279] New pseudo registers aren't supported in CSE

[steven at gcc dot gnu.org]

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

--- Comment #8 from Steven Bosscher <steven at gcc dot gnu.org> 2012-12-09 16:13:12 UTC ---
(In reply to comment #7)
> (In reply to comment #5)
> > Why can't cse_reg_info_table() be modified to intercept this?
> Correction: get_cse_reg_info()

Like so, quick-and-dirty hack. HJ, can you try and see if this
keeps valgrind happy?

Index: cse.c
===================================================================
--- cse.c       (revision 194325)
+++ cse.c       (working copy)
@@ -833,6 +833,7 @@
 static inline struct cse_reg_info *
 get_cse_reg_info (unsigned int regno)
 {
+  init_cse_reg_info (regno + 1);
   struct cse_reg_info *p = &cse_reg_info_table[regno];

   /* If this entry has not been initialized, go ahead and initialize

[Bug middle-end/55279] New pseudo registers aren't supported in CSE

[hjl.tools at gmail dot com]

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

--- Comment #9 from H.J. Lu <hjl.tools at gmail dot com> 2012-12-10 01:05:09 UTC ---
(In reply to comment #8)
> (In reply to comment #7)
> > (In reply to comment #5)
> > > Why can't cse_reg_info_table() be modified to intercept this?
> > Correction: get_cse_reg_info()
> 
> Like so, quick-and-dirty hack. HJ, can you try and see if this
> keeps valgrind happy?
> 
> Index: cse.c
> ===================================================================
> --- cse.c       (revision 194325)
> +++ cse.c       (working copy)
> @@ -833,6 +833,7 @@
>  static inline struct cse_reg_info *
>  get_cse_reg_info (unsigned int regno)
>  {
> +  init_cse_reg_info (regno + 1);
>    struct cse_reg_info *p = &cse_reg_info_table[regno];
> 
>    /* If this entry has not been initialized, go ahead and initialize

I got

/export/gnu/import/git/sources/gcc-release/gcc/testsuite/gcc.dg/Warray-bounds.c:92:1:
internal compiler error: in insert_regs, at cse.c:1159^M
0xce1db0 insert_regs^M
        /export/gnu/import/git/sources/gcc-release/gcc/cse.c:1159^M 
0xce4fbb cse_insn^M
        /export/gnu/import/git/sources/gcc-release/gcc/cse.c:5793^M
0xce719b cse_extended_basic_block^M
        /export/gnu/import/git/sources/gcc-release/gcc/cse.c:6408^M
0xce719b cse_main^M
        /export/gnu/import/git/sources/gcc-release/gcc/cse.c:6586^M
0xce7675 rest_of_handle_cse^M
        /export/gnu/import/git/sources/gcc-release/gcc/cse.c:7436^M
Please submit a full bug report,^M
with preprocessed source if appropriate.^M
Please include the complete backtrace with any bug report.^M
See <http://gcc.gnu.org/bugs.html> for instructions.^M

[Bug middle-end/55279] New pseudo registers aren't supported in CSE

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55279

--- Comment #10 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #3)
> I think combine was changed for the similar reason to support psedudos but I
> cannot find the patch right now.

Note combine was only fully fixed recently in GCC 12 with
r12-8030-g61bee6aed26eb3.