[Bug target/51345] New: [avr] Devices with 8-bit SP need their own multilib(s)

[Bug target/51345] New: [avr] Devices with 8-bit SP need their own multilib(s)

[gjl at gcc dot gnu.org]

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

             Bug #: 51345
           Summary: [avr] Devices with 8-bit SP need their own multilib(s)
    Classification: Unclassified
           Product: gcc
           Version: 4.6.2
            Status: UNCONFIRMED
          Keywords: FIXME, wrong-code
          Severity: normal
          Priority: P3
         Component: target
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: gjl@gcc.gnu.org
                CC: eric.weddington@atmel.com
            Blocks: 51002
            Target: avr


Code that reads SP register must not read from the high byte of SP if the SP is
just 8 bits wide.

The C code will be correct once PR51002 will have been fixed, however, there is
code in libgcc that also reads from __SP_H__ like __prologue_saves__ and
__epilogue_restores__.

The issue is that the 8-bit property is set on the device level in
avr_current_device->short_sp and devices with different settings of short_sp
are mixed up the the same multilib.

[Bug target/51345] [avr] Devices with 8-bit SP need their own multilib(s)

[gjl at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2011-11-29
   Target Milestone|---                         |4.7.0
     Ever Confirmed|0                           |1

[Bug target/51345] [avr] Devices with 8-bit SP need their own multilib(s)

[gjl at gcc dot gnu.org]

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

--- Comment #1 from Georg-Johann Lay <gjl at gcc dot gnu.org> 2011-12-02 19:14:20 UTC ---
Author: gjl
Date: Fri Dec  2 19:14:15 2011
New Revision: 181936

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=181936
Log:
    PR target/51002
    PR target/51345
    * config/avr/libgcc.S (__prologue_saves__, __epilogue_restores__):
    Enclose parts using __SP_H__ in !defined (__AVR_HAVE_8BIT_SP__).
    Add FIXME comments.
    * config/avr/avr.md (movhi_sp_r_irq_off, movhi_sp_r_irq_on): Set
    insn condition to !AVR_HAVE_8BIT_SP.
    * config/avr/avr.c (output_movhi): "clr%B0" instead of "in
    %B0,__SP_H__" if AVR_HAVE_8BIT_SP.
    (avr_file_start): Only print "__SP_H__ = 0x3e" if !AVR_HAVE_8BIT_SP.
    * config/avr/avr-devices.c (avr_mcu_types): ATtiny4313 and
    AT86RF401 have a 16-bit SP (their manual is bogus).


Modified:
    branches/gcc-4_6-branch/gcc/ChangeLog
    branches/gcc-4_6-branch/gcc/config/avr/avr-devices.c
    branches/gcc-4_6-branch/gcc/config/avr/avr.c
    branches/gcc-4_6-branch/gcc/config/avr/avr.md
    branches/gcc-4_6-branch/gcc/config/avr/libgcc.S

[Bug target/51345] [avr] Devices with 8-bit SP need their own multilib(s)

[gjl at gcc dot gnu.org]

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

--- Comment #2 from Georg-Johann Lay <gjl at gcc dot gnu.org> 2011-12-06 15:04:17 UTC ---
Author: gjl
Date: Tue Dec  6 15:04:09 2011
New Revision: 182052

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=182052
Log:
libgcc/
    Forward-port from gcc-4_6-branch r181936 2011-12-02.

    PR target/51345
    PR target/51002
    * config/avr/lib1funcs.S (__prologue_saves__,
    __epilogue_restores__, __divdi3_moddi3): Enclose parts using
    __SP_H__ in !defined (__AVR_HAVE_8BIT_SP__).  Add FIXME comments.

gcc/
    Forward-port from gcc-4_6-branch r181936 2011-12-02.

    PR target/51002
    * config/avr/avr.md (movhi_sp_r): Set insn condition to
    !AVR_HAVE_8BIT_SP.
    * config/avr/avr.c (output_movhi): Use "clr%B0" instead of "in
    %B0,__SP_H__" if AVR_HAVE_8BIT_SP.
    (avr_file_start): Only print "__SP_H__ = 0x3e" if !AVR_HAVE_8BIT_SP.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/avr/avr.c
    trunk/gcc/config/avr/avr.md
    trunk/libgcc/ChangeLog
    trunk/libgcc/config/avr/lib1funcs.S

[Bug target/51345] [avr] Devices with 8-bit SP need their own multilib(s)

[gjl at gcc dot gnu.org]

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

--- Comment #3 from Georg-Johann Lay <gjl at gcc dot gnu.org> 2011-12-28 12:21:40 UTC ---
Created attachment 26194
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26194
tentative patch

[Bug target/51345] [avr] Devices with 8-bit SP need their own multilib(s)

[gjl at gcc dot gnu.org]

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

--- Comment #4 from Georg-Johann Lay <gjl at gcc dot gnu.org> 2012-01-02 12:52:00 UTC ---
Author: gjl
Date: Mon Jan  2 12:51:57 2012
New Revision: 182797

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=182797
Log:
contrib/
    PR target/51345
    * gcc_update (files_and_dependencies): Add
    gcc/config/avr/t-multilib, gcc/config/avr/multilib.h.

libgcc/
    PR target/51345
    * config/avr/lib1funcs.S: Remove FIXME comments.
    (SPEED_DIV): Depend on __AVR_HAVE_8BIT_SP__.
gcc/    
    PR target/51345
    * config.gcc (tm_file target=avr]): Add avr/avr-multilib.h
    (tmake_file target=avr): Add avr/t-multilib.

    * config/avr/avr-c.c (avr_cpu_cpp_builtins): Use AVR_HAVE_8BIT_SP
    to built-in define __AVR_HAVE_8BIT_SP__, __AVR_HAVE_16BIT_SP__.
    * config/avr/genmultilib.awk: New file.
    * config/avr/t-multilib: New auto-generated file.
    * config/avr/multilib.h: New auto-generated file.
    * config/avr/t-avr (AVR_MCUS): New variable.
    (genopt.sh): Use it.
    (s-mlib): Depend on t-multilib.
    (t-multilib, multilib.h): New dependencies.
    (s-avr-mlib): New rule to build t-multilib, multilib.h from AVR_MCUS.
    (MULTILIB_OPTIONS): Remove.
    (MULTILIB_MATCHES): Remove.
    (MULTILIB_DIRNAMES): Remove.
    (MULTILIB_EXCEPTIONS): Remove:
    * config/avr/genopt.sh: Don't use hard coded file name;
    pass AVR_MCUS from t-avr instead.


