public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [patch, avr] Fix mmcu to specs issues
@ 2016-07-26 10:20 Pitchumani Sivanupandi
  2016-07-26 12:30 ` Georg-Johann Lay
  0 siblings, 1 reply; 7+ messages in thread
From: Pitchumani Sivanupandi @ 2016-07-26 10:20 UTC (permalink / raw)
  To: GCC Patches; +Cc: Denis Chertykov, Georg-Johann Lay

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

avr-gcc expected to find the device specs in the search paths specified. But
it doesn't work as expected when device specs in different place than the
installed location.

Example-1:
avr-gcc wrongly assumes the device specs will be in first identified
'device-specs' folder in prefix path. avr-gcc natively supports device
at90pwm1, but complains as it couldn't find the specs file in the first
'device-specs' directory in the search path.


$ avr-gcc test.c -mmcu=at90pwm1 -B /home/install/dev/atxmega128a1u/

avr-gcc: error: cannot access device-specs for _at90pwm1_ expected at 
_/home/install/dev/atxmega128a1u/device-specs/specs-at90pwm1_
avr-gcc: note: devices natively supported: ata5272 ata5505 ata5702m322 
ata5782 ata5790 ata5790n ata5791 ata5795 ata5831 ata6285 ata6286 ata6289 
ata6612c ata6613c ata6614q ata6616c ata6617c ata664251 ata8210 ata8510 
atmega103 atmega128 atmega128a atmega128rfa1 atmega128rfr2 atmega1280 
atmega1281 atmega1284 atmega1284p atmega1284rfr2 atmega16 atmega16a 
atmega16hva atmega16hva2 atmega16hvb atmega16hvbrevb atmega16m1 
atmega16u2 atmega16u4 atmega161 atmega162 atmega163 atmega164a 
atmega164p atmega164pa atmega165 atmega165a atmega165p atmega165pa 
atmega168 atmega168a atmega168p atmega168pa atmega168pb atmega169 
atmega169a atmega169p atmega169pa atmega256rfr2 atmega2560 atmega2561 
atmega2564rfr2 atmega32 atmega32a atmega32c1 atmega32hvb atmega32hvbrevb 
atmega32m1 atmega32u2 atmega32u4 atmega32u6 atmega323 atmega324a 
atmega324p atmega324pa atmega325 atmega325a atmega325p atmega325pa 
atmega3250 atmega3250a atmega3250p atmega3250pa atmega328 atmega328p 
atmega328pb atmega329 atmega329a atmega329p atmega329pa atmega3290 
atmega3290a atmega3290p atmega3290pa atmega406 atmega48 atmega48a 
atmega48p atmega48pa atmega48pb atmega64 atmega64a atmega64c1 
atmega64hve atmega64hve2 atmega64m1 atmega64rfr2 atmega640 atmega644 
atmega644a atmega644p atmega644pa atmega644rfr2 atmega645 atmega645a 
atmega645p atmega6450 atmega6450a atmega6450p atmega649 atmega649a 
atmega649p atmega6490 atmega6490a atmega6490p atmega8 atmega8a 
atmega8hva atmega8u2 atmega8515 atmega8535 atmega88 atmega88a atmega88p 
atmega88pa atmega88pb attiny10 attiny11 attiny12 attiny13 attiny13a 
attiny15 attiny1634 attiny167 attiny20 attiny22 attiny2313 attiny2313a 
attiny24 attiny24a attiny25 attiny26 attiny261 attiny261a attiny28 
attiny4 attiny40 attiny43u attiny4313 attiny44 attiny44a attiny441 
attiny45 attiny461 attiny461a attiny48 attiny5 attiny828 attiny84 
attiny84a attiny841 attiny85 attiny861 attiny861a attiny87 attiny88 
attiny9 atxmega128a1 atxmega128a1u atxmega128a3 atxmega128a3u 
atxmega128a4u atxmega128b1 atxmega128b3 atxmega128c3 atxmega128d3 
atxmega128d4 atxmega16a4 atxmega16a4u atxmega16c4 atxmega16d4 
atxmega16e5 atxmega192a3 atxmega192a3u atxmega192c3 atxmega192d3 
atxmega256a3 atxmega256a3b atxmega256a3bu atxmega256a3u atxmega256c3 
atxmega256d3 atxmega32a4 atxmega32a4u atxmega32c3 atxmega32c4 
atxmega32d3 atxmega32d4 atxmega32e5 atxmega384c3 atxmega384d3 
atxmega64a1 atxmega64a1u atxmega64a3 atxmega64a3u atxmega64a4u 
atxmega64b1 atxmega64b3 atxmega64c3 atxmega64d3 atxmega64d4 atxmega8e5 
at43usb320 at43usb355 at76c711 at86rf401 at90can128 at90can32 at90can64 
at90c8534 at90pwm1 at90pwm161 at90pwm2 at90pwm2b at90pwm216 at90pwm3 
at90pwm3b at90pwm316 at90pwm81 at90scr100 at90s1200 at90s2313 at90s2323 
at90s2333 at90s2343 at90s4414 at90s4433 at90s4434 at90s8515 at90s8535 
at90usb1286 at90usb1287 at90usb162 at90usb646 at90usb647 at90usb82 at94k 
m3000
avr-gcc: note: supported core architectures: avr2 avr25 avr3 avr31 avr35 
avr4 avr5 avr51 avr6 avrxmega2 avrxmega4 avrxmega5 avrxmega6 avrxmega7 
avrtiny avr1
avr-gcc: note: you can provide your own specs files, see 
<http://gcc.gnu.org/onlinedocs/gcc/Spec-Files.html> for details


Example-2:
Similar issue happens when -flto option is enabled and device specs in 
custom
search path.

atxmega128a1u device specs in custom path and that is passed with -B option.
Architecture spec files such as specs-avrxmega7 will be in installed 
location.

$ avr-gcc test.c -mmcu=atxmega128a1u -flto -B 
/home/install/dev/atxmega128a1u/

avr-gcc: error: cannot access device-specs for _avrxmega7_ expected at 
_/home/install/dev/atxmega128a1u/device-specs/specs-avrxmega7_
avr-gcc: note: supported core architectures: avr2 avr25 avr3 avr31 avr35 
avr4 avr5 avr51 avr6 avrxmega2 avrxmega4 avrxmega5 avrxmega6 avrxmega7 
avrtiny avr1
avr-gcc: note: you can provide your own specs files, see 
<http://gcc.gnu.org/onlinedocs/gcc/Spec-Files.html> for details
lto-wrapper: fatal error: avr-gcc returned 1 exit status
compilation terminated.
/home/avr-trunk/install/lib/gcc/avr/7.0.0/../../../../avr/bin/ld: error: 
lto-wrapper failed
collect2: error: ld returned 1 exit status

Attached patch to address these issues.

Fix:
Replace device-specs-file spec function with mmcu-to-device-specs. This will
not assume that specs files are in first identified device-specs directory.
Instead this spec function will let gcc identify the absolute path of 
specs file
using spec string "device-specs-file (device-specs/specs-<mcu>%s)".

New spec function for device-specs-file, that will check the access for 
device
specs file and issue diagnostics.

If Ok, could someone commit please? I do not have commit access.

Regards,
Pitchumani

gcc/ChangeLog

2016-07-26  Pitchumani Sivanupandi  <pitchumani.s@atmel.com>

     * config/avr/avr.h: Declare spec handle function 
avr_mmcu_to_devicespecs.
     (EXTRA_SPEC_FUNCTIONS): Add mmcu-to-device-specs spec function.
     (DRIVER_SELF_SPECS): Replace device-specs-file spec function with
     mmcu-to-device-specs.
     * config/avr/driver-avr.c (avr_diagnose_devicespecs_error): Remove.
     (avr_devicespecs_file): Implement it for device-specs-file spec 
function.
     Issue error if device specs file is not accessible.
     (avr_mmcu_to_devicespecs): Implement it for mmcu-to-device-specs spec
     function. Return device-specs-file spec string with deduced specs file.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: avr-mmcu-to-specs.patch --]
[-- Type: text/x-patch; name="avr-mmcu-to-specs.patch", Size: 5943 bytes --]

diff --git a/gcc/config/avr/avr.h b/gcc/config/avr/avr.h
index 5eb90b5..f69ed22 100644
--- a/gcc/config/avr/avr.h
+++ b/gcc/config/avr/avr.h
@@ -508,9 +508,11 @@ typedef struct avr_args
 #define ADJUST_INSN_LENGTH(INSN, LENGTH)                \
     (LENGTH = avr_adjust_insn_length (INSN, LENGTH))
 
+extern const char *avr_mmcu_to_devicespecs (int, const char**);
 extern const char *avr_devicespecs_file (int, const char**);
 
 #define EXTRA_SPEC_FUNCTIONS                                   \
+  { "mmcu-to-device-specs", avr_mmcu_to_devicespecs }, \
   { "device-specs-file", avr_devicespecs_file },
 
 /* Driver self specs has lmited functionality w.r.t. '%s' for dynamic specs.
@@ -519,7 +521,7 @@ extern const char *avr_devicespecs_file (int, const char**);
 
 #undef  DRIVER_SELF_SPECS
 #define DRIVER_SELF_SPECS                       \
-  " %:device-specs-file(device-specs%s %{mmcu=*:%*})"
+  " %:mmcu-to-device-specs(device-specs%s %{mmcu=*:%*})"
 
 /* No libstdc++ for now.  Empty string doesn't work.  */
 #define LIBSTDCXX "gcc"
diff --git a/gcc/config/avr/driver-avr.c b/gcc/config/avr/driver-avr.c
index 83ca373..1d75c01 100644
--- a/gcc/config/avr/driver-avr.c
+++ b/gcc/config/avr/driver-avr.c
@@ -32,12 +32,42 @@ static const char dir_separator_str[] = { DIR_SEPARATOR, 0 };
 static const char specfiles_doc_url[] =
   "http://gcc.gnu.org/onlinedocs/gcc/Spec-Files.html";
 
+/* Implement spec function 'device-specs-file'.
 
-static const char*
-avr_diagnose_devicespecs_error (const char *mcu, const char *filename)
+   Compose -specs=<spec file name>. Issue error if the spec file is not
+   accessible.
+*/
+const char*
+avr_devicespecs_file (int argc, const char **argv)
 {
-  error ("cannot access device-specs for %qs expected at %qs",
-         mcu, filename);
+  const char *specfile_name = NULL;
+  const char *mcu = NULL;
+
+  switch (argc)
+    {
+    case 2:
+      specfile_name = argv[0];
+      mcu = argv[1];
+      break;
+
+    default:
+      fatal_error (input_location,
+                   "bad usage of spec function %qs", "device-specs-file");
+      return "";
+    }
+
+  if (!access (specfile_name, R_OK))
+    {
+#ifdef DEBUG_SPECS
+  if (verbose_flag)
+    fnotice (stderr, "'%s': mmcu='%s'\n'%s': specfile='%s'\n\n",
+             __FUNCTION__, mcu, __FUNCTION__, specfile_name);
+#endif
+      return concat ("-specs=", specfile_name, NULL);
+    }
+
+  error ("cannot access device-specs for %qs expected at GCC-SEARCH-DIRS/device-specs.",
+         mcu);
 
   // Inform about natively supported devices and cores.
 
@@ -49,21 +79,21 @@ avr_diagnose_devicespecs_error (const char *mcu, const char *filename)
   inform (input_location, "you can provide your own specs files, "
           "see <%s> for details", specfiles_doc_url);
 
-  return X_NODEVLIB;
+  return "";
 }
 
 
-/* Implement spec function `device-specs-file´.
+/* Implement spec function `mmcu-to-device-specs'.
 
-   Compose -specs=<specs-file-name>%s.  If everything went well then argv[0]
-   is the inflated (absolute) specs directory and argv[1] is a device or
-   core name as supplied by -mmcu=*.  When building GCC the path might
-   be relative.  */
+   Validate mcu name given with -mmcu option. Use spec function
+   device-specs-file to get -specs=<specs-file-name>. If everything went
+   well then argv[0] is the inflated (absolute) first specs directory and
+   argv[1] is a device or core name as supplied by -mmcu=*.  When building
+   GCC the path might be relative.  */
 
 const char*
