[Bug target/65779] New: undefined local symbol on powerpc

[Bug target/65779] New: undefined local symbol on powerpc

[joel at gcc dot gnu.org]

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

            Bug ID: 65779
           Summary: undefined local symbol on powerpc
           Product: gcc
           Version: 5.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: joel at gcc dot gnu.org

Created attachment 35323
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35323&action=edit
Preprocessed source from zlib.c from RTEMS PowerPC bootloader

This file compiled without an undefined symbol with many gcc versions prior to
5.x.

powerpc-rtems4.11-gcc (GCC) 5.0.0 20150412 (experimental)

Fails:  powerpc-rtems4.11-gcc -mrelocatable -mcpu=603e -O2 -g zlib_pp.c 
Passes: powerpc-rtems4.11-gcc -mcpu=603e -O2 -g zlib_pp.c
Fails:  powerpc-rtems4.11-gcc -mrelocatable  -O2 -g zlib_pp.c

Also passes at -Os, -O0, and -O1.

The error is that the assembly language produced has a .4byte directive with a
reference to a symbol that is not generated.

$ grep LCL2 zlib_pp.s 
.LCL22:
    lwz 0,.LCL22-.LCF22(30)
.LCL23:
    lwz 0,.LCL23-.LCF23(30)
.LCL24:
    lwz 0,.LCL24-.LCF24(30)
.LCL25:
    lwz 0,.LCL25-.LCF25(30)
    .4byte    .LCL2
    .4byte    .LCL2

[Bug target/65779] [5 Regression] undefined local symbol on powerpc [regression]

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |5.0
            Summary|undefined local symbol on   |[5 Regression] undefined
                   |powerpc [regression]        |local symbol on powerpc
                   |                            |[regression]

[Bug target/65779] [5/6 Regression] undefined local symbol on powerpc [regression]

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2015-04-16
                 CC|                            |aoliva at gcc dot gnu.org
     Ever confirmed|0                           |1

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
There seems to be multiple issues here.

One is a bug in the TARGET_RELOCATABLE handling in the powerpc backend, it
emits the LCL and LCF symbols in the IL, but then emits them into the actual
assembly only if some conditions are satisfied.  If they make it through into
debug insns for some reason, that obviously results in the undefined
references.
Perhaps the target address legitimization should handle those, or if the
backend determines that it won't emit the symbols it should scan the IL for any
possible debug uses and reset them.  I'm afraid there is nothing dwarf2out can
do for this.

And another thing is why valtrack during DSE2 emits a debug instruction
containing this.
At *.split2 we have:
(insn 36 35 37 2 (set (reg/v:SI 0 0 [orig:239 s2 ] [239])
        (lshiftrt:SI (reg/v:SI 3 3 [orig:265 adler ] [265])
            (const_int 16 [0x10])))
../../../../../../../../../rtems/c/src/lib/libbsp/powerpc/motorola_powerpc/bootloader/../../../powerpc/shared/b
     (nil))
(debug_insn 37 36 39 2 (var_location:SI s2 (reg/v:SI 0 0 [orig:239 s2 ] [239]))
../../../../../../../../../rtems/c/src/lib/libbsp/powerpc/motorola_
     (nil))
which LGTM.
But then during pro_and_epilogue pass, the insn 36 is removed (perhaps some
kind of fast DCE?), and instead a code in the prologue that uses register 0 for
something completely different is added, which results in the wrong-debug
because the debug insns weren't reset.

As a workaround, -fno-var-tracking-assignments should cure this.

[Bug target/65779] [5/6 Regression] undefined local symbol on powerpc [regression]

[amodra at gmail dot com]

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

Alan Modra <amodra at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |amodra at gmail dot com

--- Comment #2 from Alan Modra <amodra at gmail dot com> ---
The underlying problem here is that this sequence setting up the GOT/TOC
pointer in adler32():

