diff mbox

[AVR] Was/Fix: error linking lto1 for target avr

Message ID 4ED51080.9090306@gjlay.de
State New
Headers show

Commit Message

Georg-Johann Lay Nov. 29, 2011, 5:04 p.m. UTC
Ian Lance Taylor wrote:
> Georg-Johann Lay <avr@gjlay.de> writes:
> 
>> So if a frontend can define address spaces and it is a generic feature, the
>> question is how to get the name of an address space in a generic, language
>> independent way.
> 
> We could decide that all frontends that use address spaces must define a
> printable name for each address space.  That would mean changing the
> middle-end address space interface to give a name to each address space.
> The current middle-end address space interface does not require that
> address spaces have a name.  I was not involved in the addition of
> address spaces to gcc, and I don't know why they followed the path they
> did.
> 
> Ian

Presumably they chose that approach to keep it simple or it is even a
performance issue to move the name around.

I attached a patch but I fail to find the right configure options for
gcc/binutils as the testsuite complains

./avr/bin/ld: bad -plugin option

Configured gcc with --enable-lto and binutils 2.21 with --enable-plugin.

Maybe the patch can be pre-approved so that the others can proceed with their work?

Johann

	* config/avr/avr.h (ADDR_SPACE_PGM, ADDR_SPACE_PGM1,
	ADDR_SPACE_PGM2, ADDR_SPACE_PGM3, ADDR_SPACE_PGM4,
	ADDR_SPACE_PGM5, ADDR_SPACE_PGMX): Write as enum.
	(avr_addrspace_t): New typedef.
	(avr_addrspace[]): New declaration.
	* config/avr/avr-c.c (avr_toupper): New static function.
	(avr_register_target_pragmas, avr_cpu_cpp_builtins): Use
	avr_addrspace to get address space information.
	* config/avr/avr.c (avr_addrspace): New variable.
	(avr_out_lpm, avr_pgm_check_var_decl, avr_insert_attributes,
	avr_asm_named_section, avr_section_type_flags,
	avr_asm_select_section, avr_addr_space_address_mode,
	avr_addr_space_convert, avr_emit_movmemhi): Use it.
	(avr_addr_space_pointer_mode): Forward to avr_addr_space_address_mode.
	(avr_pgm_segment): Remove.

Comments

Denis Chertykov Nov. 30, 2011, 7:41 a.m. UTC | #1
2011/11/29 Georg-Johann Lay <avr@gjlay.de>:
> Ian Lance Taylor wrote:
>> Georg-Johann Lay <avr@gjlay.de> writes:
>>
>>> So if a frontend can define address spaces and it is a generic feature, the
>>> question is how to get the name of an address space in a generic, language
>>> independent way.
>>
>> We could decide that all frontends that use address spaces must define a
>> printable name for each address space.  That would mean changing the
>> middle-end address space interface to give a name to each address space.
>> The current middle-end address space interface does not require that
>> address spaces have a name.  I was not involved in the addition of
>> address spaces to gcc, and I don't know why they followed the path they
>> did.
>>
>> Ian
>
> Presumably they chose that approach to keep it simple or it is even a
> performance issue to move the name around.
>
> I attached a patch but I fail to find the right configure options for
> gcc/binutils as the testsuite complains
>
> ./avr/bin/ld: bad -plugin option
>
> Configured gcc with --enable-lto and binutils 2.21 with --enable-plugin.
>
> Maybe the patch can be pre-approved so that the others can proceed with their work?

Better to complete this work.

Denis.
Georg-Johann Lay Nov. 30, 2011, 8:46 a.m. UTC | #2
Denis Chertykov wrote:
> 2011/11/29 Georg-Johann Lay:
> 
>>
>>I attached a patch but I fail to find the right configure options for
>>gcc/binutils as the testsuite complains
>>
>>./avr/bin/ld: bad -plugin option
>>
>>Configured gcc with --enable-lto and binutils 2.21 with --enable-plugin.
>>
>>Maybe the patch can be pre-approved so that the others can proceed with their work?
> 
> Better to complete this work.

The patch itself is complete from my side.

It's not passing the test suite because of some mis-configuration of gcc 
and/or binutils, not because the patch is incomplete.

I just can't find the right configure options to get the LTO stuff work, 
i.e. how to enable plugins in binutils and LTO in gcc.

Johann

> 
> Denis.
>
Ian Lance Taylor Nov. 30, 2011, 4:45 p.m. UTC | #3
Georg-Johann Lay <avr@gjlay.de> writes:

> I just can't find the right configure options to get the LTO stuff
> work, i.e. how to enable plugins in binutils and LTO in gcc.

For binutils: configure with --enable-plugins (note: not
--enable-plugin).

For gcc: should be on by default, or configure with --enable-lto.

Ian
Steven Bosscher Nov. 30, 2011, 10:02 p.m. UTC | #4
On Tue, Nov 29, 2011 at 6:04 PM, Georg-Johann Lay <avr@gjlay.de> wrote:
> I attached a patch but I fail to find the right configure options for
> gcc/binutils as the testsuite complains
>
> ./avr/bin/ld: bad -plugin option
>
> Configured gcc with --enable-lto and binutils 2.21 with --enable-plugin.