-avr_devicespecs_file (int argc, const char **argv)
+avr_mmcu_to_devicespecs (int argc, const char **argv)
 {
-  char *specfile_name;
   const char *mmcu = NULL;
 
 #ifdef DEBUG_SPECS
@@ -76,7 +106,7 @@ avr_devicespecs_file (int argc, const char **argv)
     {
     case 0:
       fatal_error (input_location,
-                   "bad usage of spec function %qs", "device-specs-file");
+                   "bad usage of spec function %qs", "mmcu-to-device-specs");
       return X_NODEVLIB;
 
     case 1:
@@ -111,12 +141,10 @@ avr_devicespecs_file (int argc, const char **argv)
       break;
     }
 
-  specfile_name = concat (argv[0], dir_separator_str, "specs-", mmcu, NULL);
-
 #ifdef DEBUG_SPECS
   if (verbose_flag)
-    fnotice (stderr, "'%s': mmcu='%s'\n'%s': specfile='%s'\n\n",
-             __FUNCTION__, mmcu, __FUNCTION__, specfile_name);
+    fnotice (stderr, "'%s': mmcu='%s'\n\n",
+             __FUNCTION__, mmcu);
 #endif
 
   // Filter out silly -mmcu= arguments like "foo bar".
@@ -131,26 +159,14 @@ avr_devicespecs_file (int argc, const char **argv)
         return X_NODEVLIB;
       }
 
-  if (/* When building / configuring the compiler we might get a relative path
-         as supplied by "-B.".  Assume that the specs file exists and MCU is
-         a core, not a proper device then, i.e. we have "-mmcu=avr*".  */
-      (0 == strncmp (mmcu, "avr", strlen ("avr"))
-       && specfile_name[0] == '.')
-      /* vanilla */
-      || (IS_ABSOLUTE_PATH (specfile_name)
-          && !access (specfile_name, R_OK)))
-    {
-      return concat ("-specs=device-specs", dir_separator_str, "specs-", mmcu,
-                     // Use '%s' instead of the expanded specfile_name.  This
-                     // is the easiest way to handle pathes containing spaces.
-                     "%s",
+  /* Use spec function 'device-specs-file' to handle device-specs file.
+  */
+  return concat ("%:device-specs-file(device-specs", dir_separator_str, "specs-",
+                 mmcu, "%s ", mmcu, ")",
 #if defined (WITH_AVRLIBC)
-                     " %{mmcu=avr*:" X_NODEVLIB "} %{!mmcu=*:" X_NODEVLIB "}",
+                 " %{mmcu=avr*:" X_NODEVLIB "} %{!mmcu=*:" X_NODEVLIB "}",
 #else
-                     " " X_NODEVLIB,
+                 " " X_NODEVLIB,
 #endif
-                     NULL);
-    }
-
-  return avr_diagnose_devicespecs_error (mmcu, specfile_name);
+                 NULL);
 }

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [patch, avr] Fix mmcu to specs issues
  2016-07-26 10:20 [patch, avr] Fix mmcu to specs issues Pitchumani Sivanupandi
@ 2016-07-26 12:30 ` Georg-Johann Lay
  2016-07-28 11:49   ` Pitchumani Sivanupandi
  0 siblings, 1 reply; 7+ messages in thread
From: Georg-Johann Lay @ 2016-07-26 12:30 UTC (permalink / raw)
  To: Pitchumani Sivanupandi; +Cc: GCC Patches, Denis Chertykov

On 26.07.2016 12:20, Pitchumani Sivanupandi wrote:
> avr-gcc expected to find the device specs in the search paths specified. But
> it doesn't work as expected when device specs in different place than the
> installed location.
>
> Example-1:
> avr-gcc wrongly assumes the device specs will be in first identified
> 'device-specs' folder in prefix path. avr-gcc natively supports device
> at90pwm1, but complains as it couldn't find the specs file in the first
> 'device-specs' directory in the search path.
>
>
> $ avr-gcc test.c -mmcu=at90pwm1 -B /home/install/dev/atxmega128a1u/
>
> avr-gcc: error: cannot access device-specs for _at90pwm1_ expected at

Are the "_"s literally? Then the spec file name should be "specs-_at90pwm1_".