(insn 358 357 359 2 (parallel [                                                 
            (set (reg:SI 65 lr)                                                 
                (symbol_ref:SI ("*.LCF2")))                                     
            (use (unspec [                                                      
                        (symbol_ref:SI ("*.LCF2"))                              
                    ] UNSPEC_TOC))                                              
        ])
../../../../../../../../../rtems/c/src/lib/libbsp/powerpc/motorola_p\
owerpc/bootloader/../../../powerpc/shared/bootloader/zlib.c:2081 -1             
     (nil))                                                                     
(insn 359 358 360 2 (set (reg:SI 30 30)                                         
        (reg:SI 65 lr))
../../../../../../../../../rtems/c/src/lib/libbsp/power\
pc/motorola_powerpc/bootloader/../../../powerpc/shared/bootloader/zlib.c:2081
-\
1                                                                               
     (expr_list:REG_DEAD (reg:SI 65 lr)                                         
        (nil)))                                                                 
(insn 360 359 376 2 (set (reg:SI 0 0)                                           
        (mem:SI (plus:SI (reg:SI 30 30)                                         
                (minus:SI (symbol_ref:SI ("*.LCL2"))                            
                    (symbol_ref:SI ("*.LCF2")))) [0  S4 A8]))
../../../../../..\
/../../../rtems/c/src/lib/libbsp/powerpc/motorola_powerpc/bootloader/../../../p\
owerpc/shared/bootloader/zlib.c:2081 -1                                         
     (nil))                                                                     
(debug_insn 376 360 361 2 (var_location:SI D#37 (reg/v:SI 0 0 [orig:239 s2 ]
[2\
39])) -1                                                                        
     (nil))                                                                     
(insn 361 376 362 2 (set (reg:SI 30 30)                                         
        (plus:SI (reg:SI 0 0)                                                   
            (reg:SI 30 30)))
../../../../../../../../../rtems/c/src/lib/libbsp/\
powerpc/motorola_powerpc/bootloader/../../../powerpc/shared/bootloader/zlib.c:2\
081 -1                                                                          
     (expr_list:REG_DEAD (reg:SI 0 0)                                           
        (expr_list:REG_UNUSED (reg:SI 30 30)                                    
            (nil))))

is deleted by peephole2

DCE: Deleting insn 361                                                          
deleting insn with uid = 361.                                                   
DCE: Deleting insn 360                                                          
deleting insn with uid = 360.                                                   
DCE: Deleting insn 359                                                          
deleting insn with uid = 359.                                                   
DCE: Deleting insn 358                                                          
deleting insn with uid = 358.

resulting in uses_TOC() returning false due to not finding UNSPEC_TOC, and thus
not emitting the labels.  Deleting the insns is correct; we don't need the GOI
pointer set up in adler32().  gcc-4.9 also does the same thing, but doesn't
leak the labels into debug insns.

[Bug target/65779] [5/6 Regression] undefined local symbol on powerpc [regression]

[amodra at gmail dot com]

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

--- Comment #3 from Alan Modra <amodra at gmail dot com> ---
Re comment #1, insn 36 is not removed, but moved later to bb3

(insn 363 364 41 3 (set (reg/v:SI 0 0 [orig:239 s2 ] [239])                     
        (lshiftrt:SI (reg/v:SI 3 3 [orig:265 adler ] [265])                     
            (const_int 16 [0x10]))) 270 {lshrsi3}                               
     (nil))                                                                     
(debug_insn 41 363 42 3 (var_location:SI s2 (reg/v:SI 0 0 [orig:239 s2 ]
[239])\
) -1                                                                            
     (nil))

The trouble is that the original debug insn is not removed.  We still have
(debug_insn 37 35 39 2 (var_location:SI s2 (reg/v:SI 0 0 [orig:239 s2 ]
[239]))\

../../../../../../../../../rtems/c/src/lib/libbsp/powerpc/motorola_powerpc/boo\
tloader/../../../powerpc/shared/bootloader/zlib.c:2083 -1                       
     (nil))                                                                     
but this is not valid.  s2 does *not* live in reg:SI 0 at this point.