Huh, wait. I don't follow binutils much, but isn't the plugin only for
the gold linker?

Ciao!
Steven
Ian Lance Taylor Nov. 30, 2011, 11 p.m. UTC | #5
Steven Bosscher <stevenb.gcc@gmail.com> writes:

> On Tue, Nov 29, 2011 at 6:04 PM, Georg-Johann Lay <avr@gjlay.de> wrote:
>> I attached a patch but I fail to find the right configure options for
>> gcc/binutils as the testsuite complains
>>
>> ./avr/bin/ld: bad -plugin option
>>
>> Configured gcc with --enable-lto and binutils 2.21 with --enable-plugin.
>
> Huh, wait. I don't follow binutils much, but isn't the plugin only for
> the gold linker?

Actually both gold and the GNU linker support plugins these days.

(But the configure option is --enable-plugins.)

Ian
Georg-Johann Lay Dec. 6, 2011, 1:25 p.m. UTC | #6
Denis Chertykov wrote:
> 2011/11/29 Georg-Johann:
>> Ian Lance Taylor wrote:
>>> Georg-Johann Lay:
>>>
>>>> So if a frontend can define address spaces and it is a generic feature, the
>>>> question is how to get the name of an address space in a generic, language
>>>> independent way.
>>> We could decide that all frontends that use address spaces must define a
>>> printable name for each address space.  That would mean changing the
>>> middle-end address space interface to give a name to each address space.
>>> The current middle-end address space interface does not require that
>>> address spaces have a name.  I was not involved in the addition of
>>> address spaces to gcc, and I don't know why they followed the path they
>>> did.
>>>
>>> Ian
>> Presumably they chose that approach to keep it simple or it is even a
>> performance issue to move the name around.
>>
>> I attached a patch but I fail to find the right configure options for
>> gcc/binutils as the testsuite complains
>>
>> ./avr/bin/ld: bad -plugin option
>>
>> Configured gcc with --enable-lto and binutils 2.21 with --enable-plugin.
>>
>> Maybe the patch can be pre-approved so that the others can proceed with their work?
> 
> Better to complete this work.
> 
> Denis.

Finally, I could get LTO to work. Problem was in bad libdl.

The patch is unchanged:

http://gcc.gnu.org/ml/gcc-patches/2011-11/msg02574.html

Diffs in testresults are:

> FAIL: gcc.c-torture/execute/builtins/memcpy-chk.c compilation,  -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects
> FAIL: gcc.c-torture/execute/builtins/memmove-chk.c compilation,  -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects
> FAIL: gcc.c-torture/execute/builtins/mempcpy-chk.c compilation,  -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects
> FAIL: gcc.c-torture/execute/builtins/memset-chk.c compilation,  -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects

Problem is non-aligned LTO stuff like

/home/georg/install/avr/bin/ld: Warning: alignment 1 of symbol `buf5' in
/tmp/ccAMUQqq.ltrans1.ltrans.o is smaller than 16 in /tmp/ccehY2eq.o.ironly
/home/georg/install/avr/bin/ld: Warning: alignment 1 of symbol `buf7' in
/tmp/ccAMUQqq.ltrans1.ltrans.o is smaller than 16 in /tmp/ccehY2eq.o.ironly
/home/georg/install/avr/bin/ld: Warning: alignment 1 of symbol `buf1' in
/tmp/ccAMUQqq.ltrans1.ltrans.o is smaller than 16 in /tmp/ccehY2eq.o.ironly
/home/georg/install/avr/bin/ld: Warning: alignment 1 of symbol `chk_calls' in
/tmp/ccAMUQqq.ltrans1.ltrans.o is smaller than 2 in /tmp/cc9w5uuC.o.ironly
/home/georg/install/avr/bin/ld: Warning: alignment 1 of symbol
`memcpy_disallowed' in /tmp/ccAMUQqq.ltrans1.ltrans.o is smaller than 2 in
/tmp/cc9w5uuC.o.ironly

> FAIL: gcc.c-torture/execute/980709-1.c compilation,  -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects

Bug in avr-libc using RJMP where not appropriate leading to

/home/georg/install/gcc-4.7/lib/gcc/avr/4.7.0/../../../../avr/lib/avr51/libc.a(log.o):../../../../../source/avr-libc-1.7.1/libm/fplib/log.S:96:
relocation truncated to fit: R_AVR_13_PCREL against symbol `__addsf3' defined
in .text section in
/home/georg/build/gcc-trunk-avr/gcc/avr51/libgcc.a(_addsub_sf.o)

> FAIL: gcc.c-torture/execute/simd-1.c compilation,  -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects  (internal compiler error)

reload fail because data (vector) too big: internal compiler error: in
find_valid_class, at reload.c:707

> FAIL: gcc.dg/visibility-d.c scan-not-hidden

Problems with visibility pragma.