> _/home/install/dev/atxmega128a1u/device-specs/specs-at90pwm1_
> avr-gcc: note: devices natively supported: ata5272 ata5505 ata5702m322 ata5782
> ata5790 ata5790n ata5791 ata5795 ata5831 ata6285 ata6286 ata6289 ata6612c
> ata6613c ata6614q ata6616c ata6617c ata664251 ata8210 ata8510 atmega103
> atmega128 atmega128a atmega128rfa1 atmega128rfr2 atmega1280 atmega1281
> atmega1284 atmega1284p atmega1284rfr2 atmega16 atmega16a atmega16hva
> atmega16hva2 atmega16hvb atmega16hvbrevb atmega16m1 atmega16u2 atmega16u4
> atmega161 atmega162 atmega163 atmega164a atmega164p atmega164pa atmega165
> atmega165a atmega165p atmega165pa atmega168 atmega168a atmega168p atmega168pa
> atmega168pb atmega169 atmega169a atmega169p atmega169pa atmega256rfr2
> atmega2560 atmega2561 atmega2564rfr2 atmega32 atmega32a atmega32c1 atmega32hvb
> atmega32hvbrevb atmega32m1 atmega32u2 atmega32u4 atmega32u6 atmega323
> atmega324a atmega324p atmega324pa atmega325 atmega325a atmega325p atmega325pa
> atmega3250 atmega3250a atmega3250p atmega3250pa atmega328 atmega328p
> atmega328pb atmega329 atmega329a atmega329p atmega329pa atmega3290 atmega3290a
> atmega3290p atmega3290pa atmega406 atmega48 atmega48a atmega48p atmega48pa
> atmega48pb atmega64 atmega64a atmega64c1 atmega64hve atmega64hve2 atmega64m1
> atmega64rfr2 atmega640 atmega644 atmega644a atmega644p atmega644pa
> atmega644rfr2 atmega645 atmega645a atmega645p atmega6450 atmega6450a
> atmega6450p atmega649 atmega649a atmega649p atmega6490 atmega6490a atmega6490p
> atmega8 atmega8a atmega8hva atmega8u2 atmega8515 atmega8535 atmega88 atmega88a
> atmega88p atmega88pa atmega88pb attiny10 attiny11 attiny12 attiny13 attiny13a
> attiny15 attiny1634 attiny167 attiny20 attiny22 attiny2313 attiny2313a attiny24
> attiny24a attiny25 attiny26 attiny261 attiny261a attiny28 attiny4 attiny40
> attiny43u attiny4313 attiny44 attiny44a attiny441 attiny45 attiny461 attiny461a
> attiny48 attiny5 attiny828 attiny84 attiny84a attiny841 attiny85 attiny861
> attiny861a attiny87 attiny88 attiny9 atxmega128a1 atxmega128a1u atxmega128a3
> atxmega128a3u atxmega128a4u atxmega128b1 atxmega128b3 atxmega128c3 atxmega128d3
> atxmega128d4 atxmega16a4 atxmega16a4u atxmega16c4 atxmega16d4 atxmega16e5
> atxmega192a3 atxmega192a3u atxmega192c3 atxmega192d3 atxmega256a3 atxmega256a3b
> atxmega256a3bu atxmega256a3u atxmega256c3 atxmega256d3 atxmega32a4 atxmega32a4u
> atxmega32c3 atxmega32c4 atxmega32d3 atxmega32d4 atxmega32e5 atxmega384c3
> atxmega384d3 atxmega64a1 atxmega64a1u atxmega64a3 atxmega64a3u atxmega64a4u
> atxmega64b1 atxmega64b3 atxmega64c3 atxmega64d3 atxmega64d4 atxmega8e5
> at43usb320 at43usb355 at76c711 at86rf401 at90can128 at90can32 at90can64
> at90c8534 at90pwm1 at90pwm161 at90pwm2 at90pwm2b at90pwm216 at90pwm3 at90pwm3b
> at90pwm316 at90pwm81 at90scr100 at90s1200 at90s2313 at90s2323 at90s2333
> at90s2343 at90s4414 at90s4433 at90s4434 at90s8515 at90s8535 at90usb1286
> at90usb1287 at90usb162 at90usb646 at90usb647 at90usb82 at94k m3000
> avr-gcc: note: supported core architectures: avr2 avr25 avr3 avr31 avr35 avr4
> avr5 avr51 avr6 avrxmega2 avrxmega4 avrxmega5 avrxmega6 avrxmega7 avrtiny avr1
> avr-gcc: note: you can provide your own specs files, see
> <http://gcc.gnu.org/onlinedocs/gcc/Spec-Files.html> for details
>
>
> Example-2:
> Similar issue happens when -flto option is enabled and device specs in custom
> search path.
>
> atxmega128a1u device specs in custom path and that is passed with -B option.
> Architecture spec files such as specs-avrxmega7 will be in installed location.
>
> $ avr-gcc test.c -mmcu=atxmega128a1u -flto -B /home/install/dev/atxmega128a1u/
>
> avr-gcc: error: cannot access device-specs for _avrxmega7_ expected at
> _/home/install/dev/atxmega128a1u/device-specs/specs-avrxmega7_
> avr-gcc: note: supported core architectures: avr2 avr25 avr3 avr31 avr35 avr4
> avr5 avr51 avr6 avrxmega2 avrxmega4 avrxmega5 avrxmega6 avrxmega7 avrtiny avr1
> avr-gcc: note: you can provide your own specs files, see
> <http://gcc.gnu.org/onlinedocs/gcc/Spec-Files.html> for details
> lto-wrapper: fatal error: avr-gcc returned 1 exit status
> compilation terminated.
> /home/avr-trunk/install/lib/gcc/avr/7.0.0/../../../../avr/bin/ld: error:
> lto-wrapper failed
> collect2: error: ld returned 1 exit status
>
> Attached patch to address these issues.
>
> Fix:
> Replace device-specs-file spec function with mmcu-to-device-specs. This will
> not assume that specs files are in first identified device-specs directory.
> Instead this spec function will let gcc identify the absolute path of specs file
> using spec string "device-specs-file (device-specs/specs-<mcu>%s)".

IIUC this leads to problems with LTO and when the install path contains spaces 
which windows distros usually have.  The problem is that the spec function 
cannot escape the spaces as it would need more than 1 escape level.

It might also be the case that -mmcu= is specified more than once (with the 
same MCU), but I don't remember exactly in which situations this happens... 
Doesn't this lead to inclusion of more than one specs-file?


Johann