[Bug debug/65779] [5/6 Regression] undefined local symbol on powerpc [regression]

[amodra at gmail dot com]

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

--- Comment #4 from Alan Modra <amodra at gmail dot com> ---
Created attachment 35342
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35342&action=edit
prototype patch

This seems to cure the testcase.  I haven't bootstrapped it, and it probably
isn't ideal since I haven't poked at debug insns a lot..
Still, it points out where the problem is, in move_insn_for_shrink_wrap.

--- Comment #5 from Alan Modra <amodra at gmail dot com> ---
Hmm, the bootstrap didn't get very far.  Seems I need a
df_insn_rescan_debug_internal (dinsn);
after setting INSN_VAR_LOCATION_LOC.  Maybe the debug insn in this case can be
deleted?

[Bug debug/65779] [5/6 Regression] undefined local symbol on powerpc [regression]

[jakub at gcc dot gnu.org]

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Alan Modra from comment #4)
> Created attachment 35342 [details]
> prototype patch
> 
> This seems to cure the testcase.  I haven't bootstrapped it, and it probably
> isn't ideal since I haven't poked at debug insns a lot..
> Still, it points out where the problem is, in move_insn_for_shrink_wrap.

Can't you use DF infrastructure for that (before actually moving the insn)?
I think pro_and_epilogue pass doesn't compute DF_DU_CHAINs, but perhaps
something along the lines of DCE reset_unmarked_insns_debug_uses?
I.e. not perform it at all if !MAY_HAVE_DEBUG_INSNS, and use DF infrastructure
to find out if the debug insn references the register.  Of course, unlike DCE,
we aren't looking at debug instructions referencing unmarked insns, but debug
instructions referencing a particular hard register.  Note that
live_edge_for_reg is called multiple times and thus could move across more than
one basic block, so perhaps you'd need to remember the sequence of basic blocks
to scan during the
  /* At this point we are committed to moving INSN, but let's try to
     move it as far as we can.  */
loop.

[Bug debug/65779] [5/6 Regression] undefined local symbol on powerpc [regression]

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|5.0                         |5.2

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 5.1 has been released.

[Bug debug/65779] [5/6 Regression] undefined local symbol on powerpc [regression]

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2

[Bug debug/65779] [5/6 Regression] undefined local symbol on powerpc [regression]

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|5.2                         |5.3

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 5.2 is being released, adjusting target milestone to 5.3.

[Bug debug/65779] [5/6 Regression] undefined local symbol on powerpc [regression]

[pangbw at gmail dot com]

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

baoshan <pangbw at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |pangbw at gmail dot com

--- Comment #9 from baoshan <pangbw at gmail dot com> ---
(In reply to Jakub Jelinek from comment #6)
>> I.e. not perform it at all if !MAY_HAVE_DEBUG_INSNS, and use D

There are some regression test cases would check if the generated RTL are same
with or without debug option enabled(like gcc.dg/pr45055.c), so if we perform
optimization depend on if there is debug instructions, these test cases would
fail. Will GCC community accept such patch with these regressions? I have a fix
that could fix this issue but with regression on pr45055.c, so far I am
hesitate if I can post it for review.

[Bug debug/65779] [5/6 Regression] undefined local symbol on powerpc [regression]

[pangbw at gmail dot com]

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

--- Comment #12 from baoshan <pangbw at gmail dot com> ---
(In reply to Andrew Pinski from comment #11)
> I suspect this was fixed for GCC 6 with the patch that fixed bug 67789
> (which is the more correct patch).

I think they are two different issues. 67789 is duplicate label defined, this
is undefined label.

[Bug debug/65779] [5/6 Regression] undefined local symbol on powerpc [regression]

[segher at gcc dot gnu.org]

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

Segher Boessenkool <segher at gcc dot gnu.org> changed:

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

--- Comment #13 from Segher Boessenkool <segher at gcc dot gnu.org> ---
Yes, not the same thing as PR67789.