> FAIL: gcc.dg/lto/20090218-2 c_lto_20090218-2_1.o assemble, -O0 -flto
-flto-partition=none -fuse-linker-plugin
> FAIL: gcc.dg/lto/20090218-2 c_lto_20090218-2_1.o assemble, -O2 -flto
-flto-partition=none -fuse-linker-plugin -fno-fat-lto-objects
> FAIL: gcc.dg/lto/20090218-2 c_lto_20090218-2_1.o assemble, -O0 -flto
-flto-partition=1to1 -fno-use-linker-plugin
> FAIL: gcc.dg/lto/20090218-2 c_lto_20090218-2_1.o assemble, -O2 -flto
-flto-partition=1to1 -fno-use-linker-plugin
> FAIL: gcc.dg/lto/20090218-2 c_lto_20090218-2_1.o assemble, -O0 -flto
-fuse-linker-plugin -fno-fat-lto-objects
> FAIL: gcc.dg/lto/20090218-2 c_lto_20090218-2_1.o assemble, -O2 -flto
-fuse-linker-plugin

Bad test case (use long instead of site_t)

> FAIL: gcc.dg/lto/20091013-1 c_lto_20091013-1_1.o assemble, -fPIC -r -nostdlib
-flto
> FAIL: gcc.dg/lto/20091013-1 c_lto_20091013-1_2.o assemble, -fPIC -r -nostdlib
-flto
> FAIL: gcc.dg/lto/20091013-1 c_lto_20091013-1_1.o assemble, -fPIC -r -nostdlib
-O2 -flto
> FAIL: gcc.dg/lto/20091013-1 c_lto_20091013-1_2.o assemble, -fPIC -r -nostdlib
-O2 -flto

Bad test case: Assumes sizeof(long) == sizeof (void*)

> FAIL: gcc.dg/lto/ipareference2
c_lto_ipareference2_0.o-c_lto_ipareference2_1.o link,  -O1 -flto
-flto-partition=1to1 -fwhole-program