> New spec function for device-specs-file, that will check the access for device
> specs file and issue diagnostics.
>
> If Ok, could someone commit please? I do not have commit access.
>
> Regards,
> Pitchumani
>
> gcc/ChangeLog
>
> 2016-07-26  Pitchumani Sivanupandi  <pitchumani.s@atmel.com>
>
>     * config/avr/avr.h: Declare spec handle function avr_mmcu_to_devicespecs.
>     (EXTRA_SPEC_FUNCTIONS): Add mmcu-to-device-specs spec function.
>     (DRIVER_SELF_SPECS): Replace device-specs-file spec function with
>     mmcu-to-device-specs.
>     * config/avr/driver-avr.c (avr_diagnose_devicespecs_error): Remove.
>     (avr_devicespecs_file): Implement it for device-specs-file spec function.
>     Issue error if device specs file is not accessible.
>     (avr_mmcu_to_devicespecs): Implement it for mmcu-to-device-specs spec
>     function. Return device-specs-file spec string with deduced specs file.
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [patch, avr] Fix mmcu to specs issues
  2016-07-26 12:30 ` Georg-Johann Lay
@ 2016-07-28 11:49   ` Pitchumani Sivanupandi
  2016-07-29  8:37     ` Georg-Johann Lay
  0 siblings, 1 reply; 7+ messages in thread
From: Pitchumani Sivanupandi @ 2016-07-28 11:49 UTC (permalink / raw)
  To: Georg-Johann Lay; +Cc: GCC Patches, Denis Chertykov

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

On Tuesday 26 July 2016 06:00 PM, Georg-Johann Lay wrote:
> On 26.07.2016 12:20, Pitchumani Sivanupandi wrote:
>> avr-gcc expected to find the device specs in the search paths 
>> specified. But
>> it doesn't work as expected when device specs in different place than 
>> the
>> installed location.
>>
>> Example-1:
>> avr-gcc wrongly assumes the device specs will be in first identified
>> 'device-specs' folder in prefix path. avr-gcc natively supports device
>> at90pwm1, but complains as it couldn't find the specs file in the first
>> 'device-specs' directory in the search path.
>>
>>
>> $ avr-gcc test.c -mmcu=at90pwm1 -B /home/install/dev/atxmega128a1u/
>>
>> avr-gcc: error: cannot access device-specs for _at90pwm1_ expected at
>
> Are the "_"s literally? Then the spec file name should be 
> "specs-_at90pwm1_".
No, It was mangled character shown by my terminal for format specifier 
"%qs" in printf.
Ignore it.
...
>> Example-2:
>> Similar issue happens when -flto option is enabled and device specs 
>> in custom
>> search path.
>>
>> atxmega128a1u device specs in custom path and that is passed with -B 
>> option.
>> Architecture spec files such as specs-avrxmega7 will be in installed 
>> location.
>>
>> $ avr-gcc test.c -mmcu=atxmega128a1u -flto -B 
>> /home/install/dev/atxmega128a1u/
>>
>> avr-gcc: error: cannot access device-specs for _avrxmega7_ expected at
>> _/home/install/dev/atxmega128a1u/device-specs/specs-avrxmega7_
>> avr-gcc: note: supported core architectures: avr2 avr25 avr3 avr31 
>> avr35 avr4
>> avr5 avr51 avr6 avrxmega2 avrxmega4 avrxmega5 avrxmega6 avrxmega7 
>> avrtiny avr1
>> avr-gcc: note: you can provide your own specs files, see
>> <http://gcc.gnu.org/onlinedocs/gcc/Spec-Files.html> for details
>> lto-wrapper: fatal error: avr-gcc returned 1 exit status
>> compilation terminated.
>> /home/avr-trunk/install/lib/gcc/avr/7.0.0/../../../../avr/bin/ld: error:
>> lto-wrapper failed
>> collect2: error: ld returned 1 exit status
>>
>> Attached patch to address these issues.
>>
>> Fix:
>> Replace device-specs-file spec function with mmcu-to-device-specs. 
>> This will
>> not assume that specs files are in first identified device-specs 
>> directory.
>> Instead this spec function will let gcc identify the absolute path of 
>> specs file
>> using spec string "device-specs-file (device-specs/specs-<mcu>%s)".
>
> IIUC this leads to problems with LTO and when the install path 
> contains spaces which windows distros usually have.  The problem is 
> that the spec function cannot escape the spaces as it would need more 
> than 1 escape level.
>
> It might also be the case that -mmcu= is specified more than once 
> (with the same MCU), but I don't remember exactly in which situations 
> this happens... Doesn't this lead to inclusion of more than one 
> specs-file?

Yes, it lead to space problem.

Modified the patch because of path with spaces problem. When LTO 
enabled, multiple specs-file are
included as -mmcu=<arch> is fed back to gcc. It happens with/ without of 
this patch.
e.g. For atmega164p, device's and architecture's specs file given when 
invoking collect2.
  -specs=device-specs/specs-atmega164p -specs=device-specs/specs-avr5

Attached new patch. It just removes the conditions that led to 
originally stated issues.
(In driver-avr.c:avr_devicespecs_file)
Removed first condition as -mmcu=avr* shall come when LTO enabled. 
Second condition to
check absolute path is wrong as the specfile_name composed here will not 
be available
if the specs file is not present in first 'device-specs' directory.

With this approach, we can't issue 'descriptive' error for unavailable 
specs-file.
But, avr-gcc will issue below error if specs file is not found.
$ avr-gcc -mmcu=unknown test.c
avr-gcc: error: device-specs/specs-unknown: No such file or directory

Is that Ok?

Regards,
Pitchumani

gcc/ChangeLog

2016-07-28  Pitchumani Sivanupandi <pitchumani.s@atmel.com>

     * config/avr/driver-avr.c (specfiles_doc_url): Remove.
     (avr_diagnose_devicespecs_error): Remove.
     (avr_devicespecs_file): Remove conditions to check specs-file,
     let -specs= option handler do the validation.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: avr-mmcu-to-specs-rev2.patch --]
[-- Type: text/x-patch; name="avr-mmcu-to-specs-rev2.patch", Size: 3478 bytes --]

diff --git a/gcc/config/avr/driver-avr.c b/gcc/config/avr/driver-avr.c
index 83ca373..647f91b 100644
--- a/gcc/config/avr/driver-avr.c
+++ b/gcc/config/avr/driver-avr.c
@@ -29,41 +29,18 @@ along with GCC; see the file COPYING3.  If not see
 
 static const char dir_separator_str[] = { DIR_SEPARATOR, 0 };
 
-static const char specfiles_doc_url[] =
-  "http://gcc.gnu.org/onlinedocs/gcc/Spec-Files.html";
-
-
-static const char*
-avr_diagnose_devicespecs_error (const char *mcu, const char *filename)
-{
-  error ("cannot access device-specs for %qs expected at %qs",
-         mcu, filename);
-
-  // Inform about natively supported devices and cores.
-
-  if (strncmp (mcu, "avr", strlen ("avr")))
-    avr_inform_devices ();
-
-  avr_inform_core_architectures ();
-
-  inform (input_location, "you can provide your own specs files, "
-          "see <%s> for details", specfiles_doc_url);
-
-  return X_NODEVLIB;
-}
-
 
 /* Implement spec function `device-specs-file´.
 
-   Compose -specs=<specs-file-name>%s.  If everything went well then argv[0]
-   is the inflated (absolute) specs directory and argv[1] is a device or
-   core name as supplied by -mmcu=*.  When building GCC the path might
-   be relative.  */
+   Validate mcu name given with -mmcu option. Compose
+   -specs=<specs-file-name>%s. If everything went well then argv[0] is the
+   inflated (absolute) first device-specs directory and argv[1] is a device
+   or core name as supplied by -mmcu=*. When building GCC the path might be
+   relative.  */
 
 const char*
 avr_devicespecs_file (int argc, const char **argv)
 {
-  char *specfile_name;
   const char *mmcu = NULL;
 
 #ifdef DEBUG_SPECS
@@ -111,12 +88,10 @@ avr_devicespecs_file (int argc, const char **argv)
       break;
     }
 
-  specfile_name = concat (argv[0], dir_separator_str, "specs-", mmcu, NULL);
-
 #ifdef DEBUG_SPECS
   if (verbose_flag)
-    fnotice (stderr, "'%s': mmcu='%s'\n'%s': specfile='%s'\n\n",
-             __FUNCTION__, mmcu, __FUNCTION__, specfile_name);
+    fnotice (stderr, "'%s': mmcu='%s'\n\n",
+             __FUNCTION__, mmcu);
 #endif
 
   // Filter out silly -mmcu= arguments like "foo bar".
@@ -131,26 +106,12 @@ avr_devicespecs_file (int argc, const char **argv)
         return X_NODEVLIB;
       }
 
