[Bug target/61044] New: Computed goto on AVR fails to use word-addressing

[Bug target/61044] New: Computed goto on AVR fails to use word-addressing

[dinuxbg at gmail dot com]

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

            Bug ID: 61044
           Summary: Computed goto on AVR fails to use word-addressing
           Product: gcc
           Version: 4.8.2
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: dinuxbg at gmail dot com

Created attachment 32727
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=32727&action=edit
Failed computed goto C example

The computed goto example taken straight from the GCC manual generates
incorrect code for AVR. When doing pointer arithmetic, the compiler fails to
mark labels with gs.

The following snippet from the attached test_fail C function:
    static const int array[] = { &&foo - &&foo, &&bar - &&foo, &&hack - &&foo
};
will generate the following rodata:
array.1464:
        .word   0
        .word   .L3-(.L2)
        .word   .L4-(.L2)


Note that straight labels without arithmetics work just fine (see test_good in
attached cg.c):
array.1472:
        .word   gs(.L2)
        .word   gs(.L3)
        .word   gs(.L4)

I've tested with avr-gcc 4.8.2 from debian sid. But I believe bug is also
present in top of tree.

[Bug target/61044] Computed goto on AVR fails to use word-addressing

[dinuxbg at gmail dot com]

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

--- Comment #1 from Dimitar Dimitrov <dinuxbg at gmail dot com> ---
The unofficial PRU GCC port (https://github.com/dinuxbg/gnupru) has the same
bug. I haven't tested the following patch on AVR, but a similar change seems to
fix the issue for PRU:


--- a/gcc/config/avr/predicates.md
+++ b/gcc/config/avr/predicates.md
@@ -122,7 +122,7 @@
-  (match_code "code_label,label_ref,symbol_ref,plus,const")
+  (match_code "code_label,label_ref,symbol_ref,plus,minus,const")
@@ -133,6 +133,7 @@
     case SYMBOL_REF :
       return SYMBOL_REF_FUNCTION_P (op);
     case PLUS :
+    case MINUS :

[Bug target/61044] Computed goto on AVR fails to use word-addressing

[gjl at gcc dot gnu.org]

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

Georg-Johann Lay <gjl at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |gjl at gcc dot gnu.org,
                   |                            |senthil_kumar.selvaraj@atme
                   |                            |l.com

--- Comment #2 from Georg-Johann Lay <gjl at gcc dot gnu.org> ---
I don't see any difference in generated code with that patch.

Anyway, the question is /what/ the generated code should be?

The label differences would be something like

    .word pm(.L3-.L2)

because we must not generate stubs for the offset.  pm() works under the
assumption that gas does not resolve the difference and emits relocs for it,
and that pm() accepts differences.  Currently, gas will compute the difference
and come up with somethink like pm(4) which it resolves to 4 and not as 2.

Senthil, I searched for a user level documentation of -mlink-relax but with no
avail.  Is there a special reason for why there is no documentation for that
new gas command line option?

And why is this option needed in the first place? Why not always emitting
relocs for differences of labels? It does not buy us anything if gas computes
the difference ... except for annoyance like checking for -mlink-relax during
GCC configury.

What does not work is to add offsets to stub addresses, e.g. gs(.L2)+4 will
point to the next stub (if any) and *not* to .L2 + 4.  Thus, I don't think it's
worth to fix this PR and go through all the hassle with/without linker
relaxation and cater for all situations.

Instead, we should mention in the manual that the second example for computed
goto (with label differences) is not supported for AVR.  Not to mention that
alternative #1 will yield better code because there is not need to add offsets
at run time.

[Bug target/61044] Computed goto on AVR fails to use word-addressing

[senthil_kumar.selvaraj at atmel dot com]

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

--- Comment #3 from Senthil Kumar Selvaraj <senthil_kumar.selvaraj at atmel dot com> ---
Johann,

The primary reason I added the diff relocs was to prevent linker relaxation
messing up DWARF line number information - as you know, relaxation can shorten
instruction sequences, and the addresses in DWARF then go out of sync.

I guess I must add some user documentation about this, but ideally, this is
supposed to be transparent to the user - just passing -mrelax to the compiler
should work.

I turned diff reloc generation on only if -mlink-relax is passed because this
is what other ports (xtensa) do, and I wasn't sure of the consequences of
resolving every subtraction expression at link time.

I tried assembling .word pm(.L3-.L2) with -mlink-relax, but the assembler gave
up with an "expression too complex" error. Like you said, I guess documenting
that this is not supported is the way to go.

[Bug target/61044] Computed goto on AVR fails to use word-addressing

[gjl at gcc dot gnu.org]

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

--- Comment #4 from Georg-Johann Lay <gjl at gcc dot gnu.org> ---
Author: gjl
Date: Wed May 28 08:42:25 2014
New Revision: 210999

URL: http://gcc.gnu.org/viewcvs?rev=210999&root=gcc&view=rev
Log:
    PR target/61044
    * doc/extend.texi (Local Labels): Note that label differences are
    not supported for AVR.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/doc/extend.texi

[Bug target/61044] Computed goto on AVR fails to use word-addressing

[gjl at gcc dot gnu.org]

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

--- Comment #5 from Georg-Johann Lay <gjl at gcc dot gnu.org> ---
Author: gjl
Date: Wed May 28 08:44:23 2014
New Revision: 211000

URL: http://gcc.gnu.org/viewcvs?rev=211000&root=gcc&view=rev
Log:
    PR target/61044
    * doc/extend.texi (Local Labels): Note that label differences are
    not supported for AVR.


Modified:
    branches/gcc-4_9-branch/gcc/ChangeLog
    branches/gcc-4_9-branch/gcc/doc/extend.texi

[Bug target/61044] Computed goto on AVR fails to use word-addressing

[gjl at gcc dot gnu.org]

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

--- Comment #6 from Georg-Johann Lay <gjl at gcc dot gnu.org> ---
Author: gjl
Date: Wed May 28 08:48:03 2014
New Revision: 211001

URL: http://gcc.gnu.org/viewcvs?rev=211001&root=gcc&view=rev
Log:
    PR target/61044
    * doc/extend.texi (Local Labels): Note that label differences are
    not supported for AVR.


Modified:
    branches/gcc-4_8-branch/gcc/ChangeLog
    branches/gcc-4_8-branch/gcc/doc/extend.texi

[Bug target/61044] Computed goto on AVR fails to use word-addressing

[gjl at gcc dot gnu.org]

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

--- Comment #7 from Georg-Johann Lay <gjl at gcc dot gnu.org> ---
Author: gjl
Date: Wed May 28 08:50:18 2014
New Revision: 211002

URL: http://gcc.gnu.org/viewcvs?rev=211002&root=gcc&view=rev
Log:
    PR target/61044
    * doc/extend.texi (Local Labels): Note that label differences are
    not supported for AVR.


Modified:
    branches/gcc-4_7-branch/gcc/ChangeLog
    branches/gcc-4_7-branch/gcc/doc/extend.texi

[Bug target/61044] Computed goto on AVR fails to use word-addressing

[gjl at gcc dot gnu.org]

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

Georg-Johann Lay <gjl at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P4
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |INVALID
   Target Milestone|---                         |4.9.1

--- Comment #8 from Georg-Johann Lay <gjl at gcc dot gnu.org> ---
Closed as INVALID after adding a note to the manual that label differences are
not supported for AVR.

[Bug target/61044] Computed goto on AVR fails to use word-addressing

[gjl at gcc dot gnu.org]

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

--- Comment #9 from Georg-Johann Lay <gjl at gcc dot gnu.org> ---
(In reply to Senthil Kumar Selvaraj from comment #3)
> The primary reason I added the diff relocs was to prevent linker relaxation
> messing up DWARF line number information - as you know, relaxation can
> shorten instruction sequences, and the addresses in DWARF then go out of
> sync.
> 
> I guess I must add some user documentation about this, but ideally, this is
> supposed to be transparent to the user - just passing -mrelax to the
> compiler should work.
> 
> I turned diff reloc generation on only if -mlink-relax is passed because
> this is what other ports (xtensa) do, and I wasn't sure of the consequences
> of resolving every subtraction expression at link time.

Resolving label differences at assemble time serves a faster linking process,
but that argument does not apply to avr:  We don't have magabytes of code that
have to be fixed at load time by a dynamic linker.

And you don't know at assemble time how the linker is called.  One example is
debugging through code that comes from a library and has been linked against
the application.  It's not very common but possible and yet another plus
(besides simplicity with less options and less GCC/Binutils dependency) for
always emitting label differences as relocs.