Warning: alignment 1 of symbol `e' in /tmp/ccXmAeaF.ltrans1.ltrans.o is smaller
than 2 in c_lto_ipareference2_0.o.ironly

> FAIL: gcc.dg/lto/trans-mem-1 c_lto_trans-mem-1_0.o-c_lto_trans-mem-1_1.o
link, -flto -fgnu-tm

Bad test case: Assumes transactional memory is available.

> FAIL: gcc.dg/torture/fp-int-convert-double.c  -O2 -flto -fuse-linker-plugin
-fno-fat-lto-objects  execution test
> FAIL: gcc.dg/torture/fp-int-convert-float.c  -O2 -flto -fuse-linker-plugin
-fno-fat-lto-objects  execution test
> FAIL: gcc.dg/torture/fp-int-convert-long-double.c  -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects  execution test

Bad test cases: Assume TImode is available.

> FAIL: gcc.dg/torture/pta-ptrarith-3.c  -O2 -flto -fno-use-linker-plugin
-flto-partition=none  execution test

Known bug: PR50063/PR49330

> FAIL: gcc.dg/torture/vec-cvt-1.c  -O2 -flto -fuse-linker-plugin
-fno-fat-lto-objects  (test for excess errors)

Again, wrong LTO alignment

> FAIL: gcc.dg/torture/stackalign/builtin-apply-2.c  -O2 -flto
-fno-use-linker-plugin -flto-partition=none  execution test

Testcase/builtin_apply have never been adapted for AVR


So the new FAILs are because the non-LTO -> LTO transition, not because of the
patch to review.

Ok to install?

Johann
Denis Chertykov Dec. 6, 2011, 2:19 p.m. UTC | #7
2011/12/6 Georg-Johann Lay <avr@gjlay.de>:
> Denis Chertykov wrote:
>> 2011/11/29 Georg-Johann:
>>> Ian Lance Taylor wrote:
>>>> Georg-Johann Lay:
>>>>
>>>>> So if a frontend can define address spaces and it is a generic feature, the
>>>>> question is how to get the name of an address space in a generic, language
>>>>> independent way.
>>>> We could decide that all frontends that use address spaces must define a
>>>> printable name for each address space.  That would mean changing the
>>>> middle-end address space interface to give a name to each address space.
>>>> The current middle-end address space interface does not require that
>>>> address spaces have a name.  I was not involved in the addition of
>>>> address spaces to gcc, and I don't know why they followed the path they
>>>> did.
>>>>
>>>> Ian
>>> Presumably they chose that approach to keep it simple or it is even a
>>> performance issue to move the name around.
>>>
>>> I attached a patch but I fail to find the right configure options for
>>> gcc/binutils as the testsuite complains
>>>
>>> ./avr/bin/ld: bad -plugin option
>>>
>>> Configured gcc with --enable-lto and binutils 2.21 with --enable-plugin.
>>>
>>> Maybe the patch can be pre-approved so that the others can proceed with their work?
>>
>> Better to complete this work.
>>
>> Denis.
>
> Finally, I could get LTO to work. Problem was in bad libdl.
>
> The patch is unchanged:
>
> http://gcc.gnu.org/ml/gcc-patches/2011-11/msg02574.html
>
> Diffs in testresults are:
>
>> FAIL: gcc.c-torture/execute/builtins/memcpy-chk.c compilation,  -O2 -flto
> -fuse-linker-plugin -fno-fat-lto-objects
>> FAIL: gcc.c-torture/execute/builtins/memmove-chk.c compilation,  -O2 -flto
> -fuse-linker-plugin -fno-fat-lto-objects
>> FAIL: gcc.c-torture/execute/builtins/mempcpy-chk.c compilation,  -O2 -flto
> -fuse-linker-plugin -fno-fat-lto-objects
>> FAIL: gcc.c-torture/execute/builtins/memset-chk.c compilation,  -O2 -flto
> -fuse-linker-plugin -fno-fat-lto-objects
>
> Problem is non-aligned LTO stuff like
>
> /home/georg/install/avr/bin/ld: Warning: alignment 1 of symbol `buf5' in
> /tmp/ccAMUQqq.ltrans1.ltrans.o is smaller than 16 in /tmp/ccehY2eq.o.ironly
> /home/georg/install/avr/bin/ld: Warning: alignment 1 of symbol `buf7' in
> /tmp/ccAMUQqq.ltrans1.ltrans.o is smaller than 16 in /tmp/ccehY2eq.o.ironly
> /home/georg/install/avr/bin/ld: Warning: alignment 1 of symbol `buf1' in
> /tmp/ccAMUQqq.ltrans1.ltrans.o is smaller than 16 in /tmp/ccehY2eq.o.ironly
> /home/georg/install/avr/bin/ld: Warning: alignment 1 of symbol `chk_calls' in
> /tmp/ccAMUQqq.ltrans1.ltrans.o is smaller than 2 in /tmp/cc9w5uuC.o.ironly
> /home/georg/install/avr/bin/ld: Warning: alignment 1 of symbol
> `memcpy_disallowed' in /tmp/ccAMUQqq.ltrans1.ltrans.o is smaller than 2 in
> /tmp/cc9w5uuC.o.ironly
>
>> FAIL: gcc.c-torture/execute/980709-1.c compilation,  -O2 -flto
> -fuse-linker-plugin -fno-fat-lto-objects
>
> Bug in avr-libc using RJMP where not appropriate leading to
>
> /home/georg/install/gcc-4.7/lib/gcc/avr/4.7.0/../../../../avr/lib/avr51/libc.a(log.o):../../../../../source/avr-libc-1.7.1/libm/fplib/log.S:96:
> relocation truncated to fit: R_AVR_13_PCREL against symbol `__addsf3' defined
> in .text section in
> /home/georg/build/gcc-trunk-avr/gcc/avr51/libgcc.a(_addsub_sf.o)
>
>> FAIL: gcc.c-torture/execute/simd-1.c compilation,  -O2 -flto
> -fuse-linker-plugin -fno-fat-lto-objects  (internal compiler error)
>
> reload fail because data (vector) too big: internal compiler error: in
> find_valid_class, at reload.c:707
>
>> FAIL: gcc.dg/visibility-d.c scan-not-hidden
>
> Problems with visibility pragma.
>
>> FAIL: gcc.dg/lto/20090218-2 c_lto_20090218-2_1.o assemble, -O0 -flto
> -flto-partition=none -fuse-linker-plugin
>> FAIL: gcc.dg/lto/20090218-2 c_lto_20090218-2_1.o assemble, -O2 -flto
> -flto-partition=none -fuse-linker-plugin -fno-fat-lto-objects
>> FAIL: gcc.dg/lto/20090218-2 c_lto_20090218-2_1.o assemble, -O0 -flto
> -flto-partition=1to1 -fno-use-linker-plugin
>> FAIL: gcc.dg/lto/20090218-2 c_lto_20090218-2_1.o assemble, -O2 -flto
> -flto-partition=1to1 -fno-use-linker-plugin
>> FAIL: gcc.dg/lto/20090218-2 c_lto_20090218-2_1.o assemble, -O0 -flto
> -fuse-linker-plugin -fno-fat-lto-objects
>> FAIL: gcc.dg/lto/20090218-2 c_lto_20090218-2_1.o assemble, -O2 -flto
> -fuse-linker-plugin
>
> Bad test case (use long instead of site_t)
>
>> FAIL: gcc.dg/lto/20091013-1 c_lto_20091013-1_1.o assemble, -fPIC -r -nostdlib
> -flto
>> FAIL: gcc.dg/lto/20091013-1 c_lto_20091013-1_2.o assemble, -fPIC -r -nostdlib
> -flto
>> FAIL: gcc.dg/lto/20091013-1 c_lto_20091013-1_1.o assemble, -fPIC -r -nostdlib
> -O2 -flto
>> FAIL: gcc.dg/lto/20091013-1 c_lto_20091013-1_2.o assemble, -fPIC -r -nostdlib
> -O2 -flto
>
> Bad test case: Assumes sizeof(long) == sizeof (void*)
>
>> FAIL: gcc.dg/lto/ipareference2
> c_lto_ipareference2_0.o-c_lto_ipareference2_1.o link,  -O1 -flto
> -flto-partition=1to1 -fwhole-program
>
> Warning: alignment 1 of symbol `e' in /tmp/ccXmAeaF.ltrans1.ltrans.o is smaller
> than 2 in c_lto_ipareference2_0.o.ironly
>
>> FAIL: gcc.dg/lto/trans-mem-1 c_lto_trans-mem-1_0.o-c_lto_trans-mem-1_1.o
> link, -flto -fgnu-tm
>
> Bad test case: Assumes transactional memory is available.
>
>> FAIL: gcc.dg/torture/fp-int-convert-double.c  -O2 -flto -fuse-linker-plugin
> -fno-fat-lto-objects  execution test
>> FAIL: gcc.dg/torture/fp-int-convert-float.c  -O2 -flto -fuse-linker-plugin
> -fno-fat-lto-objects  execution test
>> FAIL: gcc.dg/torture/fp-int-convert-long-double.c  -O2 -flto
> -fuse-linker-plugin -fno-fat-lto-objects  execution test
>
> Bad test cases: Assume TImode is available.
>
>> FAIL: gcc.dg/torture/pta-ptrarith-3.c  -O2 -flto -fno-use-linker-plugin
> -flto-partition=none  execution test
>
> Known bug: PR50063/PR49330
>
>> FAIL: gcc.dg/torture/vec-cvt-1.c  -O2 -flto -fuse-linker-plugin
> -fno-fat-lto-objects  (test for excess errors)
>
> Again, wrong LTO alignment
>
>> FAIL: gcc.dg/torture/stackalign/builtin-apply-2.c  -O2 -flto
> -fno-use-linker-plugin -flto-partition=none  execution test
>
> Testcase/builtin_apply have never been adapted for AVR
>
>
> So the new FAILs are because the non-LTO -> LTO transition, not because of the
> patch to review.
>
> Ok to install?