-  if (/* When building / configuring the compiler we might get a relative path
-         as supplied by "-B.".  Assume that the specs file exists and MCU is
-         a core, not a proper device then, i.e. we have "-mmcu=avr*".  */
-      (0 == strncmp (mmcu, "avr", strlen ("avr"))
-       && specfile_name[0] == '.')
-      /* vanilla */
-      || (IS_ABSOLUTE_PATH (specfile_name)
-          && !access (specfile_name, R_OK)))
-    {
-      return concat ("-specs=device-specs", dir_separator_str, "specs-", mmcu,
-                     // Use '%s' instead of the expanded specfile_name.  This
-                     // is the easiest way to handle pathes containing spaces.
-                     "%s",
+  return concat ("-specs=device-specs", dir_separator_str, "specs-",
+                 mmcu, "%s" 
 #if defined (WITH_AVRLIBC)
-                     " %{mmcu=avr*:" X_NODEVLIB "} %{!mmcu=*:" X_NODEVLIB "}",
+                 " %{mmcu=avr*:" X_NODEVLIB "} %{!mmcu=*:" X_NODEVLIB "}",
 #else
-                     " " X_NODEVLIB,
+                 " " X_NODEVLIB,
 #endif
-                     NULL);
-    }
-
-  return avr_diagnose_devicespecs_error (mmcu, specfile_name);
+                 NULL);
 }

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [patch, avr] Fix mmcu to specs issues
  2016-07-28 11:49   ` Pitchumani Sivanupandi
@ 2016-07-29  8:37     ` Georg-Johann Lay
  2016-07-29 11:43       ` Pitchumani Sivanupandi
  0 siblings, 1 reply; 7+ messages in thread
From: Georg-Johann Lay @ 2016-07-29  8:37 UTC (permalink / raw)
  To: Pitchumani Sivanupandi; +Cc: GCC Patches, Denis Chertykov

On 28.07.2016 13:50, Pitchumani Sivanupandi wrote:
> On Tuesday 26 July 2016 06:00 PM, Georg-Johann Lay wrote:
>> On 26.07.2016 12:20, Pitchumani Sivanupandi wrote:
>>> avr-gcc expected to find the device specs in the search paths specified. But
>>> it doesn't work as expected when device specs in different place than the
>>> installed location.
>>>
>>> Example-1:
>>> avr-gcc wrongly assumes the device specs will be in first identified
>>> 'device-specs' folder in prefix path. avr-gcc natively supports device
>>> at90pwm1, but complains as it couldn't find the specs file in the first
>>> 'device-specs' directory in the search path.
>>>
>>>
>>> $ avr-gcc test.c -mmcu=at90pwm1 -B /home/install/dev/atxmega128a1u/
>>>
>>> avr-gcc: error: cannot access device-specs for _at90pwm1_ expected at
>>
>> Are the "_"s literally? Then the spec file name should be "specs-_at90pwm1_".
> No, It was mangled character shown by my terminal for format specifier "%qs" in
> printf.
> Ignore it.
> ...
>>> Example-2:
>>> Similar issue happens when -flto option is enabled and device specs in custom
>>> search path.
>>>
>>> atxmega128a1u device specs in custom path and that is passed with -B option.
>>> Architecture spec files such as specs-avrxmega7 will be in installed location.
>>>
>>> $ avr-gcc test.c -mmcu=atxmega128a1u -flto -B /home/install/dev/atxmega128a1u/
>>>
>>> avr-gcc: error: cannot access device-specs for _avrxmega7_ expected at
>>> _/home/install/dev/atxmega128a1u/device-specs/specs-avrxmega7_
>>> avr-gcc: note: supported core architectures: avr2 avr25 avr3 avr31 avr35 avr4
>>> avr5 avr51 avr6 avrxmega2 avrxmega4 avrxmega5 avrxmega6 avrxmega7 avrtiny avr1
>>> avr-gcc: note: you can provide your own specs files, see
>>> <http://gcc.gnu.org/onlinedocs/gcc/Spec-Files.html> for details
>>> lto-wrapper: fatal error: avr-gcc returned 1 exit status
>>> compilation terminated.
>>> /home/avr-trunk/install/lib/gcc/avr/7.0.0/../../../../avr/bin/ld: error:
>>> lto-wrapper failed
>>> collect2: error: ld returned 1 exit status
>>>
>>> Attached patch to address these issues.
>>>
>>> Fix:
>>> Replace device-specs-file spec function with mmcu-to-device-specs. This will
>>> not assume that specs files are in first identified device-specs directory.
>>> Instead this spec function will let gcc identify the absolute path of specs
>>> file
>>> using spec string "device-specs-file (device-specs/specs-<mcu>%s)".
>>
>> IIUC this leads to problems with LTO and when the install path contains
>> spaces which windows distros usually have.  The problem is that the spec
>> function cannot escape the spaces as it would need more than 1 escape level.
>>
>> It might also be the case that -mmcu= is specified more than once (with the
>> same MCU), but I don't remember exactly in which situations this happens...
>> Doesn't this lead to inclusion of more than one specs-file?
>
> Yes, it lead to space problem.
>
> Modified the patch because of path with spaces problem. When LTO enabled,
> multiple specs-file are
> included as -mmcu=<arch> is fed back to gcc. It happens with/ without of this
> patch.
> e.g. For atmega164p, device's and architecture's specs file given when invoking
> collect2.
>  -specs=device-specs/specs-atmega164p -specs=device-specs/specs-avr5
>
> Attached new patch. It just removes the conditions that led to originally
> stated issues.
> (In driver-avr.c:avr_devicespecs_file)
> Removed first condition as -mmcu=avr* shall come when LTO enabled. Second
> condition to
> check absolute path is wrong as the specfile_name composed here will not be
> available
> if the specs file is not present in first 'device-specs' directory.
>
> With this approach, we can't issue 'descriptive' error for unavailable specs-file.
> But, avr-gcc will issue below error if specs file is not found.
> $ avr-gcc -mmcu=unknown test.c
> avr-gcc: error: device-specs/specs-unknown: No such file or directory
>
> Is that Ok?

Looks reasonable, just ...


> Regards,
> Pitchumani
>
> gcc/ChangeLog
>
> 2016-07-28  Pitchumani Sivanupandi <pitchumani.s@atmel.com>
>
>     * config/avr/driver-avr.c (specfiles_doc_url): Remove.
>     (avr_diagnose_devicespecs_error): Remove.
>     (avr_devicespecs_file): Remove conditions to check specs-file,
>     let -specs= option handler do the validation.
>  const char*
>  avr_devicespecs_file (int argc, const char **argv)
>  {
> -  char *specfile_name;
>    const char *mmcu = NULL;
>
>  #ifdef DEBUG_SPECS
> @@ -111,12 +88,10 @@ avr_devicespecs_file (int argc, const char **argv)
>        break;
>      }
>
> -  specfile_name = concat (argv[0], dir_separator_str, "specs-", mmcu, NULL);
> -
>  #ifdef DEBUG_SPECS
>    if (verbose_flag)
> -    fnotice (stderr, "'%s': mmcu='%s'\n'%s': specfile='%s'\n\n",
> -             __FUNCTION__, mmcu, __FUNCTION__, specfile_name);
> +    fnotice (stderr, "'%s': mmcu='%s'\n\n",
> +             __FUNCTION__, mmcu);
>  #endif

... this DEBUG_SPECS makes hardly any sense any more because it doesn't print 
any specs-related info.

Johann


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [patch, avr] Fix mmcu to specs issues
  2016-07-29  8:37     ` Georg-Johann Lay
@ 2016-07-29 11:43       ` Pitchumani Sivanupandi
  2016-08-08  6:17         ` Pitchumani Sivanupandi
  0 siblings, 1 reply; 7+ messages in thread
From: Pitchumani Sivanupandi @ 2016-07-29 11:43 UTC (permalink / raw)
  To: Georg-Johann Lay, GCC Patches; +Cc: Denis Chertykov

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