Added:
    trunk/gcc/config/avr/genmultilib.awk
    trunk/gcc/config/avr/multilib.h
    trunk/gcc/config/avr/t-multilib
Modified:
    trunk/contrib/ChangeLog
    trunk/contrib/gcc_update
    trunk/gcc/ChangeLog
    trunk/gcc/config.gcc
    trunk/gcc/config/avr/avr-c.c
    trunk/gcc/config/avr/avr-mcus.def
    trunk/gcc/config/avr/genopt.sh
    trunk/gcc/config/avr/t-avr
    trunk/libgcc/ChangeLog
    trunk/libgcc/config/avr/lib1funcs.S

[Bug target/51345] [avr] Devices with 8-bit SP need their own multilib(s)

[gjl at gcc dot gnu.org]

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

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

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

--- Comment #5 from Georg-Johann Lay <gjl at gcc dot gnu.org> 2012-01-04 15:29:29 UTC ---
Fixed in 4.7.0

[Bug target/51345] [avr] Devices with 8-bit SP need their own multilib(s)

[gjl at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|FIXED                       |
   Target Milestone|4.7.0                       |4.7.1

--- Comment #6 from Georg-Johann Lay <gjl at gcc dot gnu.org> 2012-03-24 21:06:42 UTC ---
Reopened:

The current (4.7.0) behaviour is:

-mtiny-stack acts as multilib-option for 16-bit SP targets in avr2 and avr25,
i.e. with that switch will trigger multi-lib from ./tiny-stack resp.
./avr25/tiny-stack

The problem is that the tiny-stack multilibs mix 16-bit SP targets that are
treated as 8-bit SP, and targets with 8-bit SP.

The currect implementation assumes SP.H = 0 in tiny-stack mlibs in order to
properly initialize the frame pointer, e.g. in __prologue_saves.

This is no good for 16-bit SP targets.

Solution:

Restore the behaviour for the 16-bit SP targets as was, i.e. -mtiny-stack does
not affect multilib selection for these targets, but if specified on the
command line, the resulting code will only change SP.L and assume SP.H never
changes.

For the 8-bit SP targets, -mtiny-stack should remain as in 4.7.0, i.e. used
internally to trigger tiny-stack multilibs. Specifying -mtiny-stack on the
command line will be redundant for these targets and have no effect.

[Bug target/51345] [avr] Devices with 8-bit SP need their own multilib(s)

[gjl at gcc dot gnu.org]

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

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

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

--- Comment #7 from Georg-Johann Lay <gjl at gcc dot gnu.org> 2012-03-28 10:29:40 UTC ---
Rescheduled for 4.7.1

[Bug target/51345] [avr] Devices with 8-bit SP need their own multilib(s)

[gjl at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.7.1                       |4.7.0

[Bug target/51345] [avr] Devices with 8-bit SP need their own multilib(s)

[gjl at gcc dot gnu.org]

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

--- Comment #8 from Georg-Johann Lay <gjl at gcc dot gnu.org> 2012-05-31 17:32:48 UTC ---
Author: gjl
Date: Thu May 31 17:32:42 2012
New Revision: 188070

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=188070
Log:
    PR target/51345
    * config/avr/avr.opt (-msp8): Document it.
    * doc/invoke.texi (AVR Options): Ditto.  And document related
    built-in macros.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/avr/avr.opt
    trunk/gcc/doc/invoke.texi