Ok.

Denis.
diff mbox

Patch

Index: config/avr/avr-c.c
===================================================================
--- config/avr/avr-c.c	(revision 181773)
+++ config/avr/avr-c.c	(working copy)
@@ -36,16 +36,39 @@ 
 void
 avr_register_target_pragmas (void)
 {
-  c_register_addr_space ("__pgm", ADDR_SPACE_PGM);
-  c_register_addr_space ("__pgm1", ADDR_SPACE_PGM1);
-  c_register_addr_space ("__pgm2", ADDR_SPACE_PGM2);
-  c_register_addr_space ("__pgm3", ADDR_SPACE_PGM3);
-  c_register_addr_space ("__pgm4", ADDR_SPACE_PGM4);
-  c_register_addr_space ("__pgm5", ADDR_SPACE_PGM5);
-  c_register_addr_space ("__pgmx", ADDR_SPACE_PGMX);
+  int i;
+
+  gcc_assert (ADDR_SPACE_GENERIC == ADDR_SPACE_RAM);
+
+  /* Register address spaces.  The order must be the same as in the respective
+     enum from avr.h (or designated initialized must be used in avr.c).  */
+
+  for (i = 0; avr_addrspace[i].name; i++)
+    {
+      gcc_assert (i == avr_addrspace[i].id);
+
+      if (!ADDR_SPACE_GENERIC_P (i))
+        c_register_addr_space (avr_addrspace[i].name, avr_addrspace[i].id);
+    }
 }
 
 
+/* Transorm LO into uppercase and write the result to UP.
+   You must provide enough space for UP.  Return UP.  */
+
+static char*
+avr_toupper (char *up, const char *lo)
+{
+  char *up0 = up;
+  
+  for (; *lo; lo++, up++)
+    *up = TOUPPER (*lo);
+
+  *up = '\0';
+
+  return up0;
+}
+             
 /* Worker function for TARGET_CPU_CPP_BUILTINS.  */
 
 void