On Friday 29 July 2016 02:06 PM, Georg-Johann Lay wrote:
> On 28.07.2016 13:50, Pitchumani Sivanupandi wrote:
>> On Tuesday 26 July 2016 06:00 PM, Georg-Johann Lay wrote:
>>> On 26.07.2016 12:20, Pitchumani Sivanupandi wrote:
>>>> avr-gcc expected to find the device specs in the search paths 
>>>> specified. But
>>>> it doesn't work as expected when device specs in different place 
>>>> than the
>>>> installed location.
>>>>
>>>> Example-1:
>>>> avr-gcc wrongly assumes the device specs will be in first identified
>>>> 'device-specs' folder in prefix path. avr-gcc natively supports device
>>>> at90pwm1, but complains as it couldn't find the specs file in the 
>>>> first
>>>> 'device-specs' directory in the search path.
>>>>
>>>>
>>>> $ avr-gcc test.c -mmcu=at90pwm1 -B /home/install/dev/atxmega128a1u/
>>>>
>>>> avr-gcc: error: cannot access device-specs for _at90pwm1_ expected at
>>>
>>> Are the "_"s literally? Then the spec file name should be 
>>> "specs-_at90pwm1_".
>> No, It was mangled character shown by my terminal for format 
>> specifier "%qs" in
>> printf.
>> Ignore it.
>> ...
>>>> Example-2:
>>>> Similar issue happens when -flto option is enabled and device specs 
>>>> in custom
>>>> search path.
>>>>
>>>> atxmega128a1u device specs in custom path and that is passed with 
>>>> -B option.
>>>> Architecture spec files such as specs-avrxmega7 will be in 
>>>> installed location.
>>>>
>>>> $ avr-gcc test.c -mmcu=atxmega128a1u -flto -B 
>>>> /home/install/dev/atxmega128a1u/
>>>>
>>>> avr-gcc: error: cannot access device-specs for _avrxmega7_ expected at
>>>> _/home/install/dev/atxmega128a1u/device-specs/specs-avrxmega7_
>>>> avr-gcc: note: supported core architectures: avr2 avr25 avr3 avr31 
>>>> avr35 avr4
>>>> avr5 avr51 avr6 avrxmega2 avrxmega4 avrxmega5 avrxmega6 avrxmega7 
>>>> avrtiny avr1
>>>> avr-gcc: note: you can provide your own specs files, see
>>>> <http://gcc.gnu.org/onlinedocs/gcc/Spec-Files.html> for details
>>>> lto-wrapper: fatal error: avr-gcc returned 1 exit status
>>>> compilation terminated.
>>>> /home/avr-trunk/install/lib/gcc/avr/7.0.0/../../../../avr/bin/ld: 
>>>> error:
>>>> lto-wrapper failed
>>>> collect2: error: ld returned 1 exit status
>>>>
>>>> Attached patch to address these issues.
>>>>
>>>> Fix:
>>>> Replace device-specs-file spec function with mmcu-to-device-specs. 
>>>> This will
>>>> not assume that specs files are in first identified device-specs 
>>>> directory.
>>>> Instead this spec function will let gcc identify the absolute path 
>>>> of specs
>>>> file
>>>> using spec string "device-specs-file (device-specs/specs-<mcu>%s)".
>>>
>>> IIUC this leads to problems with LTO and when the install path contains
>>> spaces which windows distros usually have.  The problem is that the 
>>> spec
>>> function cannot escape the spaces as it would need more than 1 
>>> escape level.
>>>
>>> It might also be the case that -mmcu= is specified more than once 
>>> (with the
>>> same MCU), but I don't remember exactly in which situations this 
>>> happens...
>>> Doesn't this lead to inclusion of more than one specs-file?
>>
>> Yes, it lead to space problem.
>>
>> Modified the patch because of path with spaces problem. When LTO 
>> enabled,
>> multiple specs-file are
>> included as -mmcu=<arch> is fed back to gcc. It happens with/ without 
>> of this
>> patch.
>> e.g. For atmega164p, device's and architecture's specs file given 
>> when invoking
>> collect2.
>>  -specs=device-specs/specs-atmega164p -specs=device-specs/specs-avr5
>>
>> Attached new patch. It just removes the conditions that led to 
>> originally
>> stated issues.
>> (In driver-avr.c:avr_devicespecs_file)
>> Removed first condition as -mmcu=avr* shall come when LTO enabled. 
>> Second
>> condition to
>> check absolute path is wrong as the specfile_name composed here will 
>> not be
>> available
>> if the specs file is not present in first 'device-specs' directory.
>>
>> With this approach, we can't issue 'descriptive' error for 
>> unavailable specs-file.
>> But, avr-gcc will issue below error if specs file is not found.
>> $ avr-gcc -mmcu=unknown test.c
>> avr-gcc: error: device-specs/specs-unknown: No such file or directory
>>
>> Is that Ok?
>
> Looks reasonable, just ...
>
>
>> Regards,
>> Pitchumani
>>
>> gcc/ChangeLog
>>
>> 2016-07-28  Pitchumani Sivanupandi <pitchumani.s@atmel.com>
>>
>>     * config/avr/driver-avr.c (specfiles_doc_url): Remove.
>>     (avr_diagnose_devicespecs_error): Remove.
>>     (avr_devicespecs_file): Remove conditions to check specs-file,
>>     let -specs= option handler do the validation.
>>  const char*
>>  avr_devicespecs_file (int argc, const char **argv)
>>  {
>> -  char *specfile_name;
>>    const char *mmcu = NULL;
>>
>>  #ifdef DEBUG_SPECS
>> @@ -111,12 +88,10 @@ avr_devicespecs_file (int argc, const char **argv)
>>        break;
>>      }
>>
>> -  specfile_name = concat (argv[0], dir_separator_str, "specs-", 
>> mmcu, NULL);
>> -
>>  #ifdef DEBUG_SPECS
>>    if (verbose_flag)
>> -    fnotice (stderr, "'%s': mmcu='%s'\n'%s': specfile='%s'\n\n",
>> -             __FUNCTION__, mmcu, __FUNCTION__, specfile_name);
>> +    fnotice (stderr, "'%s': mmcu='%s'\n\n",
>> +             __FUNCTION__, mmcu);
>>  #endif
>
> ... this DEBUG_SPECS makes hardly any sense any more because it 
> doesn't print any specs-related info.

Thanks for the review. Updated the patch.

If Ok, could someone commit please?

Regards,
Pitchumani


gcc/ChangeLog

2016-07-29  Pitchumani Sivanupandi <pitchumani.s@atmel.com>

     * config/avr/driver-avr.c (specfiles_doc_url): Remove.
     (avr_diagnose_devicespecs_error): Remove.
     (avr_devicespecs_file): Remove composing absolute path for specfile
     and its verbose info. Remove conditions to check specs-file,
     let -specs= option handler do the validation.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: avr-mmcu-to-specs-rev3.patch --]
[-- Type: text/x-patch; name="avr-mmcu-to-specs-rev3.patch", Size: 3437 bytes --]

diff --git a/gcc/config/avr/driver-avr.c b/gcc/config/avr/driver-avr.c
index 83ca373..8a8fd50 100644
--- a/gcc/config/avr/driver-avr.c
+++ b/gcc/config/avr/driver-avr.c
@@ -29,41 +29,18 @@ along with GCC; see the file COPYING3.  If not see
 
 static const char dir_separator_str[] = { DIR_SEPARATOR, 0 };
 
-static const char specfiles_doc_url[] =
-  "http://gcc.gnu.org/onlinedocs/gcc/Spec-Files.html";
-
-
-static const char*
-avr_diagnose_devicespecs_error (const char *mcu, const char *filename)
-{
-  error ("cannot access device-specs for %qs expected at %qs",
-         mcu, filename);
-
-  // Inform about natively supported devices and cores.
-
-  if (strncmp (mcu, "avr", strlen ("avr")))
-    avr_inform_devices ();
-
-  avr_inform_core_architectures ();
-
-  inform (input_location, "you can provide your own specs files, "
-          "see <%s> for details", specfiles_doc_url);
-
-  return X_NODEVLIB;
-}
-
 
 /* Implement spec function `device-specs-file´.
 
-   Compose -specs=<specs-file-name>%s.  If everything went well then argv[0]
-   is the inflated (absolute) specs directory and argv[1] is a device or
-   core name as supplied by -mmcu=*.  When building GCC the path might
-   be relative.  */
+   Validate mcu name given with -mmcu option. Compose
+   -specs=<specs-file-name>%s. If everything went well then argv[0] is the
+   inflated (absolute) first device-specs directory and argv[1] is a device
+   or core name as supplied by -mmcu=*. When building GCC the path might be
+   relative.  */
 
 const char*
 avr_devicespecs_file (int argc, const char **argv)
 {
-  char *specfile_name;
   const char *mmcu = NULL;
 
 #ifdef DEBUG_SPECS
@@ -111,14 +88,6 @@ avr_devicespecs_file (int argc, const char **argv)
       break;
     }
 
-  specfile_name = concat (argv[0], dir_separator_str, "specs-", mmcu, NULL);
-
-#ifdef DEBUG_SPECS
-  if (verbose_flag)
-    fnotice (stderr, "'%s': mmcu='%s'\n'%s': specfile='%s'\n\n",
-             __FUNCTION__, mmcu, __FUNCTION__, specfile_name);
-#endif
-
   // Filter out silly -mmcu= arguments like "foo bar".
 
   for (const char *s = mmcu; *s; s++)
@@ -131,26 +100,12 @@ avr_devicespecs_file (int argc, const char **argv)
         return X_NODEVLIB;
       }
 
