Re: [patch,avr] Fix PR59396, take 2: Ignore leading '*' in warning generation for ISR names

2014-03-14 Thread Denis Chertykov
2014-03-13 22:58 GMT+04:00 Georg-Johann Lay a...@gjlay.de:
 Am 03/13/2014 07:36 PM, schrieb Denis Chertykov:

 2014-03-13 21:41 GMT+04:00 Georg-Johann Lay:

 Am 03/13/2014 04:41 PM, schrieb Senthil Kumar Selvaraj:

 On Thu, Mar 13, 2014 at 02:24:06PM +0100, Georg-Johann Lay wrote:



 Problem is that the assembler name might or might not be prefixed by
 '*'
 depending on when TARGET_SET_CURRENT_FUNCTION is called.

 The change is just to fix wrong warning because the current
 implementation
 of TARGET_SET_CURRENT_FUNCTION /always/ skips the first char when the
 assembler name is set.



 FWIW, there's default_strip_name_encoding (varasm.c), which does the
 same
 thing, and is used by a couple of other targets.



 Yes, I know.

 But I would prefer targetm.strip_name_encoding then, even though avr does
 not implement it.


 I'm prefer `targetm.strip_name_encoding' or `default_strip_name_encoding'.
 May be `default_strip_name_encoding' is better because it's used in few
 ports.

 Denis.


 So here is the revised version of the patch.

 Johann


 PR target/59396
 * config/avr/avr.c (avr_set_current_function): Pass function name
 through default_strip_name_encoding before sanity checking instead
 of skipping the first char of the assembler name.


Approved.

Denis.


Re: [patch,avr] Fix PR59396, take 2: Ignore leading '*' in warning generation for ISR names

2014-03-13 Thread Georg-Johann Lay

Am 03/13/2014 07:36 PM, schrieb Denis Chertykov:

2014-03-13 21:41 GMT+04:00 Georg-Johann Lay:

Am 03/13/2014 04:41 PM, schrieb Senthil Kumar Selvaraj:


On Thu, Mar 13, 2014 at 02:24:06PM +0100, Georg-Johann Lay wrote:



Problem is that the assembler name might or might not be prefixed by '*'
depending on when TARGET_SET_CURRENT_FUNCTION is called.

The change is just to fix wrong warning because the current
implementation
of TARGET_SET_CURRENT_FUNCTION /always/ skips the first char when the
assembler name is set.



FWIW, there's default_strip_name_encoding (varasm.c), which does the same
thing, and is used by a couple of other targets.



Yes, I know.

But I would prefer targetm.strip_name_encoding then, even though avr does
not implement it.


I'm prefer `targetm.strip_name_encoding' or `default_strip_name_encoding'.
May be `default_strip_name_encoding' is better because it's used in few ports.

Denis.


So here is the revised version of the patch.

Johann


PR target/59396
* config/avr/avr.c (avr_set_current_function): Pass function name
through default_strip_name_encoding before sanity checking instead
of skipping the first char of the assembler name.

Index: config/avr/avr.c
===
--- config/avr/avr.c	(revision 208532)
+++ config/avr/avr.c	(working copy)
@@ -600,10 +600,14 @@ avr_set_current_function (tree decl)
   const char *name;
 
   name = DECL_ASSEMBLER_NAME_SET_P (decl)
-/* Remove the leading '*' added in set_user_assembler_name.  */
-? 1 + IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl))
+? IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl))
 : IDENTIFIER_POINTER (DECL_NAME (decl));
 
+  /* Skip a leading '*' that might still prefix the assembler name,
+ e.g. in non-LTO runs.  */
+
+  name = default_strip_name_encoding (name);
+
   /* Silently ignore 'signal' if 'interrupt' is present.  AVR-LibC startet
  using this when it switched from SIGNAL and INTERRUPT to ISR.  */