@@ -117,13 +140,17 @@  avr_cpu_cpp_builtins (struct cpp_reader
   
   if (!strcmp (lang_hooks.name, "GNU C"))
     {
-      cpp_define (pfile, "__PGM=__pgm");
-      cpp_define (pfile, "__PGM1=__pgm1");
-      cpp_define (pfile, "__PGM2=__pgm2");
-      cpp_define (pfile, "__PGM3=__pgm3");
-      cpp_define (pfile, "__PGM4=__pgm4");
-      cpp_define (pfile, "__PGM5=__pgm5");
-      cpp_define (pfile, "__PGMX=__pgmx");
+      int i;
+      
+      for (i = 0; avr_addrspace[i].name; i++)
+        if (!ADDR_SPACE_GENERIC_P (i))
+          {
+            const char *name = avr_addrspace[i].name;
+            char *Name = (char*) alloca (1 + strlen (name));
+
+            cpp_define_formatted (pfile, "%s=%s",
+                                  avr_toupper (Name, name), name);
+          }
     }
 
   /* Define builtin macros so that the user can
Index: config/avr/avr.c
===================================================================
--- config/avr/avr.c	(revision 181773)
+++ config/avr/avr.c	(working copy)
@@ -55,11 +55,39 @@ 
 /* Return true if STR starts with PREFIX and false, otherwise.  */
 #define STR_PREFIX_P(STR,PREFIX) (0 == strncmp (STR, PREFIX, strlen (PREFIX)))
 
-/* The 4 bits starting at SECTION_MACH_DEP are reverved to store
-   1 + flash segment where progmem data is to be located.
-   For example, data with __pgm2 is stored as (1+2) * SECTION_MACH_DEP.  */
+/* The 4 bits starting at SECTION_MACH_DEP are reserved to store the
+   address space where data is to be located.
+   As the only non-generic address spaces are all located in Flash,
+   this can be used to test if data shall go into some .progmem* section.
+   This must be the rightmost field of machine dependent section flags.  */
 #define AVR_SECTION_PROGMEM (0xf * SECTION_MACH_DEP)
 
+/* Known address spaces.  The order must be the same as in the respective
+   enum from avr.h (or designated initialized must be used).  */
+const avr_addrspace_t avr_addrspace[] =
+{
+    { ADDR_SPACE_RAM,  0, 2, ""     ,   0 },
+    { ADDR_SPACE_PGM,  1, 2, "__pgm",   0 },
+    { ADDR_SPACE_PGM1, 1, 2, "__pgm1",  1 },
+    { ADDR_SPACE_PGM2, 1, 2, "__pgm2",  2 },
+    { ADDR_SPACE_PGM3, 1, 2, "__pgm3",  3 },
+    { ADDR_SPACE_PGM4, 1, 2, "__pgm4",  4 },
+    { ADDR_SPACE_PGM5, 1, 2, "__pgm5",  5 },
+    { ADDR_SPACE_PGMX, 1, 3, "__pgmx",  0 },
+    { 0              , 0, 0, NULL,      0 }
+};
+
+/* Map 64-k Flash segment to section prefix.  */
+static const char* const progmem_section_prefix[6] =
+  {
+    ".progmem.data",
+    ".progmem1.data",
+    ".progmem2.data",
+    ".progmem3.data",
+    ".progmem4.data",
+    ".progmem5.data"
+  };
+
 
 /* Prototypes for local helper functions.  */
 
@@ -132,19 +160,10 @@  const struct mcu_type_s *avr_current_dev
 /* Section to put switch tables in.  */
 static GTY(()) section *progmem_swtable_section;
 
-/* Unnamed section associated to __attribute__((progmem)) aka. PROGMEM.  */
+/* Unnamed sections associated to __attribute__((progmem)) aka. PROGMEM
+   or to address space __pgm*.  */
 static GTY(()) section *progmem_section[6];
 
-static const char * const progmem_section_prefix[6] =
-  {
-    ".progmem.data",
-    ".progmem1.data",
-    ".progmem2.data",
-    ".progmem3.data",
-    ".progmem4.data",
-    ".progmem5.data"
-  };
-
 /* To track if code will use .bss and/or .data.  */
 bool avr_need_clear_bss_p = false;
 bool avr_need_copy_data_p = false;
@@ -462,29 +481,6 @@  avr_scalar_mode_supported_p (enum machin
 }
 
 
-/* Return the segment number of pgm address space AS, i.e.
-   the 64k block it lives in.
-   Return -1 if unknown, i.e. 24-bit AS in flash.
-   Return -2 for anything else.  */
-
-static int
-avr_pgm_segment (addr_space_t as)
-{
-  switch (as)
-    {
-    default: return -2;
-
-    case ADDR_SPACE_PGMX:  return -1;
-    case ADDR_SPACE_PGM:   return 0;
-    case ADDR_SPACE_PGM1:  return 1;
-    case ADDR_SPACE_PGM2:  return 2;
-    case ADDR_SPACE_PGM3:  return 3;
-    case ADDR_SPACE_PGM4:  return 4;
-    case ADDR_SPACE_PGM5:  return 5;
-    }
-}
-
-
 /* Return TRUE if DECL is a VAR_DECL located in Flash and FALSE, otherwise.  */
 
 static bool
@@ -2582,6 +2578,9 @@  avr_out_lpm (rtx insn, rtx *op, int *ple
   int n_bytes = GET_MODE_SIZE (GET_MODE (dest));
   int regno_dest;
   int segment;
+  RTX_CODE code;
+  
+  addr_space_t as;
 
   if (plen)
     *plen = 0;
@@ -2589,27 +2588,29 @@  avr_out_lpm (rtx insn, rtx *op, int *ple
   if (MEM_P (dest))
     {
       warning (0, "writing to address space %qs not supported",
-               c_addr_space_name (MEM_ADDR_SPACE (dest)));
+               avr_addrspace[MEM_ADDR_SPACE (dest)].name);
       
       return "";
     }
 
-  addr = XEXP (src, 0);
-
-  segment = avr_pgm_segment (MEM_ADDR_SPACE (src));
+  as = MEM_ADDR_SPACE (src);
 
-  gcc_assert (REG_P (dest)
-              && ((segment >= 0
-                   && (REG_P (addr) || POST_INC == GET_CODE (addr)))
-                  || (GET_CODE (addr) == LO_SUM && segment == -1)));
+  addr = XEXP (src, 0);
+  code = GET_CODE (addr);
 
-  if (segment == -1)
+  gcc_assert (REG_P (dest));
+  
+  if (as == ADDR_SPACE_PGMX)
     {
       /* We are called from avr_out_xload because someone wrote
          __pgmx on a device with just one flash segment.  */
 
+      gcc_assert (LO_SUM == code);
+
       addr = XEXP (addr, 1);
     }
+  else
+    gcc_assert (REG == code || POST_INC == code);
 
   xop[0] = dest;
   xop[1] = addr;
@@ -2619,11 +2620,10 @@  avr_out_lpm (rtx insn, rtx *op, int *ple
 
   regno_dest = REGNO (dest);
 
-  /* Cut down segment number to a number the device actually
-     supports.  We do this late to preserve the address space's
-     name for diagnostics.  */
+  /* Cut down segment number to a number the device actually supports.
+     We do this late to preserve the address space's name for diagnostics.  */
 
-  segment %= avr_current_arch->n_segments;
+  segment = avr_addrspace[as].segment % avr_current_arch->n_segments;
 
   /* Set RAMPZ as needed.  */
 
@@ -2652,14 +2652,17 @@  avr_out_lpm (rtx insn, rtx *op, int *ple
         }
       
       xop[4] = xstring_e;
-    }
 
-  if ((segment == 0 && !AVR_HAVE_LPMX)
-      || (segment != 0 && !AVR_HAVE_ELPMX))
+      if (!AVR_HAVE_ELPMX)
+        return avr_out_lpm_no_lpmx (insn, xop, plen);
+    }
+  else if (!AVR_HAVE_LPMX)
     {
       return avr_out_lpm_no_lpmx (insn, xop, plen);
     }
 
+  /* We have [E]LPMX: Output reading from Flash the comfortable way.  */
+
   switch (GET_CODE (addr))
     {
     default:
@@ -6919,10 +6922,10 @@  avr_pgm_check_var_decl (tree node)
     {
       if (TYPE_P (node))
         error ("pointer targeting address space %qs must be const in %qT",
-               c_addr_space_name (as), node);
+               avr_addrspace[as].name, node);
       else
         error ("pointer targeting address space %qs must be const in %s %q+D",
-               c_addr_space_name (as), reason, node);
+               avr_addrspace[as].name, reason, node);
     }
 
   return reason == NULL;