-  if (/* When building / configuring the compiler we might get a relative path
-         as supplied by "-B.".  Assume that the specs file exists and MCU is
-         a core, not a proper device then, i.e. we have "-mmcu=avr*".  */
-      (0 == strncmp (mmcu, "avr", strlen ("avr"))
-       && specfile_name[0] == '.')
-      /* vanilla */
-      || (IS_ABSOLUTE_PATH (specfile_name)
-          && !access (specfile_name, R_OK)))
-    {
-      return concat ("-specs=device-specs", dir_separator_str, "specs-", mmcu,
-                     // Use '%s' instead of the expanded specfile_name.  This
-                     // is the easiest way to handle pathes containing spaces.
-                     "%s",
+  return concat ("-specs=device-specs", dir_separator_str, "specs-",
+                 mmcu, "%s"
 #if defined (WITH_AVRLIBC)
-                     " %{mmcu=avr*:" X_NODEVLIB "} %{!mmcu=*:" X_NODEVLIB "}",
+                 " %{mmcu=avr*:" X_NODEVLIB "} %{!mmcu=*:" X_NODEVLIB "}",
 #else
-                     " " X_NODEVLIB,
+                 " " X_NODEVLIB,
 #endif
-                     NULL);
-    }
-
-  return avr_diagnose_devicespecs_error (mmcu, specfile_name);
+                 NULL);
 }

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [patch, avr] Fix mmcu to specs issues
  2016-07-29 11:43       ` Pitchumani Sivanupandi
@ 2016-08-08  6:17         ` Pitchumani Sivanupandi
  2016-08-08 17:38           ` Denis Chertykov
  0 siblings, 1 reply; 7+ messages in thread
From: Pitchumani Sivanupandi @ 2016-08-08  6:17 UTC (permalink / raw)
  To: Pitchumani Sivanupandi, Georg-Johann Lay, GCC Patches; +Cc: Denis Chertykov

Ping!

On Friday 29 July 2016 05:14 PM, Pitchumani Sivanupandi wrote:
> On Friday 29 July 2016 02:06 PM, Georg-Johann Lay wrote:
>> On 28.07.2016 13:50, Pitchumani Sivanupandi wrote:
>>> On Tuesday 26 July 2016 06:00 PM, Georg-Johann Lay wrote:
>>>> On 26.07.2016 12:20, Pitchumani Sivanupandi wrote:
>>>>> avr-gcc expected to find the device specs in the search paths 
>>>>> specified. But
>>>>> it doesn't work as expected when device specs in different place 
>>>>> than the
>>>>> installed location.
>>>>>
>>>>> Example-1:
>>>>> avr-gcc wrongly assumes the device specs will be in first identified
>>>>> 'device-specs' folder in prefix path. avr-gcc natively supports 
>>>>> device
>>>>> at90pwm1, but complains as it couldn't find the specs file in the 
>>>>> first
>>>>> 'device-specs' directory in the search path.
>>>>>
>>>>>
>>>>> $ avr-gcc test.c -mmcu=at90pwm1 -B /home/install/dev/atxmega128a1u/
>>>>>
>>>>> avr-gcc: error: cannot access device-specs for _at90pwm1_ expected at
>>>>
>>>> Are the "_"s literally? Then the spec file name should be 
>>>> "specs-_at90pwm1_".
>>> No, It was mangled character shown by my terminal for format 
>>> specifier "%qs" in
>>> printf.
>>> Ignore it.
>>> ...
>>>>> Example-2:
>>>>> Similar issue happens when -flto option is enabled and device 
>>>>> specs in custom
>>>>> search path.
>>>>>
>>>>> atxmega128a1u device specs in custom path and that is passed with 
>>>>> -B option.
>>>>> Architecture spec files such as specs-avrxmega7 will be in 
>>>>> installed location.
>>>>>
>>>>> $ avr-gcc test.c -mmcu=atxmega128a1u -flto -B 
>>>>> /home/install/dev/atxmega128a1u/
>>>>>
>>>>> avr-gcc: error: cannot access device-specs for _avrxmega7_ 
>>>>> expected at
>>>>> _/home/install/dev/atxmega128a1u/device-specs/specs-avrxmega7_
>>>>> avr-gcc: note: supported core architectures: avr2 avr25 avr3 avr31 
>>>>> avr35 avr4
>>>>> avr5 avr51 avr6 avrxmega2 avrxmega4 avrxmega5 avrxmega6 avrxmega7 
>>>>> avrtiny avr1
>>>>> avr-gcc: note: you can provide your own specs files, see
>>>>> <http://gcc.gnu.org/onlinedocs/gcc/Spec-Files.html> for details
>>>>> lto-wrapper: fatal error: avr-gcc returned 1 exit status
>>>>> compilation terminated.
>>>>> /home/avr-trunk/install/lib/gcc/avr/7.0.0/../../../../avr/bin/ld: 
>>>>> error:
>>>>> lto-wrapper failed
>>>>> collect2: error: ld returned 1 exit status
>>>>>
>>>>> Attached patch to address these issues.
>>>>>
>>>>> Fix:
>>>>> Replace device-specs-file spec function with mmcu-to-device-specs. 
>>>>> This will
>>>>> not assume that specs files are in first identified device-specs 
>>>>> directory.
>>>>> Instead this spec function will let gcc identify the absolute path 
>>>>> of specs
>>>>> file
>>>>> using spec string "device-specs-file (device-specs/specs-<mcu>%s)".
>>>>
>>>> IIUC this leads to problems with LTO and when the install path 
>>>> contains
>>>> spaces which windows distros usually have.  The problem is that the 
>>>> spec
>>>> function cannot escape the spaces as it would need more than 1 
>>>> escape level.
>>>>
>>>> It might also be the case that -mmcu= is specified more than once 
>>>> (with the
>>>> same MCU), but I don't remember exactly in which situations this 
>>>> happens...
>>>> Doesn't this lead to inclusion of more than one specs-file?
>>>
>>> Yes, it lead to space problem.
>>>
>>> Modified the patch because of path with spaces problem. When LTO 
>>> enabled,
>>> multiple specs-file are
>>> included as -mmcu=<arch> is fed back to gcc. It happens with/ 
>>> without of this
>>> patch.
>>> e.g. For atmega164p, device's and architecture's specs file given 
>>> when invoking
>>> collect2.
>>>  -specs=device-specs/specs-atmega164p -specs=device-specs/specs-avr5
>>>
>>> Attached new patch. It just removes the conditions that led to 
>>> originally
>>> stated issues.
>>> (In driver-avr.c:avr_devicespecs_file)
>>> Removed first condition as -mmcu=avr* shall come when LTO enabled. 
>>> Second
>>> condition to
>>> check absolute path is wrong as the specfile_name composed here will 
>>> not be
>>> available
>>> if the specs file is not present in first 'device-specs' directory.
>>>
>>> With this approach, we can't issue 'descriptive' error for 
>>> unavailable specs-file.
>>> But, avr-gcc will issue below error if specs file is not found.
>>> $ avr-gcc -mmcu=unknown test.c
>>> avr-gcc: error: device-specs/specs-unknown: No such file or directory
>>>
>>> Is that Ok?
>>
>> Looks reasonable, just ...
>>
>>
>>> Regards,
>>> Pitchumani
>>>
>>> gcc/ChangeLog
>>>
>>> 2016-07-28  Pitchumani Sivanupandi <pitchumani.s@atmel.com>
>>>
>>>     * config/avr/driver-avr.c (specfiles_doc_url): Remove.
>>>     (avr_diagnose_devicespecs_error): Remove.
>>>     (avr_devicespecs_file): Remove conditions to check specs-file,
>>>     let -specs= option handler do the validation.
>>>  const char*
>>>  avr_devicespecs_file (int argc, const char **argv)
>>>  {
>>> -  char *specfile_name;
>>>    const char *mmcu = NULL;
>>>
>>>  #ifdef DEBUG_SPECS
>>> @@ -111,12 +88,10 @@ avr_devicespecs_file (int argc, const char **argv)
>>>        break;
>>>      }
>>>
>>> -  specfile_name = concat (argv[0], dir_separator_str, "specs-", 
>>> mmcu, NULL);
>>> -
>>>  #ifdef DEBUG_SPECS
>>>    if (verbose_flag)
>>> -    fnotice (stderr, "'%s': mmcu='%s'\n'%s': specfile='%s'\n\n",
>>> -             __FUNCTION__, mmcu, __FUNCTION__, specfile_name);
>>> +    fnotice (stderr, "'%s': mmcu='%s'\n\n",
>>> +             __FUNCTION__, mmcu);
>>>  #endif
>>
>> ... this DEBUG_SPECS makes hardly any sense any more because it 
>> doesn't print any specs-related info.
>
> Thanks for the review. Updated the patch.
>
> If Ok, could someone commit please?
>
> Regards,
> Pitchumani
>
>
> gcc/ChangeLog
>
> 2016-07-29  Pitchumani Sivanupandi <pitchumani.s@atmel.com>
>
>     * config/avr/driver-avr.c (specfiles_doc_url): Remove.
>     (avr_diagnose_devicespecs_error): Remove.
>     (avr_devicespecs_file): Remove composing absolute path for specfile
>     and its verbose info. Remove conditions to check specs-file,
>     let -specs= option handler do the validation.
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [patch, avr] Fix mmcu to specs issues
  2016-08-08  6:17         ` Pitchumani Sivanupandi
