[Patch, AVR]: Fix PR34734

[Patch, AVR]: Fix PR34734

[Georg-Johann Lay]

[-- Attachment #1: Type: text/plain, Size: 507 bytes --]

PR34734 produces annoying, false warnings if __attribute__((progmem))
is used in conjunction with C++.  DECL_INITIAL is not yet set up in
avr_handle_progmem_attribute.

Johann

	PR target/34734
	* config/avr/avr.c (avr_handle_progmem_attribute): Move warning
	about uninitialized data attributed 'progmem' from here...
	(avr_encode_section_info): ...to this new function.
	(TARGET_ENCODE_SECTION_INFO): New define.
	(avr_section_type_flags): For data in ".progmem.data", remove
	section flag SECTION_WRITE.

[-- Attachment #2: pr34734.diff --]
[-- Type: text/x-patch, Size: 2345 bytes --]

Index: config/avr/avr.c
===================================================================
--- config/avr/avr.c	(revision 175201)
+++ config/avr/avr.c	(working copy)
@@ -109,6 +109,7 @@ static void avr_function_arg_advance (cu
 static void avr_help (void);
 static bool avr_function_ok_for_sibcall (tree, tree);
 static void avr_asm_named_section (const char *name, unsigned int flags, tree decl);
+static void avr_encode_section_info (tree, rtx, int);
 
 /* Allocate registers from r25 to r8 for parameters for function calls.  */
 #define FIRST_CUM_REG 26
@@ -200,6 +201,8 @@ static const struct attribute_spec avr_a
 
 #undef TARGET_ASM_INIT_SECTIONS
 #define TARGET_ASM_INIT_SECTIONS avr_asm_init_sections
+#undef TARGET_ENCODE_SECTION_INFO
+#define TARGET_ENCODE_SECTION_INFO avr_encode_section_info
 
 #undef TARGET_REGISTER_MOVE_COST
 #define TARGET_REGISTER_MOVE_COST avr_register_move_cost
@@ -5088,12 +5091,7 @@ avr_handle_progmem_attribute (tree *node
 	}
       else if (TREE_STATIC (*node) || DECL_EXTERNAL (*node))
 	{
-	  if (DECL_INITIAL (*node) == NULL_TREE && !DECL_EXTERNAL (*node))
-	    {
-	      warning (0, "only initialized variables can be placed into "
-		       "program memory area");
-	      *no_add_attrs = true;
-	    }
+          *no_add_attrs = false;
 	}
       else
 	{
@@ -5308,10 +5306,35 @@ avr_section_type_flags (tree decl, const
 		 ".noinit section");
     }
 
+  if (0 == strncmp (name, ".progmem.data", strlen (".progmem.data")))
+    flags &= ~SECTION_WRITE;
+  
   return flags;
 }
 
 
+/* Implement `TARGET_ENCODE_SECTION_INFO'.  */
+
+static void
+avr_encode_section_info (tree decl, rtx rtl ATTRIBUTE_UNUSED,
+                         int new_decl_p)
+{
+  /* In avr_handle_progmem_attribute, DECL_INITIAL is not yet
+     readily available, see PR34734.  So we postpone the warning
+     about uninitialized data in program memory section until here.  */
+   
+  if (new_decl_p
+      && decl && DECL_P (decl)
+      && NULL_TREE == DECL_INITIAL (decl)
+      && avr_progmem_p (decl, DECL_ATTRIBUTES (decl)))
+    {
+      warning (OPT_Wuninitialized,
+               "uninitialized variable %q+D put into "
+               "program memory area", decl);
+    }
+}
+
+
 /* Implement `TARGET_ASM_FILE_START'.  */
 /* Outputs some appropriate text to go at the start of an assembler
    file.  */

Ping #1: [Patch, AVR]: Fix PR34734

[Georg-Johann Lay]

http://gcc.gnu.org/ml/gcc-patches/2011-06/msg01462.html

Georg-Johann Lay wrote:
> PR34734 produces annoying, false warnings if __attribute__((progmem))
> is used in conjunction with C++.  DECL_INITIAL is not yet set up in
> avr_handle_progmem_attribute.
> 
> Johann
> 
> 	PR target/34734
> 	* config/avr/avr.c (avr_handle_progmem_attribute): Move warning
> 	about uninitialized data attributed 'progmem' from here...
> 	(avr_encode_section_info): ...to this new function.
> 	(TARGET_ENCODE_SECTION_INFO): New define.
> 	(avr_section_type_flags): For data in ".progmem.data", remove
> 	section flag SECTION_WRITE.

avr_encode_section_info is good place to emit the warning:
DECL_INITIAL has stabilized for C++, the warning will appear even for
unused variables that will eventually be thrown away, and the warning
appears only once (new_decl_p).

Johann

Re: Ping #1: [Patch, AVR]: Fix PR34734

[Denis Chertykov]

2011/6/28 Georg-Johann Lay <avr@gjlay.de>:
> http://gcc.gnu.org/ml/gcc-patches/2011-06/msg01462.html
>
> Georg-Johann Lay wrote:
>> PR34734 produces annoying, false warnings if __attribute__((progmem))
>> is used in conjunction with C++.  DECL_INITIAL is not yet set up in
>> avr_handle_progmem_attribute.
>>
>> Johann
>>
>>       PR target/34734
>>       * config/avr/avr.c (avr_handle_progmem_attribute): Move warning
>>       about uninitialized data attributed 'progmem' from here...
>>       (avr_encode_section_info): ...to this new function.
>>       (TARGET_ENCODE_SECTION_INFO): New define.
>>       (avr_section_type_flags): For data in ".progmem.data", remove
>>       section flag SECTION_WRITE.
>
> avr_encode_section_info is good place to emit the warning:
> DECL_INITIAL has stabilized for C++, the warning will appear even for
> unused variables that will eventually be thrown away, and the warning
> appears only once (new_decl_p).

Approved.

Denis.

Re: Ping #1: [Patch, AVR]: Fix PR34734

[Georg-Johann Lay]

Denis Chertykov wrote:
> 2011/6/28 Georg-Johann Lay <avr@gjlay.de>:
>> http://gcc.gnu.org/ml/gcc-patches/2011-06/msg01462.html
>>
>> Georg-Johann Lay wrote:
>>> PR34734 produces annoying, false warnings if __attribute__((progmem))
>>> is used in conjunction with C++.  DECL_INITIAL is not yet set up in
>>> avr_handle_progmem_attribute.
>>>
>>> Johann
>>>
>>>       PR target/34734
>>>       * config/avr/avr.c (avr_handle_progmem_attribute): Move warning
>>>       about uninitialized data attributed 'progmem' from here...
>>>       (avr_encode_section_info): ...to this new function.
>>>       (TARGET_ENCODE_SECTION_INFO): New define.
>>>       (avr_section_type_flags): For data in ".progmem.data", remove
>>>       section flag SECTION_WRITE.
>> avr_encode_section_info is good place to emit the warning:
>> DECL_INITIAL has stabilized for C++, the warning will appear even for
>> unused variables that will eventually be thrown away, and the warning
>> appears only once (new_decl_p).
> 
> Approved.
> 
> Denis.

Is this a patch that should be backported?
4.6?
4.5?

It's not fix for "bug or doc" but very annoying, false warning.

Johann

Re: Ping #1: [Patch, AVR]: Fix PR34734

[Denis Chertykov]

2011/6/29 Georg-Johann Lay <avr@gjlay.de>:
> Denis Chertykov wrote:
>> 2011/6/28 Georg-Johann Lay <avr@gjlay.de>:
>>> http://gcc.gnu.org/ml/gcc-patches/2011-06/msg01462.html
>>>
>>> Georg-Johann Lay wrote:
>>>> PR34734 produces annoying, false warnings if __attribute__((progmem))
>>>> is used in conjunction with C++.  DECL_INITIAL is not yet set up in
>>>> avr_handle_progmem_attribute.
>>>>
>>>> Johann
>>>>
>>>>       PR target/34734
>>>>       * config/avr/avr.c (avr_handle_progmem_attribute): Move warning
>>>>       about uninitialized data attributed 'progmem' from here...
>>>>       (avr_encode_section_info): ...to this new function.
>>>>       (TARGET_ENCODE_SECTION_INFO): New define.
>>>>       (avr_section_type_flags): For data in ".progmem.data", remove
>>>>       section flag SECTION_WRITE.
>>> avr_encode_section_info is good place to emit the warning:
>>> DECL_INITIAL has stabilized for C++, the warning will appear even for
>>> unused variables that will eventually be thrown away, and the warning
>>> appears only once (new_decl_p).
>>
>> Approved.
>>
>> Denis.
>
> Is this a patch that should be backported?
> 4.6?
> 4.5?
>
> It's not fix for "bug or doc" but very annoying, false warning.

You can backport it if you want.

I'm usually didn't backport such patches.

Denis.

Re: Ping #1: [Patch, AVR]: Fix PR34734

[Georg-Johann Lay]

Denis Chertykov wrote:
> 2011/6/29 Georg-Johann Lay <avr@gjlay.de>:
>> Denis Chertykov wrote:
>>> 2011/6/28 Georg-Johann Lay <avr@gjlay.de>:
>>>> http://gcc.gnu.org/ml/gcc-patches/2011-06/msg01462.html
>>>>
>>>> Georg-Johann Lay wrote:
>>>>> PR34734 produces annoying, false warnings if __attribute__((progmem))
>>>>> is used in conjunction with C++.  DECL_INITIAL is not yet set up in
>>>>> avr_handle_progmem_attribute.
>>>>>
>>>>> Johann
>>>>>
>>>>>       PR target/34734
>>>>>       * config/avr/avr.c (avr_handle_progmem_attribute): Move warning
>>>>>       about uninitialized data attributed 'progmem' from here...
>>>>>       (avr_encode_section_info): ...to this new function.
>>>>>       (TARGET_ENCODE_SECTION_INFO): New define.
>>>>>       (avr_section_type_flags): For data in ".progmem.data", remove
>>>>>       section flag SECTION_WRITE.
>>>> avr_encode_section_info is good place to emit the warning:
>>>> DECL_INITIAL has stabilized for C++, the warning will appear even for
>>>> unused variables that will eventually be thrown away, and the warning
>>>> appears only once (new_decl_p).
>>> Approved.
>>>
>>> Denis.
>> Is this a patch that should be backported?
>> 4.6?
>> 4.5?
>>
>> It's not fix for "bug or doc" but very annoying, false warning.
> 
> You can backport it if you want.
> 
> I'm usually didn't backport such patches.
> 
> Denis.

Ok, maybe Eric or Anatoly have some preference for 4.5/4.6.

Applied to 4.7 together with following corrigendum:

Index: config/avr/avr.c
===================================================================
--- config/avr/avr.c    (revision 175628)
+++ config/avr/avr.c    (working copy)
@@ -5161,7 +5161,7 @@ avr_section_type_flags (tree decl, const
 /* Implement `TARGET_ENCODE_SECTION_INFO'.  */

 static void
-avr_encode_section_info (tree decl, rtx rtl ATTRIBUTE_UNUSED,
+avr_encode_section_info (tree decl, rtx rtl,
                          int new_decl_p)
 {
   /* In avr_handle_progmem_attribute, DECL_INITIAL is not yet
@@ -5177,6 +5177,8 @@ avr_encode_section_info (tree decl, rtx
                "uninitialized variable %q+D put into "
                "program memory area", decl);
     }
+
+  default_encode_section_info (decl, rtl, new_decl_p);
 }

RE: Ping #1: [Patch, AVR]: Fix PR34734

[Weddington, Eric]

> -----Original Message-----
> From: Georg-Johann Lay [mailto:avr@gjlay.de]
> Sent: Wednesday, June 29, 2011 5:26 AM
> To: Denis Chertykov
> Cc: gcc-patches@gcc.gnu.org; Weddington, Eric; Anatoly Sokolov
> Subject: Re: Ping #1: [Patch, AVR]: Fix PR34734
> 
> >
> > You can backport it if you want.
> >
> > I'm usually didn't backport such patches.
> >
> > Denis.
> 
> Ok, maybe Eric or Anatoly have some preference for 4.5/4.6.
> 

I would like it for the 4.6 series, if possible. I think that the next releases of the various avr toolchain distributions will very probably be moving up to 4.6.x.

Eric

Re: Ping #1: [Patch, AVR]: Fix PR34734

[Georg-Johann Lay]

Weddington, Eric wrote:
>> -----Original Message-----

>>> You can backport it if you want.
>>> 
>>> I'm usually didn't backport such patches.
>>> 
>>> Denis.
>> Ok, maybe Eric or Anatoly have some preference for 4.5/4.6.
> 
> I would like it for the 4.6 series, if possible. I think that the
> next releases of the various avr toolchain distributions will very
> probably be moving up to 4.6.x.
> 
> Eric

http://gcc.gnu.org/viewcvs/branches/gcc-4_6-branch/gcc/config/avr/avr.c?r1=175706&r2=175704&pathrev=175706

Backported to 4.6 and set PR34734 milestone to 4.6.2.

Johann