@@ -6959,7 +6962,7 @@  avr_insert_attributes (tree node, tree *
           const char *reason = "__attribute__((progmem))";
 
           if (!ADDR_SPACE_GENERIC_P (as))
-            reason = c_addr_space_name (as);
+            reason = avr_addrspace[as].name;
           
           if (avr_log.progmem)
             avr_edump ("\n%?: %t\n%t\n", node, node0);
@@ -7136,7 +7139,8 @@  avr_asm_named_section (const char *name,
 {
   if (flags & AVR_SECTION_PROGMEM)
     {
-      int segment = (flags & AVR_SECTION_PROGMEM) / SECTION_MACH_DEP - 1;
+      addr_space_t as = (flags & AVR_SECTION_PROGMEM) / SECTION_MACH_DEP;
+      int segment = avr_addrspace[as].segment % avr_current_arch->n_segments;
       const char *old_prefix = ".rodata";
       const char *new_prefix = progmem_section_prefix[segment];
       const char *sname = new_prefix;
@@ -7165,7 +7169,6 @@  avr_asm_named_section (const char *name,
 static unsigned int
 avr_section_type_flags (tree decl, const char *name, int reloc)
 {
-  int prog;
   unsigned int flags = default_section_type_flags (decl, name, reloc);
 
   if (STR_PREFIX_P (name, ".noinit"))
@@ -7179,16 +7182,20 @@  avr_section_type_flags (tree decl, const
     }
 
   if (decl && DECL_P (decl)
-      && (prog = avr_progmem_p (decl, DECL_ATTRIBUTES (decl)), prog))
+      && avr_progmem_p (decl, DECL_ATTRIBUTES (decl)))
     {
-      int segment = 0;
+      addr_space_t as = TYPE_ADDR_SPACE (TREE_TYPE (decl));
+
+      /* Attribute progmem puts data in generic address space.
+         Set section flags as if it was in __pgm to get the right
+         section prefix in the remainder.  */
 
-      if (prog == 1)
-        segment = avr_pgm_segment (TYPE_ADDR_SPACE (TREE_TYPE (decl)));
+      if (ADDR_SPACE_GENERIC_P (as))
+        as = ADDR_SPACE_PGM;
 
+      flags |= as * SECTION_MACH_DEP;
       flags &= ~SECTION_WRITE;
       flags &= ~SECTION_BSS;
-      flags |= (1 + segment % avr_current_arch->n_segments) * SECTION_MACH_DEP;
     }
   
   return flags;
@@ -7198,8 +7205,7 @@  avr_section_type_flags (tree decl, const
 /* Implement `TARGET_ENCODE_SECTION_INFO'.  */
 
 static void
-avr_encode_section_info (tree decl, rtx rtl,
-                         int new_decl_p)
+avr_encode_section_info (tree decl, rtx rtl, int new_decl_p)
 {
   /* In avr_handle_progmem_attribute, DECL_INITIAL is not yet
      readily available, see PR34734.  So we postpone the warning
@@ -7224,19 +7230,13 @@  avr_encode_section_info (tree decl, rtx
 static section *
 avr_asm_select_section (tree decl, int reloc, unsigned HOST_WIDE_INT align)
 {
-  int prog;
-  
   section * sect = default_elf_select_section (decl, reloc, align);
   
   if (decl && DECL_P (decl)
-      && (prog = avr_progmem_p (decl, DECL_ATTRIBUTES (decl)), prog))
+      && avr_progmem_p (decl, DECL_ATTRIBUTES (decl)))
     {
-      int segment = 0;
-      
-      if (prog == 1)
-        segment = avr_pgm_segment (TYPE_ADDR_SPACE (TREE_TYPE (decl)));
-
-      segment %= avr_current_arch->n_segments;
+      addr_space_t as = TYPE_ADDR_SPACE (TREE_TYPE (decl));
+      int segment = avr_addrspace[as].segment % avr_current_arch->n_segments;
       
       if (sect->common.flags & SECTION_NAMED)
         {
@@ -9386,7 +9386,7 @@  avr_case_values_threshold (void)
 static enum machine_mode
 avr_addr_space_address_mode (addr_space_t as)
 {
-  return as == ADDR_SPACE_PGMX ? PSImode : HImode;
+  return avr_addrspace[as].pointer_size == 3 ? PSImode : HImode;
 }
 
 
@@ -9395,7 +9395,7 @@  avr_addr_space_address_mode (addr_space_
 static enum machine_mode
 avr_addr_space_pointer_mode (addr_space_t as)
 {
-  return as == ADDR_SPACE_PGMX ? PSImode : HImode;
+  return avr_addr_space_address_mode (as);
 }
 
 
@@ -9573,7 +9573,7 @@  avr_addr_space_convert (rtx src, tree ty
         }
       else
         {
-          int segment = avr_pgm_segment (as_from) % n_segments;
+          int segment = avr_addrspace[as_from].segment % n_segments;
 
           new_src = gen_reg_rtx (PSImode);
           emit_insn (gen_n_extendhipsi2 (new_src, GEN_INT (segment), src));
@@ -9642,15 +9642,12 @@  avr_emit_movmemhi (rtx *xop)
     }
   else
     {
-      int seg = avr_pgm_segment (as);
+      int segment = avr_addrspace[as].segment % avr_current_arch->n_segments;
       
       addr1 = a_src;
 
-      if (seg > 0
-          && seg % avr_current_arch->n_segments > 0)
-        {
-          a_hi8 = GEN_INT (seg % avr_current_arch->n_segments);
-        }
+      if (segment)
+        a_hi8 = GEN_INT (segment);
     }
 
   if (a_hi8
Index: config/avr/avr.h
===================================================================
--- config/avr/avr.h	(revision 181772)
+++ config/avr/avr.h	(working copy)
@@ -133,6 +133,40 @@  extern const struct mcu_type_s *avr_curr
 extern const struct mcu_type_s avr_mcu_types[];
 extern const struct base_arch_s avr_arch_types[];
 
+typedef struct
+{
+  /* Id of the address space as used in c_register_addr_space */
+  unsigned char id;
+
+  /* Flavour of memory: 0 = RAM, 1 = Flash */
+  int memory_class;
+
+  /* Width of pointer (in bytes) */
+  int pointer_size;
+
+  /* Name of the address space as visible to the user */
+  const char *name;
+
+  /* Segment (i.e. 64k memory chunk) number.  */
+  int segment;
+} avr_addrspace_t;
+
+extern const avr_addrspace_t avr_addrspace[];
+
+/* Known address spaces */
+
+enum
+  {
+    ADDR_SPACE_RAM,
+    ADDR_SPACE_PGM,
+    ADDR_SPACE_PGM1,
+    ADDR_SPACE_PGM2,
+    ADDR_SPACE_PGM3,
+    ADDR_SPACE_PGM4,
+    ADDR_SPACE_PGM5,
+    ADDR_SPACE_PGMX
+  };
+
 #define TARGET_CPU_CPP_BUILTINS()	avr_cpu_cpp_builtins (pfile)
 
 #define AVR_HAVE_JMP_CALL (avr_current_arch->have_jmp_call && !TARGET_SHORT_CALLS)
@@ -401,15 +435,6 @@  typedef struct avr_args {
 
 #define NO_FUNCTION_CSE
 
-
-#define ADDR_SPACE_PGM  1
-#define ADDR_SPACE_PGM1 2
-#define ADDR_SPACE_PGM2 3
-#define ADDR_SPACE_PGM3 4
-#define ADDR_SPACE_PGM4 5
-#define ADDR_SPACE_PGM5 6
-#define ADDR_SPACE_PGMX 7
-
 #define REGISTER_TARGET_PRAGMAS()                                       \
   do {                                                                  \
     avr_register_target_pragmas();                                      \