@ 2016-08-08 17:38           ` Denis Chertykov
  0 siblings, 0 replies; 7+ messages in thread
From: Denis Chertykov @ 2016-08-08 17:38 UTC (permalink / raw)
  To: Pitchumani Sivanupandi
  Cc: Pitchumani Sivanupandi, Georg-Johann Lay, GCC Patches

Sorry for delay. Committed.

2016-08-08 9:17 GMT+03:00 Pitchumani Sivanupandi
<pitchumani.sivanupandi@microchip.com>:
> Ping!
>
>
> On Friday 29 July 2016 05:14 PM, Pitchumani Sivanupandi wrote:
>>
>> On Friday 29 July 2016 02:06 PM, Georg-Johann Lay wrote:
>>>
>>> On 28.07.2016 13:50, Pitchumani Sivanupandi wrote:
>>>>
>>>> On Tuesday 26 July 2016 06:00 PM, Georg-Johann Lay wrote:
>>>>>
>>>>> On 26.07.2016 12:20, Pitchumani Sivanupandi wrote:
>>>>>>
>>>>>> avr-gcc expected to find the device specs in the search paths
>>>>>> specified. But
>>>>>> it doesn't work as expected when device specs in different place than
>>>>>> the
>>>>>> installed location.
>>>>>>
>>>>>> Example-1:
>>>>>> avr-gcc wrongly assumes the device specs will be in first identified
>>>>>> 'device-specs' folder in prefix path. avr-gcc natively supports device
>>>>>> at90pwm1, but complains as it couldn't find the specs file in the
>>>>>> first
>>>>>> 'device-specs' directory in the search path.
>>>>>>
>>>>>>
>>>>>> $ avr-gcc test.c -mmcu=at90pwm1 -B /home/install/dev/atxmega128a1u/
>>>>>>
>>>>>> avr-gcc: error: cannot access device-specs for _at90pwm1_ expected at
>>>>>
>>>>>
>>>>> Are the "_"s literally? Then the spec file name should be
>>>>> "specs-_at90pwm1_".
>>>>
>>>> No, It was mangled character shown by my terminal for format specifier
>>>> "%qs" in
>>>> printf.
>>>> Ignore it.
>>>> ...
>>>>>>
>>>>>> Example-2:
>>>>>> Similar issue happens when -flto option is enabled and device specs in
>>>>>> custom
>>>>>> search path.
>>>>>>
>>>>>> atxmega128a1u device specs in custom path and that is passed with -B
>>>>>> option.
>>>>>> Architecture spec files such as specs-avrxmega7 will be in installed
>>>>>> location.
>>>>>>
>>>>>> $ avr-gcc test.c -mmcu=atxmega128a1u -flto -B
>>>>>> /home/install/dev/atxmega128a1u/
>>>>>>
>>>>>> avr-gcc: error: cannot access device-specs for _avrxmega7_ expected at
>>>>>> _/home/install/dev/atxmega128a1u/device-specs/specs-avrxmega7_
>>>>>> avr-gcc: note: supported core architectures: avr2 avr25 avr3 avr31
>>>>>> avr35 avr4
>>>>>> avr5 avr51 avr6 avrxmega2 avrxmega4 avrxmega5 avrxmega6 avrxmega7
>>>>>> avrtiny avr1
>>>>>> avr-gcc: note: you can provide your own specs files, see
>>>>>> <http://gcc.gnu.org/onlinedocs/gcc/Spec-Files.html> for details
>>>>>> lto-wrapper: fatal error: avr-gcc returned 1 exit status
>>>>>> compilation terminated.
>>>>>> /home/avr-trunk/install/lib/gcc/avr/7.0.0/../../../../avr/bin/ld:
>>>>>> error:
>>>>>> lto-wrapper failed
>>>>>> collect2: error: ld returned 1 exit status
>>>>>>
>>>>>> Attached patch to address these issues.
>>>>>>
>>>>>> Fix:
>>>>>> Replace device-specs-file spec function with mmcu-to-device-specs.
>>>>>> This will
>>>>>> not assume that specs files are in first identified device-specs
>>>>>> directory.
>>>>>> Instead this spec function will let gcc identify the absolute path of
>>>>>> specs
>>>>>> file
>>>>>> using spec string "device-specs-file (device-specs/specs-<mcu>%s)".
>>>>>
>>>>>
>>>>> IIUC this leads to problems with LTO and when the install path contains
>>>>> spaces which windows distros usually have.  The problem is that the
>>>>> spec
>>>>> function cannot escape the spaces as it would need more than 1 escape
>>>>> level.
>>>>>
>>>>> It might also be the case that -mmcu= is specified more than once (with
>>>>> the
>>>>> same MCU), but I don't remember exactly in which situations this
>>>>> happens...
>>>>> Doesn't this lead to inclusion of more than one specs-file?
>>>>
>>>>
>>>> Yes, it lead to space problem.
>>>>
>>>> Modified the patch because of path with spaces problem. When LTO
>>>> enabled,
>>>> multiple specs-file are
>>>> included as -mmcu=<arch> is fed back to gcc. It happens with/ without of
>>>> this
>>>> patch.
>>>> e.g. For atmega164p, device's and architecture's specs file given when
>>>> invoking
>>>> collect2.
>>>>  -specs=device-specs/specs-atmega164p -specs=device-specs/specs-avr5
>>>>
>>>> Attached new patch. It just removes the conditions that led to
>>>> originally
>>>> stated issues.
>>>> (In driver-avr.c:avr_devicespecs_file)
>>>> Removed first condition as -mmcu=avr* shall come when LTO enabled.
>>>> Second
>>>> condition to
>>>> check absolute path is wrong as the specfile_name composed here will not
>>>> be
>>>> available
>>>> if the specs file is not present in first 'device-specs' directory.
>>>>
>>>> With this approach, we can't issue 'descriptive' error for unavailable
>>>> specs-file.
>>>> But, avr-gcc will issue below error if specs file is not found.
>>>> $ avr-gcc -mmcu=unknown test.c
>>>> avr-gcc: error: device-specs/specs-unknown: No such file or directory
>>>>
>>>> Is that Ok?
>>>
>>>
>>> Looks reasonable, just ...
>>>
>>>
>>>> Regards,
>>>> Pitchumani
>>>>
>>>> gcc/ChangeLog
>>>>
>>>> 2016-07-28  Pitchumani Sivanupandi <pitchumani.s@atmel.com>
>>>>
>>>>     * config/avr/driver-avr.c (specfiles_doc_url): Remove.
>>>>     (avr_diagnose_devicespecs_error): Remove.
>>>>     (avr_devicespecs_file): Remove conditions to check specs-file,
>>>>     let -specs= option handler do the validation.
>>>>  const char*
>>>>  avr_devicespecs_file (int argc, const char **argv)
>>>>  {
>>>> -  char *specfile_name;
>>>>    const char *mmcu = NULL;
>>>>
>>>>  #ifdef DEBUG_SPECS
>>>> @@ -111,12 +88,10 @@ avr_devicespecs_file (int argc, const char **argv)
>>>>        break;
>>>>      }
>>>>
>>>> -  specfile_name = concat (argv[0], dir_separator_str, "specs-", mmcu,
>>>> NULL);
>>>> -
>>>>  #ifdef DEBUG_SPECS
>>>>    if (verbose_flag)
>>>> -    fnotice (stderr, "'%s': mmcu='%s'\n'%s': specfile='%s'\n\n",
>>>> -             __FUNCTION__, mmcu, __FUNCTION__, specfile_name);
>>>> +    fnotice (stderr, "'%s': mmcu='%s'\n\n",
>>>> +             __FUNCTION__, mmcu);
>>>>  #endif
>>>
>>>
>>> ... this DEBUG_SPECS makes hardly any sense any more because it doesn't
>>> print any specs-related info.
>>
>>
>> Thanks for the review. Updated the patch.
>>
>> If Ok, could someone commit please?
>>
>> Regards,
>> Pitchumani
>>
>>
>> gcc/ChangeLog
>>
>> 2016-07-29  Pitchumani Sivanupandi <pitchumani.s@atmel.com>
>>
>>     * config/avr/driver-avr.c (specfiles_doc_url): Remove.
>>     (avr_diagnose_devicespecs_error): Remove.
>>     (avr_devicespecs_file): Remove composing absolute path for specfile
>>     and its verbose info. Remove conditions to check specs-file,
>>     let -specs= option handler do the validation.
>>
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2016-08-08 17:38 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-26 10:20 [patch, avr] Fix mmcu to specs issues Pitchumani Sivanupandi
2016-07-26 12:30 ` Georg-Johann Lay
2016-07-28 11:49   ` Pitchumani Sivanupandi
2016-07-29  8:37     ` Georg-Johann Lay
2016-07-29 11:43       ` Pitchumani Sivanupandi
2016-08-08  6:17         ` Pitchumani Sivanupandi
2016-08-08 17:38           ` Denis Chertykov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).