[Patch, testsuite] Fix sra-13.c for 16 bit int
Fix test case sra-13.c that assumed int is always 4 bytes. Regards, Pitchumani 2013-04-01 Pitchumani Sivanupandi pitchuman...@atmel.com testsuite * gcc.dg/tree-ssa/sra-13.c: Fix for 16 bit int --- gcc/testsuite/gcc.dg/tree-ssa/sra-13.c (revision 197081) +++ gcc/testsuite/gcc.dg/tree-ssa/sra-13.c (working copy) @@ -95,7 +95,7 @@ b = 0; gu1.b.l = 2000; s = bar (); - if (s != 2000) + if (s != (int)2000) __builtin_abort (); if (gu2.b.l != 2000) __builtin_abort ();
Re: [Patch, testsuite] Fix sra-13.c for 16 bit int
could someone please review and commit if it is OK? I don't have commit access. Regards, Pitchumani On 4/1/2013 4:30 PM, Pitchumani Sivanupandi wrote: Fix test case sra-13.c that assumed int is always 4 bytes. Regards, Pitchumani 2013-04-01 Pitchumani Sivanupandi pitchuman...@atmel.com testsuite * gcc.dg/tree-ssa/sra-13.c: Fix for 16 bit int --- gcc/testsuite/gcc.dg/tree-ssa/sra-13.c (revision 197081) +++ gcc/testsuite/gcc.dg/tree-ssa/sra-13.c (working copy) @@ -95,7 +95,7 @@ b = 0; gu1.b.l = 2000; s = bar (); - if (s != 2000) + if (s != (int)2000) __builtin_abort (); if (gu2.b.l != 2000) __builtin_abort ();
[patch, avr] Fix PR67353
Hi,
This patch introduces new flags for warning 'misspelled interrupt/
signal handler'. Flag -Wmisspelled-isr is enabled by default and it
will warn user if the interrupt/ signal handler is without '__vector'
prefix. Flag -Wno-misspelled-isr shall be enabled by user to allow
custom names, i.e. without __vector prefix.
// avr-gcc -c test.c
void custom_interruption(void) __attribute__((signal));
void custom_interruption(void) {}
Behavior after applying this patch:
$ avr-gcc test.c
test.c: In function 'custom_interruption':
test.c:2:6: warning: 'custom_interruption' appears to be a misspelled
signal handler
void custom_interruption(void) {}
^~~
$ avr-gcc test.c -Wmisspelled-isr
test.c: In function
'custom_interruption':
test.c:2:6: warning: 'custom_interruption'
appears to be a misspelled signal handler
void
custom_interruption(void) {}
^~~
$ avr-gcc test.c -Wno-misspelled-isr
$
If OK, could someone commit please? I do not have commit access.
Regards,
Pitchumani
gcc/ChangeLog
2016-06-10 Pitchumani Sivanupandi <pitchuman...@atmel.com>
* config/avr/avr.c (avr_set_current_function): Warn misspelled
interrupt/ signal handler if warn_misspelled_isr flag is set.
* config/avr/avr.opt (Wmisspelled-isr): New warning flag. Enabled
by default to warn misspelled interrupt/ signal handler.
diff --git a/gcc/config/avr/avr.c b/gcc/config/avr/avr.c
index ba5cd91..587bdbc 100644
--- a/gcc/config/avr/avr.c
+++ b/gcc/config/avr/avr.c
@@ -753,7 +753,7 @@ avr_set_current_function (tree decl)
that the name of the function is "__vector_NN" so as to catch
when the user misspells the vector name. */
- if (!STR_PREFIX_P (name, "__vector"))
+ if ((!STR_PREFIX_P (name, "__vector")) && (avr_warn_misspelled_isr))
warning_at (loc, 0, "%qs appears to be a misspelled %s handler",
name, isr);
}
diff --git a/gcc/config/avr/avr.opt b/gcc/config/avr/avr.opt
index 8809b9b..0703f5a 100644
--- a/gcc/config/avr/avr.opt
+++ b/gcc/config/avr/avr.opt
@@ -91,6 +91,10 @@ Waddr-space-convert
Warning C Report Var(avr_warn_addr_space_convert) Init(0)
Warn if the address space of an address is changed.
+Wmisspelled-isr
+Target Warning Report Var(avr_warn_misspelled_isr) Init(1)
+Warn if the ISR is misspelled, i.e. without __vector prefix. Enabled by default.
+
mfract-convert-truncate
Target Report Mask(FRACT_CONV_TRUNC)
Allow to use truncation instead of rounding towards 0 for fractional int types.
Re: [patch, avr] Fix PR67353
On Fri, 2016-06-10 at 20:08 +0200, Georg-Johann Lay wrote:
> Pitchumani Sivanupandi schrieb:
> >
> > Hi,
> >
> > This patch introduces new flags for warning 'misspelled interrupt/
> > signal handler'. Flag -Wmisspelled-isr is enabled by default and it
> > will warn user if the interrupt/ signal handler is without
> > '__vector'
> > prefix. Flag -Wno-misspelled-isr shall be enabled by user to allow
> > custom names, i.e. without __vector prefix.
> >
> > // avr-gcc -c test.c
> > void custom_interruption(void) __attribute__((signal));
> > void custom_interruption(void) {}
> >
> > Behavior after applying this patch:
> >
> > $ avr-gcc test.c
> > test.c: In function 'custom_interruption':
> > test.c:2:6: warning: 'custom_interruption' appears to be a
> > misspelled
> > signal handler
> > void custom_interruption(void) {}
> > ^~~
> >
> > $ avr-gcc test.c -Wmisspelled-isr
> > test.c: In function
> > 'custom_interruption':
> > test.c:2:6: warning: 'custom_interruption'
> > appears to be a misspelled signal handler
> > void
> > custom_interruption(void) {}
> > ^~~
> >
> > $ avr-gcc test.c -Wno-misspelled-isr
> > $
> >
> > If OK, could someone commit please? I do not have commit access.
> >
> > Regards,
> > Pitchumani
> >
> > gcc/ChangeLog
> >
> > 2016-06-10 Pitchumani Sivanupandi <pitchuman...@atmel.com>
> >
> Missing PR target/67353
> >
> > * config/avr/avr.c (avr_set_current_function): Warn misspelled
> > interrupt/ signal handler if warn_misspelled_isr flag is set.
> > * config/avr/avr.opt (Wmisspelled-isr): New warning flag.
> > Enabled
> > by default to warn misspelled interrupt/ signal handler.
> Shouldn't it also be documented in doc/invoke.texi?
Thanks Johann.
Updated the patch. Updated description for -nodevicelib option
as well, device library should be lib.a.
Regards,
Pitchumani
gcc/ChangeLog
2016-06-10 Pitchumani Sivanupandi <pitchuman...@atmel.com>
PR target/67353
* config/avr/avr.c (avr_set_current_function): Warn misspelled
interrupt/ signal handler if warn_misspelled_isr flag is set.
* config/avr/avr.opt (Wmisspelled-isr): New warning flag. Enabled
by default to warn misspelled interrupt/ signal handler.
* doc/invoke.texi (AVR Options): Document it. Update description
for -nodevicelib option.diff --git a/gcc/config/avr/avr.c b/gcc/config/avr/avr.c
index ba5cd91..587bdbc 100644
--- a/gcc/config/avr/avr.c
+++ b/gcc/config/avr/avr.c
@@ -753,7 +753,7 @@ avr_set_current_function (tree decl)
that the name of the function is "__vector_NN" so as to catch
when the user misspells the vector name. */
- if (!STR_PREFIX_P (name, "__vector"))
+ if ((!STR_PREFIX_P (name, "__vector")) && (avr_warn_misspelled_isr))
warning_at (loc, 0, "%qs appears to be a misspelled %s handler",
name, isr);
}
diff --git a/gcc/config/avr/avr.opt b/gcc/config/avr/avr.opt
index 8809b9b..0703f5a 100644
--- a/gcc/config/avr/avr.opt
+++ b/gcc/config/avr/avr.opt
@@ -91,6 +91,10 @@ Waddr-space-convert
Warning C Report Var(avr_warn_addr_space_convert) Init(0)
Warn if the address space of an address is changed.
+Wmisspelled-isr
+Target Warning Report Var(avr_warn_misspelled_isr) Init(1)
+Warn if the ISR is misspelled, i.e. without __vector prefix. Enabled by default.
+
mfract-convert-truncate
Target Report Mask(FRACT_CONV_TRUNC)
Allow to use truncation instead of rounding towards 0 for fractional int types.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index aa11209..0bf39c5 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -640,7 +640,8 @@ Objective-C and Objective-C++ Dialects}.
@emph{AVR Options}
@gccoptlist{-mmcu=@var{mcu} -maccumulate-args -mbranch-cost=@var{cost} @gol
-mcall-prologues -mint8 -mn_flash=@var{size} -mno-interrupts @gol
--mrelax -mrmw -mstrict-X -mtiny-stack -nodevicelib -Waddr-space-convert}
+-mrelax -mrmw -mstrict-X -mtiny-stack -nodevicelib -Waddr-space-convert @gol
+-Wmisspelled-isr}
@emph{Blackfin Options}
@gccoptlist{-mcpu=@var{cpu}@r{[}-@var{sirevision}@r{]} @gol
@@ -14554,12 +14555,17 @@ Only change the lower 8@tie{}bits of the stack pointer.
@item -nodevicelib
@opindex nodevicelib
-Don't link against AVR-LibC's device specific library @code{libdev.a}.
+Don't link against AVR-LibC's device specific library @code{lib.a}.
@item -Waddr-space-convert
@opindex Waddr-space-convert
Warn about conversions between address spaces in the case where the
resulting address space is not contained in the incoming address space.
+
+@item -Wmisspelled-isr
+@opindex Wmisspelled-isr
+Warn if the ISR is misspelled, i.e. without __vector prefix.
+Enabled by default.
@end table
@subsubsection @code{EIND} and Devices with More Than 128 Ki Bytes of Flash
Re: [patch, avr] Fix PR67353
On Mon, 2016-06-13 at 17:48 +0200, Georg-Johann Lay wrote:
> Pitchumani Sivanupandi schrieb:
> >
> > $ avr-gcc test.c -Wno-misspelled-isr
> > $
> What about -Werror=misspelled-isr?
Updated patch.
> >
> > diff --git a/gcc/config/avr/avr.c b/gcc/config/avr/avr.c
> > index ba5cd91..587bdbc 100644
> > --- a/gcc/config/avr/avr.c
> > +++ b/gcc/config/avr/avr.c
> > @@ -753,7 +753,7 @@ avr_set_current_function (tree decl)
> > that the name of the function is "__vector_NN" so as to
> > catch
> > when the user misspells the vector name. */
> >
> > - if (!STR_PREFIX_P (name, "__vector"))
> > + if ((!STR_PREFIX_P (name, "__vector")) &&
> > (avr_warn_misspelled_isr))
> > warning_at (loc, 0, "%qs appears to be a misspelled %s
> > handler",
> If, instead of the "0" the respective OPT_... enum is used in the
> call
> to warning_at, the -Werror= should work as expected (and explicit
> "&&
> avr_warn_misspelled_isr" no more needed).
Ok. Updated patch as per the comments.
If OK, could someone commit please?
Regards,
Pitchumani
diff --git a/gcc/config/avr/avr.c b/gcc/config/avr/avr.c
index ba5cd91..b327624 100644
--- a/gcc/config/avr/avr.c
+++ b/gcc/config/avr/avr.c
@@ -754,8 +754,8 @@ avr_set_current_function (tree decl)
when the user misspells the vector name. */
if (!STR_PREFIX_P (name, "__vector"))
-warning_at (loc, 0, "%qs appears to be a misspelled %s handler",
-name, isr);
+warning_at (loc, OPT_Wmisspelled_isr, "%qs appears to be a misspelled "
+ "%s handler, missing __vector prefix", name, isr);
}
/* Don't print the above diagnostics more than once. */
diff --git a/gcc/config/avr/avr.opt b/gcc/config/avr/avr.opt
index 8809b9b..05aa4b6 100644
--- a/gcc/config/avr/avr.opt
+++ b/gcc/config/avr/avr.opt
@@ -91,6 +91,10 @@ Waddr-space-convert
Warning C Report Var(avr_warn_addr_space_convert) Init(0)
Warn if the address space of an address is changed.
+Wmisspelled-isr
+Warning C C++ Report Var(avr_warn_misspelled_isr) Init(1)
+Warn if the ISR is misspelled, i.e. without __vector prefix. Enabled by default.
+
mfract-convert-truncate
Target Report Mask(FRACT_CONV_TRUNC)
Allow to use truncation instead of rounding towards 0 for fractional int types.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index aa11209..0bf39c5 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -640,7 +640,8 @@ Objective-C and Objective-C++ Dialects}.
@emph{AVR Options}
@gccoptlist{-mmcu=@var{mcu} -maccumulate-args -mbranch-cost=@var{cost} @gol
-mcall-prologues -mint8 -mn_flash=@var{size} -mno-interrupts @gol
--mrelax -mrmw -mstrict-X -mtiny-stack -nodevicelib -Waddr-space-convert}
+-mrelax -mrmw -mstrict-X -mtiny-stack -nodevicelib -Waddr-space-convert @gol
+-Wmisspelled-isr}
@emph{Blackfin Options}
@gccoptlist{-mcpu=@var{cpu}@r{[}-@var{sirevision}@r{]} @gol
@@ -14554,12 +14555,17 @@ Only change the lower 8@tie{}bits of the stack pointer.
@item -nodevicelib
@opindex nodevicelib
-Don't link against AVR-LibC's device specific library @code{libdev.a}.
+Don't link against AVR-LibC's device specific library @code{lib.a}.
@item -Waddr-space-convert
@opindex Waddr-space-convert
Warn about conversions between address spaces in the case where the
resulting address space is not contained in the incoming address space.
+
+@item -Wmisspelled-isr
+@opindex Wmisspelled-isr
+Warn if the ISR is misspelled, i.e. without __vector prefix.
+Enabled by default.
@end table
@subsubsection @code{EIND} and Devices with More Than 128 Ki Bytes of Flash
Re: [patch, avr] Fix PR67353
On Mon, 2016-06-13 at 17:48 +0200, Georg-Johann Lay wrote:
> Pitchumani Sivanupandi schrieb:
> >
> > $ avr-gcc test.c -Wno-misspelled-isr
> > $
> What about -Werror=misspelled-isr?
Updated patch.
> >
> > diff --git a/gcc/config/avr/avr.c b/gcc/config/avr/avr.c
> > index ba5cd91..587bdbc 100644
> > --- a/gcc/config/avr/avr.c
> > +++ b/gcc/config/avr/avr.c
> > @@ -753,7 +753,7 @@ avr_set_current_function (tree decl)
> > that the name of the function is "__vector_NN" so as to
> > catch
> > when the user misspells the vector name. */
> >
> > - if (!STR_PREFIX_P (name, "__vector"))
> > + if ((!STR_PREFIX_P (name, "__vector")) &&
> > (avr_warn_misspelled_isr))
> > warning_at (loc, 0, "%qs appears to be a misspelled %s
> > handler",
> If, instead of the "0" the respective OPT_... enum is used in the
> call
> to warning_at, the -Werror= should work as expected (and explicit
> "&&
> avr_warn_misspelled_isr" no more needed).
Ok. Updated patch as per the comments.
If OK, could someone commit please?
Regards,
Pitchumani
gcc/ChangeLog
2016-06-15 Pitchumani Sivanupandi <pitchuman...@atmel.com>
PR target/67353
* config/avr/avr.c (avr_set_current_function): Warn misspelled
interrupt/ signal handler if -Wmisspelled-isr flag is enabled.
* config/avr/avr.opt (Wmisspelled-isr): New warning flag. Enabled
by default to warn misspelled interrupt/ signal handler.
* doc/invoke.texi (AVR Options): Document it. Update description
for -nodevicelib option.
diff --git a/gcc/config/avr/avr.c b/gcc/config/avr/avr.c
index ba5cd91..b327624 100644
--- a/gcc/config/avr/avr.c
+++ b/gcc/config/avr/avr.c
@@ -754,8 +754,8 @@ avr_set_current_function (tree decl)
when the user misspells the vector name. */
if (!STR_PREFIX_P (name, "__vector"))
-warning_at (loc, 0, "%qs appears to be a misspelled %s handler",
-name, isr);
+warning_at (loc, OPT_Wmisspelled_isr, "%qs appears to be a misspelled "
+ "%s handler, missing __vector prefix", name, isr);
}
/* Don't print the above diagnostics more than once. */
diff --git a/gcc/config/avr/avr.opt b/gcc/config/avr/avr.opt
index 8809b9b..05aa4b6 100644
--- a/gcc/config/avr/avr.opt
+++ b/gcc/config/avr/avr.opt
@@ -91,6 +91,10 @@ Waddr-space-convert
Warning C Report Var(avr_warn_addr_space_convert) Init(0)
Warn if the address space of an address is changed.
+Wmisspelled-isr
+Warning C C++ Report Var(avr_warn_misspelled_isr) Init(1)
+Warn if the ISR is misspelled, i.e. without __vector prefix. Enabled by default.
+
mfract-convert-truncate
Target Report Mask(FRACT_CONV_TRUNC)
Allow to use truncation instead of rounding towards 0 for fractional int types.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index aa11209..0bf39c5 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -640,7 +640,8 @@ Objective-C and Objective-C++ Dialects}.
@emph{AVR Options}
@gccoptlist{-mmcu=@var{mcu} -maccumulate-args -mbranch-cost=@var{cost} @gol
-mcall-prologues -mint8 -mn_flash=@var{size} -mno-interrupts @gol
--mrelax -mrmw -mstrict-X -mtiny-stack -nodevicelib -Waddr-space-convert}
+-mrelax -mrmw -mstrict-X -mtiny-stack -nodevicelib -Waddr-space-convert @gol
+-Wmisspelled-isr}
@emph{Blackfin Options}
@gccoptlist{-mcpu=@var{cpu}@r{[}-@var{sirevision}@r{]} @gol
@@ -14554,12 +14555,17 @@ Only change the lower 8@tie{}bits of the stack pointer.
@item -nodevicelib
@opindex nodevicelib
-Don't link against AVR-LibC's device specific library @code{libdev.a}.
+Don't link against AVR-LibC's device specific library @code{lib.a}.
@item -Waddr-space-convert
@opindex Waddr-space-convert
Warn about conversions between address spaces in the case where the
resulting address space is not contained in the incoming address space.
+
+@item -Wmisspelled-isr
+@opindex Wmisspelled-isr
+Warn if the ISR is misspelled, i.e. without __vector prefix.
+Enabled by default.
@end table
@subsubsection @code{EIND} and Devices with More Than 128 Ki Bytes of Flash
[patch, avr] PR 58655
Attached patches add documentation for -mfract-convert-truncate option
and add that info to release notes (gcc-4.9 changes).
If OK, could someone commit please? I do not have commit access.
Regards,
Pitchumani
gcc/ChangeLog
2016-06-21 Pitchumani Sivanupandi <pitchuman...@atmel.com>
PR target/58655
* doc/invoke.texi (AVR Options): Document -mfract-convert-truncate
option.
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -643,8 +643,8 @@ Objective-C and Objective-C++ Dialects}.
@emph{AVR Options}
@gccoptlist{-mmcu=@var{mcu} -maccumulate-args -mbranch-cost=@var{cost} @gol
-mcall-prologues -mint8 -mn_flash=@var{size} -mno-interrupts @gol
--mrelax -mrmw -mstrict-X -mtiny-stack -nodevicelib -Waddr-space-convert @gol
--Wmisspelled-isr}
+-mrelax -mrmw -mstrict-X -mtiny-stack -mfract-convert-truncate -nodevicelib @gol
+-Waddr-space-convert -Wmisspelled-isr}
@emph{Blackfin Options}
@gccoptlist{-mcpu=@var{cpu}@r{[}-@var{sirevision}@r{]} @gol
@@ -14586,6 +14586,10 @@ sbiw r26, const ; X -= const
@opindex mtiny-stack
Only change the lower 8@tie{}bits of the stack pointer.
+@item -mfract-convert-truncate
+@opindex mfract-convert-truncate
+Allow to use truncation instead of rounding towards 0 for fractional int types.
+
@item -nodevicelib
@opindex nodevicelib
Don't link against AVR-LibC's device specific library @code{lib.a}.
--- a/wwwdocs/htdocs/gcc-4.9/changes.html
+++ b/wwwdocs/htdocs/gcc-4.9/changes.html
@@ -579,6 +579,14 @@ auto incr(T x) { return x++; }
size when compiling for the M-profile processors.
+AVR
+
+
+A new command-line option -mfract-convert-truncate has been added.
+It allows compiler to use truncation instead of rounding towards
+0 for fractional int types.
+
+
IA-32/x86-64
-mfpmath=sse is now implied by -ffast-math
Re: [patch, avr,wwwdocs] PR 58655
On Tuesday 21 June 2016 09:39 PM, Georg-Johann Lay wrote:
Pitchumani Sivanupandi schrieb:
Attached patches add documentation for -mfract-convert-truncate option
and add that info to release notes (gcc-4.9 changes).
If OK, could someone commit please? I do not have commit access.
Regards,
Pitchumani
gcc/ChangeLog
2016-06-21 Pitchumani Sivanupandi <pitchuman...@atmel.com>
PR target/58655
* doc/invoke.texi (AVR Options): Document -mfract-convert-truncate
option.
--- a/wwwdocs/htdocs/gcc-4.9/changes.html
+++ b/wwwdocs/htdocs/gcc-4.9/changes.html
@@ -579,6 +579,14 @@ auto incr(T x) { return x++; }
size when compiling for the M-profile processors.
+AVR
+
+
+A new command-line option -mfract-convert-truncate has been added.
tags around the option.
+It allows compiler to use truncation instead of rounding towards
+0 for fractional int types.
"zero" instead of "0", and it's for fixed-point types, not for int types.
+
+
IA-32/x86-64
-mfpmath=sse is now implied by
-ffast-math
...
@emph{Blackfin Options}
@gccoptlist{-mcpu=@var{cpu}@r{[}-@var{sirevision}@r{]} @gol
@@ -14586,6 +14586,10 @@ sbiw r26, const ; X -= const
@opindex mtiny-stack
Only change the lower 8@tie{}bits of the stack pointer.
+@item -mfract-convert-truncate
+@opindex mfract-convert-truncate
+Allow to use truncation instead of rounding towards 0 for fractional
int types.
Same here: "zero" and "fixed-point".
+
@item -nodevicelib
@opindex nodevicelib
Don't link against AVR-LibC's device specific library
@code{lib.a}.
Thanks Johann.
Updated the patches.
Regards,
Pitchumani
gcc/ChangeLog
2016-06-22 Pitchumani Sivanupandi <pitchuman...@atmel.com>
PR target/58655
* config/avr/avr.opt (-mfract-convert-truncate): Update description.
* doc/invoke.texi (AVR Options): Document it.
diff --git a/gcc/config/avr/avr.opt b/gcc/config/avr/avr.opt
index 05aa4b6..1af792b 100644
--- a/gcc/config/avr/avr.opt
+++ b/gcc/config/avr/avr.opt
@@ -97,7 +97,7 @@ Warn if the ISR is misspelled, i.e. without __vector prefix. Enabled by default.
mfract-convert-truncate
Target Report Mask(FRACT_CONV_TRUNC)
-Allow to use truncation instead of rounding towards 0 for fractional int types.
+Allow to use truncation instead of rounding towards zero for fractional fixed-point types.
nodevicelib
Driver Target Report RejectNegative
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index e000218..040fb6e 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -643,8 +643,8 @@ Objective-C and Objective-C++ Dialects}.
@emph{AVR Options}
@gccoptlist{-mmcu=@var{mcu} -maccumulate-args -mbranch-cost=@var{cost} @gol
-mcall-prologues -mint8 -mn_flash=@var{size} -mno-interrupts @gol
--mrelax -mrmw -mstrict-X -mtiny-stack -nodevicelib -Waddr-space-convert @gol
--Wmisspelled-isr}
+-mrelax -mrmw -mstrict-X -mtiny-stack -mfract-convert-truncate -nodevicelib @gol
+-Waddr-space-convert -Wmisspelled-isr}
@emph{Blackfin Options}
@gccoptlist{-mcpu=@var{cpu}@r{[}-@var{sirevision}@r{]} @gol
@@ -14586,6 +14586,10 @@ sbiw r26, const ; X -= const
@opindex mtiny-stack
Only change the lower 8@tie{}bits of the stack pointer.
+@item -mfract-convert-truncate
+@opindex mfract-convert-truncate
+Allow to use truncation instead of rounding towards zero for fractional fixed-point types.
+
@item -nodevicelib
@opindex nodevicelib
Don't link against AVR-LibC's device specific library @code{lib.a}.
--- a/wwwdocs/htdocs/gcc-4.9/changes.html
+++ b/wwwdocs/htdocs/gcc-4.9/changes.html
@@ -579,6 +579,14 @@ auto incr(T x) { return x++; }
size when compiling for the M-profile processors.
+AVR
+
+
+A new command-line option -mfract-convert-truncate has been
+added. It allows compiler to use truncation instead of rounding towards
+zero for fractional fixed-point types.
+
+
IA-32/x86-64
-mfpmath=sse is now implied by -ffast-math
Ping: [patch, avr] Fix unrecognizable insn ICE for avr (PR71103)
Ping!
Note: Removed the garbled characters and added ChangeLog
--
avr-gcc crashes for following test as it couldn't recognize the
instruction pattern.
struct st {
unsigned char uc1;
unsigned int *ui1;
};
unsigned int ui1;
struct st foo () {
struct st ret;
ret.uc1 = 6;
ret.ui1 =
return ret;
}
$ avr-gcc -mmcu=atmega328p -O1 test.c
(-- snip --)
test.c: In function 'foo':
test.c:12:1: error: unrecognizable insn:
}
^
(insn 6 5 7 2 (set (subreg:QI (reg:PSI 42 [ D.1499 ]) 1)
(subreg:QI (symbol_ref:HI ("ui1") ) 0)) test.c:11 -1
(nil))
test.c:12:1: internal compiler error: in extract_insn, at recog.c:2287
0xd51195 _fatal_insn(char const*, rtx_def const*, char const*, int,
char const*)
/home/rudran/code/gcc/gcc/rtl-error.c:108
(--snip--)
There is no valid pattern in avr to match the "subreg:QI
(symbol_ref:HI)". Attached patch forces the symbol_ref of subreg
operand to register so that it will become register operand and movqi
pattern shall recognize it.
Ran gcc regression test with internal simulators. No new regressions
found.
If Ok, could someone commit please?
Regards,
Pitchumani
gcc/ChangeLog
2016-05-20 Pitchumani Sivanupandi <pitchuman...@atmel.com>
PR target/71103
* config/avr/avr.md (define_expand "mov"): If the source
operand is
subreg (symbol_ref) then move the symbol ref to register.
gcc/testsuite/ChangeLog
2016-05-20 Pitchumani Sivanupandi <pitchuman...@atmel.com>
PR target/71103
* gcc.target/avr/pr71103.c: New test.diff --git a/gcc/config/avr/avr.md b/gcc/config/avr/avr.md
index c988446..927bc69 100644
--- a/gcc/config/avr/avr.md
+++ b/gcc/config/avr/avr.md
@@ -638,6 +638,13 @@
rtx dest = operands[0];
rtx src = avr_eval_addr_attrib (operands[1]);
+if (SUBREG_P(src) && (GET_CODE(XEXP(src,0)) == SYMBOL_REF) &&
+can_create_pseudo_p())
+ {
+rtx symbol_ref = XEXP(src, 0);
+XEXP (src, 0) = copy_to_mode_reg (GET_MODE(symbol_ref), symbol_ref);
+ }
+
if (avr_mem_flash_p (dest))
DONE;
diff --git a/gcc/testsuite/gcc.target/avr/pr71103.c b/gcc/testsuite/gcc.target/avr/pr71103.c
new file mode 100644
index 000..43244d1
--- /dev/null
+++ b/gcc/testsuite/gcc.target/avr/pr71103.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-O1" } */
+
+struct ResponseStruct{
+unsigned char responseLength;
+char *response;
+};
+
+static char response[5];
+struct ResponseStruct something(){
+struct ResponseStruct returnValue;
+returnValue.responseLength = 5;
+returnValue.response = response;
+return returnValue;
+}
+
[patch, avr] Fix unrecognizable insn ICE for avr (PR71103)
avr-gcc crashes for following test as it couldn't recognize the
instruction pattern.
struct st {
unsigned char uc1;
unsigned int *ui1;
};
unsigned int ui1;
struct st foo () {
struct st ret;
ret.uc1 = 6;
ret.ui1 =
return ret;
}
$ avr-gcc -mmcu=atmega328p -O1 test.c
(-- snip --)
test.c: In function 'foo':
test.c:12:1: error: unrecognizable insn:
}
^
(insn 6 5 7 2 (set (subreg:QI (reg:PSI 42 [ D.1499 ]) 1)
(subreg:QI (symbol_ref:HI ("ui1") ) 0)) test.c:11 -1
(nil))
test.c:12:1: internal compiler error: in extract_insn, at recog.c:2287
0xd51195 _fatal_insn(char const*, rtx_def const*, char const*, int,
char const*)
/home/rudran/code/gcc/gcc/rtl-error.c:108
(-- snip --)
There is no valid pattern in avr to match the "subreg:QI
(symbol_ref:HI)". Attached patch forces the symbol_ref of subreg
operand to register so that it will become register operand and movqi
pattern shall recognize it.
Ran gcc regression test with internal simulators. No new regressions
found.
If ok, could someone commit please?
Regards,
Pitchumanidiff --git a/gcc/config/avr/avr.md b/gcc/config/avr/avr.md
index c988446..927bc69 100644
--- a/gcc/config/avr/avr.md
+++ b/gcc/config/avr/avr.md
@@ -638,6 +638,13 @@
rtx dest = operands[0];
rtx src = avr_eval_addr_attrib (operands[1]);
+if (SUBREG_P(src) && (GET_CODE(XEXP(src,0)) == SYMBOL_REF) &&
+can_create_pseudo_p())
+ {
+rtx symbol_ref = XEXP(src, 0);
+XEXP (src, 0) = copy_to_mode_reg (GET_MODE(symbol_ref), symbol_ref);
+ }
+
if (avr_mem_flash_p (dest))
DONE;
diff --git a/gcc/testsuite/gcc.target/avr/pr71103.c b/gcc/testsuite/gcc.target/avr/pr71103.c
new file mode 100644
index 000..43244d1
--- /dev/null
+++ b/gcc/testsuite/gcc.target/avr/pr71103.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-O1" } */
+
+struct ResponseStruct{
+unsigned char responseLength;
+char *response;
+};
+
+static char response[5];
+struct ResponseStruct something(){
+struct ResponseStruct returnValue;
+returnValue.responseLength = 5;
+returnValue.response = response;
+return returnValue;
+}
+
Re: [patch, avr] Fix mmcu to specs issues
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-%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= 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 <pitchuman...@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.
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);
-
- re
Re: [patch, avr] Fix mmcu to specs issues
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-%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= 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 <pitchuman...@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,
Pitc
[patch, avr] Fix mmcu to specs issues
cs directory.
Instead this spec function will let gcc identify the absolute path of
specs file
using spec string "device-specs-file (device-specs/specs-%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 <pitchuman...@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.
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=. 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=%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=. 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,
-
Re: [patch, avr,wwwdocs] PR 58655
Ping!
On Wednesday 22 June 2016 12:05 PM, Pitchumani Sivanupandi wrote:
On Tuesday 21 June 2016 09:39 PM, Georg-Johann Lay wrote:
Pitchumani Sivanupandi schrieb:
Attached patches add documentation for -mfract-convert-truncate option
and add that info to release notes (gcc-4.9 changes).
If OK, could someone commit please? I do not have commit access.
Regards,
Pitchumani
gcc/ChangeLog
2016-06-21 Pitchumani Sivanupandi <pitchuman...@atmel.com>
PR target/58655
* doc/invoke.texi (AVR Options): Document -mfract-convert-truncate
option.
--- a/wwwdocs/htdocs/gcc-4.9/changes.html
+++ b/wwwdocs/htdocs/gcc-4.9/changes.html
@@ -579,6 +579,14 @@ auto incr(T x) { return x++; }
size when compiling for the M-profile processors.
+AVR
+
+
+A new command-line option -mfract-convert-truncate has been added.
tags around the option.
+It allows compiler to use truncation instead of rounding towards
+0 for fractional int types.
"zero" instead of "0", and it's for fixed-point types, not for int
types.
+
+
IA-32/x86-64
-mfpmath=sse is now implied by
-ffast-math
...
@emph{Blackfin Options}
@gccoptlist{-mcpu=@var{cpu}@r{[}-@var{sirevision}@r{]} @gol
@@ -14586,6 +14586,10 @@ sbiw r26, const ; X -= const
@opindex mtiny-stack
Only change the lower 8@tie{}bits of the stack pointer.
+@item -mfract-convert-truncate
+@opindex mfract-convert-truncate
+Allow to use truncation instead of rounding towards 0 for fractional
int types.
Same here: "zero" and "fixed-point".
+
@item -nodevicelib
@opindex nodevicelib
Don't link against AVR-LibC's device specific library
@code{lib.a}.
Thanks Johann.
Updated the patches.
Regards,
Pitchumani
gcc/ChangeLog
2016-06-22 Pitchumani Sivanupandi <pitchuman...@atmel.com>
PR target/58655
* config/avr/avr.opt (-mfract-convert-truncate): Update description.
* doc/invoke.texi (AVR Options): Document it.
Re: [patch, avr] Fix mmcu to specs issues
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-%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= 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 <pitchuman...@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.
[patch, avr, pr71676 and pr71678] Issues with casesi expand
Attached patch for PR71676 and PR71678.
PR71676 is for AVR target that generates wrong code when switch case
index is
more than 16 bits.
Switch case index of larger than SImode are checked for out of range before
'casesi' expand. RTL expand of casesi gets index as SImode, but index is
compared in HImode and ignores upper 16bits.
Attached patch changes the expansion for casesi to make the index comparison
in SImode and code generation accordingly.
PR71678 is ICE because below pattern in 'casesi' is not recognized.
(set (reg:HI 47)
(minus:HI (subreg:HI (subreg:SI (reg:DI 44) 0) 0)
(reg:HI 45)))
Fix of PR71676 avoids the above pattern as it changes the comparison
to SImode.
Regtested using avrtest. No regression found.
If OK, could someone commit please?
Is this OK for gcc-5-branch?
Regards,
Pitchumani
gcc/ChangeLog
2016-09-26 Pitchumani Sivanupandi <pitchuman...@atmel.com>
PR target/71676
PR target/71678
* config/avr/avr.md (casesi): Change index compare to SI mode.
gcc/testsuite/ChangeLog
2016-09-26 Pitchumani Sivanupandi <pitchuman...@atmel.com>
PR target/71676
PR target/71678
* gcc.target/avr/pr71676-1.c: New test.
* gcc.target/avr/pr71676.c: New test.
* gcc.target/avr/pr71678.c: New test.
diff --git a/gcc/config/avr/avr.md b/gcc/config/avr/avr.md
index 97f3561..4b1bf9c 100644
--- a/gcc/config/avr/avr.md
+++ b/gcc/config/avr/avr.md
@@ -5155,12 +5155,12 @@
(define_expand "casesi"
[(parallel [(set (match_dup 6)
- (minus:HI (subreg:HI (match_operand:SI 0 "register_operand" "") 0)
- (match_operand:HI 1 "register_operand" "")))
+ (minus:SI (match_operand:SI 0 "register_operand" "")
+ (match_operand:SI 1 "register_operand" "")))
(clobber (scratch:QI))])
(parallel [(set (cc0)
(compare (match_dup 6)
-(match_operand:HI 2 "register_operand" "")))
+(match_operand:SI 2 "register_operand" "")))
(clobber (match_scratch:QI 9 ""))])
(set (pc)
@@ -5179,20 +5179,20 @@
(clobber (match_dup 8))])]
""
{
-operands[6] = gen_reg_rtx (HImode);
+operands[6] = gen_reg_rtx (SImode);
if (AVR_HAVE_EIJMP_EICALL)
{
-operands[7] = operands[6];
+operands[7] = simplify_gen_subreg (HImode, operands[6], SImode, 0);
operands[8] = all_regs_rtx[24];
operands[10] = gen_rtx_REG (HImode, REG_Z);
}
else
{
-operands[7] = gen_rtx_PLUS (HImode, operands[6],
+operands[7] = gen_rtx_PLUS (HImode, simplify_gen_subreg (HImode, operands[6], SImode, 0),
gen_rtx_LABEL_REF (VOIDmode, operands[3]));
operands[8] = const0_rtx;
-operands[10] = operands[6];
+operands[10] = simplify_gen_subreg (HImode, operands[6], SImode, 0);
}
})
diff --git a/gcc/testsuite/gcc.target/avr/pr71676-1.c b/gcc/testsuite/gcc.target/avr/pr71676-1.c
new file mode 100644
index 000..9a74909
--- /dev/null
+++ b/gcc/testsuite/gcc.target/avr/pr71676-1.c
@@ -0,0 +1,332 @@
+/* { dg-do run } */
+/* { dg-options "-Os -Wno-overflow" } */
+
+#include "exit-abort.h"
+volatile unsigned char y;
+
+unsigned char __attribute__((noinline)) foo1 (char x)
+{
+switch (x)
+{
+ case (char)0x11: y = 7; break;
+ case (char)0x12: y = 4; break;
+ case (char)0x13: y = 8; break;
+ case (char)0x14: y = 21; break;
+ case (char)0x15: y = 65; break;
+ case (char)0x16: y = 27; break;
+ case (char)0x17: y = 72; break;
+ case (char)0x18: y = 39; break;
+ default: y=0;
+}
+return y;
+}
+
+unsigned char __attribute__((noinline)) foo2 (char x)
+{
+switch (x)
+{
+ case 0x01: y = 7; break;
+ case 0x02: y = 4; break;
+ case 0x03: y = 8; break;
+ case 0x04: y = 21; break;
+ case 0x05: y = 65; break;
+ case 0x06: y = 27; break;
+ case 0x07: y = 72; break;
+ case 0x08: y = 39; break;
+ default: y=0;
+}
+return y;
+}
+
+unsigned char __attribute__((noinline)) foo3 (char x)
+{
+switch (x)
+{
+ case 0x101L: y = 7; break;
+ case 0x102L: y = 4; break;
+ case 0x103L: y = 8; break;
+ case 0x104L: y = 21; break;
+ case 0x105L: y = 65; break;
+ case 0x106L: y = 27; break;
+ case 0x107L: y = 72; break;
+ case 0x108L: y = 39; break;
+ default: y=0;
+}
+return y;
+}
+
+unsigned char __attribute__((noinline)) foo4 (char x)
+{
+switch (x)
+{
+ case 0x10001LL: y = 7; break;
+ case 0x10002LL: y = 4; break;
+ case 0x10003LL: y = 8; break;
+ case 0x
[patch, avr] Add flash size to device info and make wrap around default (was: Re: [patch, avr] Make pmem-wrap-around option as default)
On Thursday 03 November 2016 06:19 PM, Georg-Johann Lay wrote:
On 03.11.2016 08:58, Pitchumani Sivanupandi wrote:
Most of the AVR's 8k memorydevices have only rjmp instruction, not
jmp. So, it
is important to wrap around jump destination to check if it can reach
backwards.
Currently link specs passes --pmem-wrap-around=xxK when mrelax and
mpmem-wrap-around options are enabled. Attached patch changes the
specs so that
option --pmem-wrap-around=8K is passed for 8k memory devices if
-mno-pmem-wrap-around is not enabled.
If OK, could someone commit please?
Note: Currently 8k devices are identified based on name prefix. We
are working
on alternative method to incorporate flash memory size.
Currently, "at90usb8" this the only prefix that results in wrap_k = 8,
so without adding knowledge about flash sizes to the compiler, the
change makes not much sense... (but does not do harm either).
There are ~40 devices with 8K flash. Current checks are out-dated.
The right place for flash size info would be avr-mcus.def together
with a new command option to specify the flash size and draw
conclusions from it. When I asked Atmel about a mapping of Device to
flash size, the support told me to check the data sheets -- not really
practical for ~300 devices in avr-mcus.def :-) Atmel *must* have
information about this...
I have updated patch to include the flash size as well. Took that info
from device headers (it was fed into crt's device information note
section also).
The new option would render -mn-flash superfluous, but we should keep
it for backward compatibility.
Ok.
Shouldn't link_pmem_wrap then be removed from link_relax, i.e. from
LINK_RELAX_SPEC? And what happens if relaxation is off?
Yes. Removed link_pmem_wrap from link_relax.
Disabling relaxation doesn't change -mpmem-wrap-around behavior.
Now, wrap around behavior is changed as follows:
For 8K flash devices:
Device specs adds --pmem-wrap-around=8k linker option if
-mno-pmem-wrap-around is NOT enabled.
It makes the --pmem-wrap-around=8k linker option default for 8k flash
devices.
For 16/32/64K flash devices:
Spec string 'link_pmem_wrap' added to all 16/32/64k flash devices specs.
Other wise no changes i.e. It adds --pmem-wrap-around=16/32/64k option
if -mpmem-wrap-around option is enabled.
For other devices, no changes in device specs.
Reg tested with default and -mrelax options enabled. No issues.
If OK, could someone commit please?
Regards,
Pitchumani
gcc/ChangeLog
2016-11-08 Pitchumani Sivanupandi <pitchuman...@atmel.com>
* config/avr/avr-arch.h (avr_mcu_t): Add flash_size member.
* config/avr/avr-devices.c(avr_mcu_types): Add flash size info.
* config/avr/avr-mcu.def: Likewise.
* config/avr/gen-avr-mmcu-specs.c (print_mcu): Remove hard-coded prefix
check to find wrap-around value, instead use MCU flash size. For 8k
flash
devices, update link_pmem_wrap spec string to add
--pmem-wrap-around=8k.
* config/avr/specs.h: Remove link_pmem_wrap from LINK_RELAX_SPEC and
add to linker specs (LINK_SPEC) directly.
diff --git a/gcc/config/avr/avr-arch.h b/gcc/config/avr/avr-arch.h
index 42eaee5..e0961d4 100644
--- a/gcc/config/avr/avr-arch.h
+++ b/gcc/config/avr/avr-arch.h
@@ -122,6 +122,9 @@ typedef struct
/* Number of 64k segments in the flash. */
int n_flash;
+
+ /* Flash size in bytes. */
+ int flash_size;
} avr_mcu_t;
/* AVR device specific features.
diff --git a/gcc/config/avr/avr-devices.c b/gcc/config/avr/avr-devices.c
index 7d13ba4..cef3b9a 100644
--- a/gcc/config/avr/avr-devices.c
+++ b/gcc/config/avr/avr-devices.c
@@ -111,12 +111,12 @@ avr_texinfo[] =
const avr_mcu_t
avr_mcu_types[] =
{
-#define AVR_MCU(NAME, ARCH, DEV_ATTRIBUTE, MACRO, DATA_SEC, TEXT_SEC, N_FLASH)\
- { NAME, ARCH, DEV_ATTRIBUTE, MACRO, DATA_SEC, TEXT_SEC, N_FLASH },
+#define AVR_MCU(NAME, ARCH, DEV_ATTRIBUTE, MACRO, DATA_SEC, TEXT_SEC, N_FLASH, FLASH_SIZE)\
+ { NAME, ARCH, DEV_ATTRIBUTE, MACRO, DATA_SEC, TEXT_SEC, N_FLASH, FLASH_SIZE },
#include "avr-mcus.def"
#undef AVR_MCU
/* End of list. */
- { NULL, ARCH_UNKNOWN, AVR_ISA_NONE, NULL, 0, 0, 0 }
+ { NULL, ARCH_UNKNOWN, AVR_ISA_NONE, NULL, 0, 0, 0, 0 }
};
diff --git a/gcc/config/avr/avr-mcus.def b/gcc/config/avr/avr-mcus.def
index 6bcc6ff..9d4aa1a 100644
--- a/gcc/config/avr/avr-mcus.def
+++ b/gcc/config/avr/avr-mcus.def
@@ -62,295 +62,297 @@
N_FLASH Number of 64 KiB flash segments, rounded up. The default
value for -mn-flash=.
+ FLASH_SIZEFlash size in bytes.
+
"avr2" must be first for the "0" default to work as intended. */
/* Classic, <= 8K. */
-AVR_MCU ("avr2", ARCH_AVR2, AVR_ERRATA_SKIP, NULL, 0x0060, 0x0, 6)
-AVR_MCU ("at90s2313",ARCH_AVR2, AVR_SHORT_SP, "__AVR_AT90S2313__", 0x0060, 0x0, 1)
-AVR_MCU ("at90s2323",A
Re: [patch, avr] Add flash size to device info and make wrap around default
On Wednesday 09 November 2016 08:05 PM, Georg-Johann Lay wrote:
On 09.11.2016 10:14, Pitchumani Sivanupandi wrote:
On Tuesday 08 November 2016 02:57 PM, Georg-Johann Lay wrote:
On 08.11.2016 08:08, Pitchumani Sivanupandi wrote:
I have updated patch to include the flash size as well. Took that
info from
device headers (it was fed into crt's device information note
section also).
The new option would render -mn-flash superfluous, but we should
keep it for
backward compatibility.
Ok.
Shouldn't link_pmem_wrap then be removed from link_relax, i.e. from
LINK_RELAX_SPEC? And what happens if relaxation is off?
Yes. Removed link_pmem_wrap from link_relax.
Disabling relaxation doesn't change -mpmem-wrap-around behavior.
flashsize-and-wrap-around.patch
diff --git a/gcc/config/avr/avr-mcus.def b/gcc/config/avr/avr-mcus.def
index 6bcc6ff..9d4aa1a 100644
/*
/* Classic, > 8K, <= 64K. */
-AVR_MCU ("avr3", ARCH_AVR3, AVR_ISA_NONE,
NULL,0x0060, 0x0, 1)
-AVR_MCU ("at43usb355", ARCH_AVR3, AVR_ISA_NONE,
"__AVR_AT43USB355__",0x0060, 0x0, 1)
-AVR_MCU ("at76c711", ARCH_AVR3, AVR_ISA_NONE,
"__AVR_AT76C711__", 0x0060, 0x0, 1)
+AVR_MCU ("avr3", ARCH_AVR3, AVR_ISA_NONE,
NULL,0x0060, 0x0, 1, 0x6000)
+AVR_MCU ("at43usb355", ARCH_AVR3, AVR_ISA_NONE,
"__AVR_AT43USB355__",0x0060, 0x0, 1, 0x6000)
+AVR_MCU ("at76c711", ARCH_AVR3, AVR_ISA_NONE,
"__AVR_AT76C711__", 0x0060, 0x0, 1, 0x4000)
+AVR_MCU ("at43usb320", ARCH_AVR3, AVR_ISA_NONE,
"__AVR_AT43USB320__",0x0060, 0x0, 1, 0x1)
/* Classic, == 128K. */
-AVR_MCU ("avr31",ARCH_AVR31, AVR_ERRATA_SKIP,
NULL,0x0060, 0x0, 2)
-AVR_MCU ("atmega103",ARCH_AVR31, AVR_ERRATA_SKIP,
"__AVR_ATmega103__", 0x0060, 0x0, 2)
-AVR_MCU ("at43usb320", ARCH_AVR31, AVR_ISA_NONE,
"__AVR_AT43USB320__", 0x0060, 0x0, 2)
+AVR_MCU ("avr31",ARCH_AVR31, AVR_ERRATA_SKIP,
NULL,0x0060, 0x0, 2, 0x2)
+AVR_MCU ("atmega103",ARCH_AVR31, AVR_ERRATA_SKIP,
"__AVR_ATmega103__", 0x0060, 0x0, 2, 0x2)
/* Classic + MOVW + JMP/CALL. */
If at43usb320 is in the wrong multilib, then this should be handled as
separate issue / patch together with its own PR. Sorry for the
confusion. I just noticed that some fields don't match...
It is not even clear to me from the data sheet if avr3 is the correct
multilib or perhaps avr35 (if it supports MOVW) or even avr5 (if it
also has MUL) as there is no reference to the exact instruction set --
Atmochip will know.
Moreover, such a change should be sync'ed with avr-libc as all
multilib stuff is hand-wired there: no use of --print-foo meta
information retrieval by avr-libc :-((
I filed PR78275 and https://savannah.nongnu.org/bugs/index.php?49565
for this one.
Thats better. I've attached the updated patch. If OK, could someone
commit please?
I'll try if I could find some more info for AT43USB320.
Regards,
Pitchumani
diff --git a/gcc/config/avr/avr-arch.h b/gcc/config/avr/avr-arch.h
index 42eaee5..e0961d4 100644
--- a/gcc/config/avr/avr-arch.h
+++ b/gcc/config/avr/avr-arch.h
@@ -122,6 +122,9 @@ typedef struct
/* Number of 64k segments in the flash. */
int n_flash;
+
+ /* Flash size in bytes. */
+ int flash_size;
} avr_mcu_t;
/* AVR device specific features.
diff --git a/gcc/config/avr/avr-devices.c b/gcc/config/avr/avr-devices.c
index 7d13ba4..cef3b9a 100644
--- a/gcc/config/avr/avr-devices.c
+++ b/gcc/config/avr/avr-devices.c
@@ -111,12 +111,12 @@ avr_texinfo[] =
const avr_mcu_t
avr_mcu_types[] =
{
-#define AVR_MCU(NAME, ARCH, DEV_ATTRIBUTE, MACRO, DATA_SEC, TEXT_SEC, N_FLASH)\
- { NAME, ARCH, DEV_ATTRIBUTE, MACRO, DATA_SEC, TEXT_SEC, N_FLASH },
+#define AVR_MCU(NAME, ARCH, DEV_ATTRIBUTE, MACRO, DATA_SEC, TEXT_SEC, N_FLASH, FLASH_SIZE)\
+ { NAME, ARCH, DEV_ATTRIBUTE, MACRO, DATA_SEC, TEXT_SEC, N_FLASH, FLASH_SIZE },
#include "avr-mcus.def"
#undef AVR_MCU
/* End of list. */
- { NULL, ARCH_UNKNOWN, AVR_ISA_NONE, NULL, 0, 0, 0 }
+ { NULL, ARCH_UNKNOWN, AVR_ISA_NONE, NULL, 0, 0, 0, 0 }
};
diff --git a/gcc/config/avr/avr-mcus.def b/gcc/config/avr/avr-mcus.def
index 6bcc6ff..6e51c2e 100644
--- a/gcc/config/avr/avr-mcus.def
+++ b/gcc/config/avr/avr-mcus.def
@@ -62,295 +62,297 @@
N_FLASH Number of 64 KiB flash segments, rounded up. The default
value for -mn-flash=.
+ FLASH_SIZEFlash size in bytes.
+
"avr2" must be first for the "0" default to work as intended. */
/* Classic, <= 8K. */
-AVR_MCU ("avr2", ARCH_AVR2, AVR_ERRATA_SKIP, N
Re: [patch, avr, pr71676 and pr71678] Issues with casesi expand
On Wednesday 19 October 2016 07:51 PM, Georg-Johann Lay wrote: On 17.10.2016 09:27, Pitchumani Sivanupandi wrote: On Thursday 13 October 2016 08:42 PM, Georg-Johann Lay wrote: On 13.10.2016 13:44, Pitchumani Sivanupandi wrote: On Monday 26 September 2016 08:19 PM, Georg-Johann Lay wrote: On 26.09.2016 15:19, Pitchumani Sivanupandi wrote: Attached patch for PR71676 and PR71678. PR71676 is for AVR target that generates wrong code when switch case index is more than 16 bits. Switch case index of larger than SImode are checked for out of range before 'casesi' expand. RTL expand of casesi gets index as SImode, but index is compared in HImode and ignores upper 16bits. Attached patch changes the expansion for casesi to make the index comparison in SImode and code generation accordingly. PR71678 is ICE because below pattern in 'casesi' is not recognized. (set (reg:HI 47) (minus:HI (subreg:HI (subreg:SI (reg:DI 44) 0) 0) (reg:HI 45))) Fix of PR71676 avoids the above pattern as it changes the comparison to SImode. But this means that all comparisons are now performed in SImode which is a great performance loss for most programs which will switch on 16-bit values. IMO we need a less intrusive (w.r.t. performance) approach. Yes. I tried to split 'casesi' into several based on case values so that compare is done in less expensive modes (i.e. QI or HI). In few cases it is not possible without SImode subtract/ compare. Pattern casesi will have index in SI mode. So, out of range checks will be expensive as most common uses (in AVR) of case values will be in QI/HI mode. e.g. if case values in QI range if upper three bytes index is set goto out_of_range offset = index - lower_bound (QImode) if offset > case_range (QImode) goto out_of_range goto jump_table + offset else if case values in HI range if index[2,3] is set goto out_of_range offset = index - lower_bound (HImode) if offset > case_range (HImode) goto out_of_range goto jump_table + offset This modification will not work for the negative index values. Because code to check upper bytes of index will be expensive than the SImode subtract/ compare. So, I'm trying to update fix to have SImode subtract/ compare if the case values include negative integers. For, others will try to optimize as mentioned above. Is that approach OK? But the above code will be executed at run time and add even more overhead, or am I missing something? If you conclude statically at expand time from the case ranges then we might hit a similar problem as with the original subreg computation. No. Lower bound and case range are const_int_operand, known at compile time. Yes, but if the range if form 10 to 90, say, then you still don't know whether HImode and QImode is appropriate or not which adds to code size and register pressure. As I mentioned earlier, I am working on a different approach which would revert your changes: The casesi is basically unaltered (except for operand clean-ups and avoidance of clobbering subregs). The ups of my approach are: * The original value is known and whether is is QI or HI. * The signedness is known which allows to use the maximum range of QI resp. HI depending on the sign. * Also works on negative values. * All is done at compile time, no need for extra code. * No intermediate 32-bit values, no unnecessary increase of reg pressure. * Optimization can be switched off by -fdisable if desired. * Result can be seen in dumps. The downsides are: * Also needs some lines of code (~400). * Makes assumptions on the anatomy of the code, i.e. extension precedes casesi. First we should decide which route to follow as the changes are conflicting each other. I have not so much time to work on the stuff but the results are promising. If you are interested in the changes, I can supply it but it's all still work in progress. Ok. I'll put this patch on hold for now. Please share if you have draft version of your fix is ready. Thanks. Regards, Pitchumani
Re: [patch, avr, pr71676 and pr71678] Issues with casesi expand
On Thursday 13 October 2016 08:42 PM, Georg-Johann Lay wrote: On 13.10.2016 13:44, Pitchumani Sivanupandi wrote: On Monday 26 September 2016 08:19 PM, Georg-Johann Lay wrote: On 26.09.2016 15:19, Pitchumani Sivanupandi wrote: Attached patch for PR71676 and PR71678. PR71676 is for AVR target that generates wrong code when switch case index is more than 16 bits. Switch case index of larger than SImode are checked for out of range before 'casesi' expand. RTL expand of casesi gets index as SImode, but index is compared in HImode and ignores upper 16bits. Attached patch changes the expansion for casesi to make the index comparison in SImode and code generation accordingly. PR71678 is ICE because below pattern in 'casesi' is not recognized. (set (reg:HI 47) (minus:HI (subreg:HI (subreg:SI (reg:DI 44) 0) 0) (reg:HI 45))) Fix of PR71676 avoids the above pattern as it changes the comparison to SImode. But this means that all comparisons are now performed in SImode which is a great performance loss for most programs which will switch on 16-bit values. IMO we need a less intrusive (w.r.t. performance) approach. Yes. I tried to split 'casesi' into several based on case values so that compare is done in less expensive modes (i.e. QI or HI). In few cases it is not possible without SImode subtract/ compare. Pattern casesi will have index in SI mode. So, out of range checks will be expensive as most common uses (in AVR) of case values will be in QI/HI mode. e.g. if case values in QI range if upper three bytes index is set goto out_of_range offset = index - lower_bound (QImode) if offset > case_range (QImode) goto out_of_range goto jump_table + offset else if case values in HI range if index[2,3] is set goto out_of_range offset = index - lower_bound (HImode) if offset > case_range (HImode) goto out_of_range goto jump_table + offset This modification will not work for the negative index values. Because code to check upper bytes of index will be expensive than the SImode subtract/ compare. So, I'm trying to update fix to have SImode subtract/ compare if the case values include negative integers. For, others will try to optimize as mentioned above. Is that approach OK? But the above code will be executed at run time and add even more overhead, or am I missing something? If you conclude statically at expand time from the case ranges then we might hit a similar problem as with the original subreg computation. No. Lower bound and case range are const_int_operand, known at compile time. Tried to optimize code generated based on case values range. Attached the revised patch. Tested with avrtest, no regression found. Is it OK? Unfortunately, the generated code (setting cc0, a reg and pc) cannot be wrapped into an unspec or parallel and then later be rectified... I am thinking about a new avr target pass to tidy up the code if no 32-bit computation is needed, but this will be some effort. Ok. Regards, Pitchumani gcc/ChangeLog 2016-10-17 Pitchumani Sivanupandi <pitchuman...@atmel.com> PR target/71676 PR target/71678 * config/avr/avr.md (casesi_index_qi, casesi_index_hi, casesi_index_si): Add new expands, called by casesi based on case values range. gcc/testsuite/ChangeLog 2016-10-17 Pitchumani Sivanupandi <pitchuman...@atmel.com> PR target/71676 PR target/71678 * gcc.target/avr/pr71676-1.c: New test. * gcc.target/avr/pr71676.c: New test. * gcc.target/avr/pr71678.c: New test. diff --git a/gcc/config/avr/avr.md b/gcc/config/avr/avr.md index 97f3561..b58db14 100644 --- a/gcc/config/avr/avr.md +++ b/gcc/config/avr/avr.md @@ -5152,16 +5152,95 @@ (set_attr "isa" "eijmp") (set_attr "cc" "clobber")]) +; casesi for QI mode case values +(define_expand "casesi_index_qi" + [(set (cc0) +(compare:QI (subreg:QI (match_dup 0) 0) +(match_operand 2 "const_int_operand" ""))) -(define_expand "casesi" - [(parallel [(set (match_dup 6) - (minus:HI (subreg:HI (match_operand:SI 0 "register_operand" "") 0) - (match_operand:HI 1 "register_operand" ""))) - (clobber (scratch:QI))]) - (parallel [(set (cc0) - (compare (match_dup 6) -(match_operand:HI 2 "register_operand" ""))) - (clobber (match_scratch:QI 9 ""))]) + (set (pc) +(if_then_else (gtu (cc0) + (const_int 0)) + (label_ref (match_operand 4 "" "")) + (pc))) + + (set (match_dup 9) +(match_dup 7)) + + (parallel [(set (pc) + (unspec:HI [(match_dup 9)] UNSPEC_IND
Re: [patch, avr] Add flash size to device info and make wrap around default
Ping!
On Thursday 10 November 2016 01:53 PM, Pitchumani Sivanupandi wrote:
On Wednesday 09 November 2016 08:05 PM, Georg-Johann Lay wrote:
On 09.11.2016 10:14, Pitchumani Sivanupandi wrote:
On Tuesday 08 November 2016 02:57 PM, Georg-Johann Lay wrote:
On 08.11.2016 08:08, Pitchumani Sivanupandi wrote:
I have updated patch to include the flash size as well. Took that
info from
device headers (it was fed into crt's device information note
section also).
The new option would render -mn-flash superfluous, but we should
keep it for
backward compatibility.
Ok.
Shouldn't link_pmem_wrap then be removed from link_relax, i.e. from
LINK_RELAX_SPEC? And what happens if relaxation is off?
Yes. Removed link_pmem_wrap from link_relax.
Disabling relaxation doesn't change -mpmem-wrap-around behavior.
flashsize-and-wrap-around.patch
diff --git a/gcc/config/avr/avr-mcus.def
b/gcc/config/avr/avr-mcus.def
index 6bcc6ff..9d4aa1a 100644
/*
/* Classic, > 8K, <= 64K. */
-AVR_MCU ("avr3", ARCH_AVR3, AVR_ISA_NONE,
NULL,0x0060, 0x0, 1)
-AVR_MCU ("at43usb355", ARCH_AVR3, AVR_ISA_NONE,
"__AVR_AT43USB355__",0x0060, 0x0, 1)
-AVR_MCU ("at76c711", ARCH_AVR3, AVR_ISA_NONE,
"__AVR_AT76C711__", 0x0060, 0x0, 1)
+AVR_MCU ("avr3", ARCH_AVR3, AVR_ISA_NONE,
NULL,0x0060, 0x0, 1, 0x6000)
+AVR_MCU ("at43usb355", ARCH_AVR3, AVR_ISA_NONE,
"__AVR_AT43USB355__",0x0060, 0x0, 1, 0x6000)
+AVR_MCU ("at76c711", ARCH_AVR3, AVR_ISA_NONE,
"__AVR_AT76C711__", 0x0060, 0x0, 1, 0x4000)
+AVR_MCU ("at43usb320", ARCH_AVR3, AVR_ISA_NONE,
"__AVR_AT43USB320__",0x0060, 0x0, 1, 0x1)
/* Classic, == 128K. */
-AVR_MCU ("avr31",ARCH_AVR31, AVR_ERRATA_SKIP,
NULL,0x0060, 0x0, 2)
-AVR_MCU ("atmega103",ARCH_AVR31, AVR_ERRATA_SKIP,
"__AVR_ATmega103__", 0x0060, 0x0, 2)
-AVR_MCU ("at43usb320", ARCH_AVR31, AVR_ISA_NONE,
"__AVR_AT43USB320__", 0x0060, 0x0, 2)
+AVR_MCU ("avr31",ARCH_AVR31, AVR_ERRATA_SKIP,
NULL,0x0060, 0x0, 2, 0x2)
+AVR_MCU ("atmega103",ARCH_AVR31, AVR_ERRATA_SKIP,
"__AVR_ATmega103__", 0x0060, 0x0, 2, 0x2)
/* Classic + MOVW + JMP/CALL. */
If at43usb320 is in the wrong multilib, then this should be handled
as separate issue / patch together with its own PR. Sorry for the
confusion. I just noticed that some fields don't match...
It is not even clear to me from the data sheet if avr3 is the correct
multilib or perhaps avr35 (if it supports MOVW) or even avr5 (if it
also has MUL) as there is no reference to the exact instruction set
-- Atmochip will know.
Moreover, such a change should be sync'ed with avr-libc as all
multilib stuff is hand-wired there: no use of --print-foo meta
information retrieval by avr-libc :-((
I filed PR78275 and https://savannah.nongnu.org/bugs/index.php?49565
for this one.
Thats better. I've attached the updated patch. If OK, could someone
commit please?
I'll try if I could find some more info for AT43USB320.
Regards,
Pitchumani
[RFC, tentative patch] Adjust cost for conversion expression
GCC inlines small functions if the code size after expansion is not
excedded.
For test case (inline.c, avr-gcc -Os -S inline.c) code size become higher if
'func2' is inlined. It happens because the CONVERT_EXPR/ NOP_EXPR are
considered
as zero cost expression.
Few conversions will cost additional instructions. For targets like AVR
it will cost considerably as it's register size is just one byte.
Attached the tentative patch that changes the CONVERT_EXPR/ NOP_EXPR cost
to 1 if the LHS is bigger than RHS and target's word_mode.
Is this Ok?
Would it be reasonable if cost evaluated as below instead of constant 1?
if (LHS PRECISION > RHS PRECISION)
cost = LHS_PRECISION / word_mode - 1
else
cost = 0
Built GCC for native with bootstrap enabled. No issues.
Regards,
Pitchumani
typedef unsigned int uint16_t __attribute__((__mode__(__HI__)));
typedef unsigned int uint32_t __attribute__ ((__mode__ (__SI__)));
char c = 12;
volatile long l = 1;
volatile uint32_t (*p)();
static uint16_t bar ()
{
return c;
}
static uint32_t foo ()
{
return ((uint32_t)bar() << 16 | bar());
}
__attribute__((noinline))
void func2 (uint32_t(*ptr)())
{
p = ptr();
l = foo();
}
void main ()
{
func2(foo);
}
diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
index 6899d2a..dbb305d 100644
--- a/gcc/tree-inline.c
+++ b/gcc/tree-inline.c
@@ -3867,19 +3867,28 @@ estimate_move_cost (tree type, bool ARG_UNUSED (speed_p))
static int
estimate_operator_cost (enum tree_code code, eni_weights *weights,
- tree op1 ATTRIBUTE_UNUSED, tree op2)
+ tree op1, tree op2)
{
+ unsigned int lhs_prec, rhs_prec;
switch (code)
{
/* These are "free" conversions, or their presumed cost
is folded into other operations. */
case RANGE_EXPR:
-CASE_CONVERT:
case COMPLEX_EXPR:
case PAREN_EXPR:
case VIEW_CONVERT_EXPR:
return 0;
-
+CASE_CONVERT:
+{
+ if (op1 && op2) {
+lhs_prec = element_precision (TREE_TYPE (op1));
+rhs_prec = element_precision (TREE_TYPE (op2));
+if ((lhs_prec > rhs_prec) && (lhs_prec > word_mode))
+ return 1;
+ }
+ return 0;
+}
/* Assign cost of 1 to usual operations.
??? We may consider mapping RTL costs to this. */
case COND_EXPR:
@@ -4026,6 +4035,7 @@ estimate_num_insns (gimple *stmt, eni_weights *weights)
{
unsigned cost, i;
enum gimple_code code = gimple_code (stmt);
+ enum tree_code rhs_code;
tree lhs;
tree rhs;
@@ -4064,9 +4074,17 @@ estimate_num_insns (gimple *stmt, eni_weights *weights)
if (gimple_assign_load_p (stmt))
cost += estimate_move_cost (TREE_TYPE (rhs), weights->time_based);
- cost += estimate_operator_cost (gimple_assign_rhs_code (stmt), weights,
+ rhs_code = gimple_assign_rhs_code (stmt);
+ if ((rhs_code == NOP_EXPR) || (rhs_code == CONVERT_EXPR))
+ {
+cost += estimate_operator_cost (rhs_code, weights,
+ gimple_assign_lhs (stmt),
+ gimple_assign_rhs1 (stmt));
+ }
+ else
+cost += estimate_operator_cost (rhs_code, weights,
gimple_assign_rhs1 (stmt),
- get_gimple_rhs_class (gimple_assign_rhs_code (stmt))
+ get_gimple_rhs_class (rhs_code)
== GIMPLE_BINARY_RHS
? gimple_assign_rhs2 (stmt) : NULL);
break;
Re: [RFC, tentative patch] Adjust cost for conversion expression
On Thursday 24 November 2016 04:54 PM, Richard Biener wrote:
On Thu, 24 Nov 2016, Pitchumani Sivanupandi wrote:
GCC inlines small functions if the code size after expansion is not excedded.
For test case (inline.c, avr-gcc -Os -S inline.c) code size become higher if
'func2' is inlined. It happens because the CONVERT_EXPR/ NOP_EXPR are
considered
as zero cost expression.
Few conversions will cost additional instructions. For targets like AVR
it will cost considerably as it's register size is just one byte.
Attached the tentative patch that changes the CONVERT_EXPR/ NOP_EXPR cost
to 1 if the LHS is bigger than RHS and target's word_mode.
Is this Ok?
Would it be reasonable if cost evaluated as below instead of constant 1?
if (LHS PRECISION > RHS PRECISION)
cost = LHS_PRECISION / word_mode - 1
else
cost = 0
Built GCC for native with bootstrap enabled. No issues.
I believe a better check would be tree_nop_conversion_p (). Thus
CASE_CONVERT:
return tree_nop_conversion_p (type, TREE_TYPE (op0)) ? 0 : 1;
note that
+ rhs_code = gimple_assign_rhs_code (stmt);
+ if ((rhs_code == NOP_EXPR) || (rhs_code == CONVERT_EXPR))
+ {
+cost += estimate_operator_cost (rhs_code, weights,
+ gimple_assign_lhs (stmt),
+ gimple_assign_rhs1 (stmt));
+ }
is super-ugly - please simply add the type of the expression as an
additional argument (see gimple_expr_type (), but the type of the
LHS would do as well).
Note that doing this change might require some re-tuning of
inliner params, but otherwise it's totally sensible.
Thanks. Attached the revised patch.
When reg-tested for x86_64 found following failures.
FAIL: gcc.dg/uninit-19.c
FAIL: gcc.dg/vect/vect-104.c
For uninit-19.c, index to dereference float array is converted to
long unsigned int and that is not tree_nop_conversion_p. This caused
function cost to increase and auto inline is rejected.
I think, this may be huge penalty for target like x86_64 which has rich ISA.
Any suggestions to avoid hitting such targets?
Regards,
Pitchumani
diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
index 6899d2a..e9f45be 100644
--- a/gcc/tree-inline.c
+++ b/gcc/tree-inline.c
@@ -3867,19 +3867,19 @@ estimate_move_cost (tree type, bool ARG_UNUSED (speed_p))
static int
estimate_operator_cost (enum tree_code code, eni_weights *weights,
- tree op1 ATTRIBUTE_UNUSED, tree op2)
+ tree op1, tree op2, tree op0)
{
switch (code)
{
/* These are "free" conversions, or their presumed cost
is folded into other operations. */
case RANGE_EXPR:
-CASE_CONVERT:
case COMPLEX_EXPR:
case PAREN_EXPR:
case VIEW_CONVERT_EXPR:
return 0;
-
+CASE_CONVERT:
+ return tree_nop_conversion_p (op0, TREE_TYPE (op1)) ? 0 : 1;
/* Assign cost of 1 to usual operations.
??? We may consider mapping RTL costs to this. */
case COND_EXPR:
@@ -4068,13 +4068,14 @@ estimate_num_insns (gimple *stmt, eni_weights *weights)
gimple_assign_rhs1 (stmt),
get_gimple_rhs_class (gimple_assign_rhs_code (stmt))
== GIMPLE_BINARY_RHS
- ? gimple_assign_rhs2 (stmt) : NULL);
+ ? gimple_assign_rhs2 (stmt) : NULL,
+ gimple_expr_type (stmt));
break;
case GIMPLE_COND:
cost = 1 + estimate_operator_cost (gimple_cond_code (stmt), weights,
gimple_op (stmt, 0),
- gimple_op (stmt, 1));
+ gimple_op (stmt, 1), gimple_expr_type (stmt));
break;
case GIMPLE_SWITCH:
@@ -4129,7 +4130,7 @@ estimate_num_insns (gimple *stmt, eni_weights *weights)
)))
return estimate_operator_cost
(MULT_EXPR, weights, gimple_call_arg (stmt, 0),
- gimple_call_arg (stmt, 0));
+ gimple_call_arg (stmt, 0), gimple_expr_type (stmt));
break;
default:
Re: Ping: Re: [patch, avr] Add flash size to device info and make wrap around default
On Saturday 26 November 2016 12:11 AM, Denis Chertykov wrote:
I'm sorry for delay.
I have a problem with the patch:
(Stripping trailing CRs from patch; use --binary to disable.)
patching file avr-arch.h
(Stripping trailing CRs from patch; use --binary to disable.)
patching file avr-devices.c
(Stripping trailing CRs from patch; use --binary to disable.)
patching file avr-mcus.def
Hunk #1 FAILED at 62.
1 out of 1 hunk FAILED -- saving rejects to file avr-mcus.def.rej
(Stripping trailing CRs from patch; use --binary to disable.)
patching file gen-avr-mmcu-specs.c
Hunk #1 succeeded at 215 (offset 5 lines).
(Stripping trailing CRs from patch; use --binary to disable.)
patching file specs.h
Hunk #1 succeeded at 58 (offset 1 line).
Hunk #2 succeeded at 66 (offset 1 line).
There are changes in avr-mcus.def after this patch is submitted.
Now, I have incorporated the changes and attached the resolved patch.
Regards,
Pitchumani
gcc/ChangeLog
2016-11-09 Pitchumani Sivanupandi <pitchuman...@atmel.com>
* config/avr/avr-arch.h (avr_mcu_t): Add flash_size member.
* config/avr/avr-devices.c(avr_mcu_types): Add flash size info.
* config/avr/avr-mcu.def: Likewise.
* config/avr/gen-avr-mmcu-specs.c (print_mcu): Remove hard-coded
prefix
check to find wrap-around value, instead use MCU flash size. For 8k
flash
devices, update link_pmem_wrap spec string to add
--pmem-wrap-around=8k.
* config/avr/specs.h: Remove link_pmem_wrap from LINK_RELAX_SPEC and
add to linker specs (LINK_SPEC) directly.
diff --git a/gcc/config/avr/avr-arch.h b/gcc/config/avr/avr-arch.h
index a740a15..e6a2d75 100644
--- a/gcc/config/avr/avr-arch.h
+++ b/gcc/config/avr/avr-arch.h
@@ -122,6 +122,9 @@ typedef struct
/* Number of 64k segments in the flash. */
int n_flash;
+
+ /* Flash size in bytes. */
+ int flash_size;
} avr_mcu_t;
/* AVR device specific features.
diff --git a/gcc/config/avr/avr-devices.c b/gcc/config/avr/avr-devices.c
index 7d13ba4..cef3b9a 100644
--- a/gcc/config/avr/avr-devices.c
+++ b/gcc/config/avr/avr-devices.c
@@ -111,12 +111,12 @@ avr_texinfo[] =
const avr_mcu_t
avr_mcu_types[] =
{
-#define AVR_MCU(NAME, ARCH, DEV_ATTRIBUTE, MACRO, DATA_SEC, TEXT_SEC, N_FLASH)\
- { NAME, ARCH, DEV_ATTRIBUTE, MACRO, DATA_SEC, TEXT_SEC, N_FLASH },
+#define AVR_MCU(NAME, ARCH, DEV_ATTRIBUTE, MACRO, DATA_SEC, TEXT_SEC, N_FLASH, FLASH_SIZE)\
+ { NAME, ARCH, DEV_ATTRIBUTE, MACRO, DATA_SEC, TEXT_SEC, N_FLASH, FLASH_SIZE },
#include "avr-mcus.def"
#undef AVR_MCU
/* End of list. */
- { NULL, ARCH_UNKNOWN, AVR_ISA_NONE, NULL, 0, 0, 0 }
+ { NULL, ARCH_UNKNOWN, AVR_ISA_NONE, NULL, 0, 0, 0, 0 }
};
diff --git a/gcc/config/avr/avr-mcus.def b/gcc/config/avr/avr-mcus.def
index e5b4cda..4008741 100644
--- a/gcc/config/avr/avr-mcus.def
+++ b/gcc/config/avr/avr-mcus.def
@@ -62,295 +62,298 @@
N_FLASH Number of 64 KiB flash segments, rounded up. The default
value for -mn-flash=.
+ FLASH_SIZEFlash size in bytes.
+
"avr2" must be first for the "0" default to work as intended. */
/* Classic, <= 8K. */
-AVR_MCU ("avr2", ARCH_AVR2, AVR_ERRATA_SKIP, NULL, 0x0060, 0x0, 6)
-AVR_MCU ("at90s2313",ARCH_AVR2, AVR_SHORT_SP, "__AVR_AT90S2313__", 0x0060, 0x0, 1)
-AVR_MCU ("at90s2323",ARCH_AVR2, AVR_SHORT_SP, "__AVR_AT90S2323__", 0x0060, 0x0, 1)
-AVR_MCU ("at90s2333",ARCH_AVR2, AVR_SHORT_SP, "__AVR_AT90S2333__", 0x0060, 0x0, 1)
-AVR_MCU ("at90s2343",ARCH_AVR2, AVR_SHORT_SP, "__AVR_AT90S2343__", 0x0060, 0x0, 1)
-AVR_MCU ("attiny22", ARCH_AVR2, AVR_SHORT_SP, "__AVR_ATtiny22__", 0x0060, 0x0, 1)
-AVR_MCU ("attiny26", ARCH_AVR2, AVR_SHORT_SP, "__AVR_ATtiny26__", 0x0060, 0x0, 1)
-AVR_MCU ("at90s4414",ARCH_AVR2, AVR_ISA_NONE, "__AVR_AT90S4414__", 0x0060, 0x0, 1)
-AVR_MCU ("at90s4433",ARCH_AVR2, AVR_SHORT_SP, "__AVR_AT90S4433__", 0x0060, 0x0, 1)
-AVR_MCU ("at90s4434",ARCH_AVR2, AVR_ISA_NONE, "__AVR_AT90S4434__", 0x0060, 0x0, 1)
-AVR_MCU ("at90s8515",ARCH_AVR2, AVR_ERRATA_SKIP, "__AVR_AT90S8515__", 0x0060, 0x0, 1)
-AVR_MCU ("at90c8534",ARCH_AVR2, AVR_ISA_NONE, "__AVR_AT90C8534__", 0x0060, 0x0, 1)
-AVR_MCU ("at90s8535",ARCH_AVR2, AVR_ISA_NONE, "__AVR_AT90S8535__", 0x0060, 0x0, 1)
+AVR_MCU ("avr2", ARCH_AVR2, AVR_ERRATA_SKIP, NULL, 0x0060, 0x0, 6, 0x2000)
+
+AVR_MCU ("at90s2313",ARCH_AVR2, AVR_SHORT_SP, "__AVR_AT90S2313__", 0x0060, 0x0, 1, 0x800)
+AVR_MCU ("at90s
Ping: Re: [patch, avr] Add flash size to device info and make wrap around default
Ping!
On Monday 14 November 2016 07:03 PM, Pitchumani Sivanupandi wrote:
Ping!
On Thursday 10 November 2016 01:53 PM, Pitchumani Sivanupandi wrote:
On Wednesday 09 November 2016 08:05 PM, Georg-Johann Lay wrote:
On 09.11.2016 10:14, Pitchumani Sivanupandi wrote:
On Tuesday 08 November 2016 02:57 PM, Georg-Johann Lay wrote:
On 08.11.2016 08:08, Pitchumani Sivanupandi wrote:
I have updated patch to include the flash size as well. Took that
info from
device headers (it was fed into crt's device information note
section also).
The new option would render -mn-flash superfluous, but we should
keep it for
backward compatibility.
Ok.
Shouldn't link_pmem_wrap then be removed from link_relax, i.e. from
LINK_RELAX_SPEC? And what happens if relaxation is off?
Yes. Removed link_pmem_wrap from link_relax.
Disabling relaxation doesn't change -mpmem-wrap-around behavior.
flashsize-and-wrap-around.patch
diff --git a/gcc/config/avr/avr-mcus.def
b/gcc/config/avr/avr-mcus.def
index 6bcc6ff..9d4aa1a 100644
/*
/* Classic, > 8K, <= 64K. */
-AVR_MCU ("avr3", ARCH_AVR3, AVR_ISA_NONE,
NULL,0x0060, 0x0, 1)
-AVR_MCU ("at43usb355", ARCH_AVR3, AVR_ISA_NONE,
"__AVR_AT43USB355__",0x0060, 0x0, 1)
-AVR_MCU ("at76c711", ARCH_AVR3, AVR_ISA_NONE,
"__AVR_AT76C711__", 0x0060, 0x0, 1)
+AVR_MCU ("avr3", ARCH_AVR3, AVR_ISA_NONE,
NULL,0x0060, 0x0, 1, 0x6000)
+AVR_MCU ("at43usb355", ARCH_AVR3, AVR_ISA_NONE,
"__AVR_AT43USB355__",0x0060, 0x0, 1, 0x6000)
+AVR_MCU ("at76c711", ARCH_AVR3, AVR_ISA_NONE,
"__AVR_AT76C711__", 0x0060, 0x0, 1, 0x4000)
+AVR_MCU ("at43usb320", ARCH_AVR3, AVR_ISA_NONE,
"__AVR_AT43USB320__",0x0060, 0x0, 1, 0x1)
/* Classic, == 128K. */
-AVR_MCU ("avr31",ARCH_AVR31, AVR_ERRATA_SKIP,
NULL,0x0060, 0x0, 2)
-AVR_MCU ("atmega103",ARCH_AVR31, AVR_ERRATA_SKIP,
"__AVR_ATmega103__", 0x0060, 0x0, 2)
-AVR_MCU ("at43usb320", ARCH_AVR31, AVR_ISA_NONE,
"__AVR_AT43USB320__", 0x0060, 0x0, 2)
+AVR_MCU ("avr31",ARCH_AVR31, AVR_ERRATA_SKIP,
NULL,0x0060, 0x0, 2, 0x2)
+AVR_MCU ("atmega103",ARCH_AVR31, AVR_ERRATA_SKIP,
"__AVR_ATmega103__", 0x0060, 0x0, 2, 0x2)
/* Classic + MOVW + JMP/CALL. */
If at43usb320 is in the wrong multilib, then this should be handled
as separate issue / patch together with its own PR. Sorry for the
confusion. I just noticed that some fields don't match...
It is not even clear to me from the data sheet if avr3 is the
correct multilib or perhaps avr35 (if it supports MOVW) or even avr5
(if it also has MUL) as there is no reference to the exact
instruction set -- Atmochip will know.
Moreover, such a change should be sync'ed with avr-libc as all
multilib stuff is hand-wired there: no use of --print-foo meta
information retrieval by avr-libc :-((
I filed PR78275 and https://savannah.nongnu.org/bugs/index.php?49565
for this one.
Thats better. I've attached the updated patch. If OK, could someone
commit please?
I'll try if I could find some more info for AT43USB320.
Regards,
Pitchumani
[patch, avr] Make pmem-wrap-around option as default
Most of the AVR's 8k memorydevices have only rjmp instruction, not jmp.
So, it
is important to wrap around jump destination to check if it can reach
backwards.
Currently link specs passes --pmem-wrap-around=xxK when mrelax and
mpmem-wrap-around options are enabled. Attached patch changes the specs
so that
option --pmem-wrap-around=8K is passed for 8k memory devices if
-mno-pmem-wrap-around is not enabled.
If OK, could someone commit please?
Note: Currently 8k devices are identified based on name prefix. We are
working
on alternative method to incorporate flash memory size.
Regards,
Pitchumani
gcc/ChangeLog
2016-11-03 Pitchumani Sivanupandi <pitchuman...@atmel.com>
* config/avr/gen-avr-mmcu-specs.c (print_mcu): Update
link_pmem_wrap spec
string to add linker option --pmem-wrap-around=8k.
* config/avr/specs.h: Add link_pmem_wrap to linker specs (LINK_SPEC).
diff --git a/gcc/config/avr/gen-avr-mmcu-specs.c b/gcc/config/avr/gen-avr-mmcu-specs.c
index 7fca756..41dbd80 100644
--- a/gcc/config/avr/gen-avr-mmcu-specs.c
+++ b/gcc/config/avr/gen-avr-mmcu-specs.c
@@ -220,7 +220,9 @@ print_mcu (const avr_mcu_t *mcu)
: 0;
fprintf (f, "*link_pmem_wrap:\n");
- if (wrap_k)
+ if (wrap_k == 8)
+fprintf (f, "\t%%{!mno-pmem-wrap-around: --pmem-wrap-around=8k}");
+ else if (wrap_k > 8)
fprintf (f, "\t%%{mpmem-wrap-around: --pmem-wrap-around=%dk}", wrap_k);
fprintf (f, "\n\n");
diff --git a/gcc/config/avr/specs.h b/gcc/config/avr/specs.h
index 52763cc..fbf0ce6 100644
--- a/gcc/config/avr/specs.h
+++ b/gcc/config/avr/specs.h
@@ -65,6 +65,7 @@ along with GCC; see the file COPYING3. If not see
"%(link_data_start) " \
"%(link_text_start) " \
"%(link_relax) " \
+ "%(link_pmem_wrap) " \
"%{shared:%eshared is not supported} "
#undef LIB_SPEC
Re: [patch, avr, pr71676 and pr71678] Issues with casesi expand
On Monday 26 September 2016 08:19 PM, Georg-Johann Lay wrote: On 26.09.2016 15:19, Pitchumani Sivanupandi wrote: Attached patch for PR71676 and PR71678. PR71676 is for AVR target that generates wrong code when switch case index is more than 16 bits. Switch case index of larger than SImode are checked for out of range before 'casesi' expand. RTL expand of casesi gets index as SImode, but index is compared in HImode and ignores upper 16bits. Attached patch changes the expansion for casesi to make the index comparison in SImode and code generation accordingly. PR71678 is ICE because below pattern in 'casesi' is not recognized. (set (reg:HI 47) (minus:HI (subreg:HI (subreg:SI (reg:DI 44) 0) 0) (reg:HI 45))) Fix of PR71676 avoids the above pattern as it changes the comparison to SImode. But this means that all comparisons are now performed in SImode which is a great performance loss for most programs which will switch on 16-bit values. IMO we need a less intrusive (w.r.t. performance) approach. Yes. I tried to split 'casesi' into several based on case values so that compare is done in less expensive modes (i.e. QI or HI). In few cases it is not possible without SImode subtract/ compare. Pattern casesi will have index in SI mode. So, out of range checks will be expensive as most common uses (in AVR) of case values will be in QI/HI mode. e.g. if case values in QI range if upper three bytes index is set goto out_of_range offset = index - lower_bound (QImode) if offset > case_range (QImode) goto out_of_range goto jump_table + offset else if case values in HI range if index[2,3] is set goto out_of_range offset = index - lower_bound (HImode) if offset > case_range (HImode) goto out_of_range goto jump_table + offset This modification will not work for the negative index values. Because code to check upper bytes of index will be expensive than the SImode subtract/ compare. So, I'm trying to update fix to have SImode subtract/ compare if the case values include negative integers. For, others will try to optimize as mentioned above. Is that approach OK? Alternatively we can have flags to generate shorter code for 'casesi' using HImode subtract/ compare. But correctness is not guaranteed (PR71676). Regards, Pitchumani
Re: Ping: Re: [patch, avr] Add flash size to device info and make wrap around default
On Tuesday 29 November 2016 10:06 PM, Denis Chertykov wrote: 2016-11-28 10:17 GMT+03:00 Pitchumani Sivanupandi <pitchumani.sivanupa...@microchip.com>: On Saturday 26 November 2016 12:11 AM, Denis Chertykov wrote: I'm sorry for delay. I have a problem with the patch: (Stripping trailing CRs from patch; use --binary to disable.) patching file avr-arch.h (Stripping trailing CRs from patch; use --binary to disable.) patching file avr-devices.c (Stripping trailing CRs from patch; use --binary to disable.) patching file avr-mcus.def Hunk #1 FAILED at 62. 1 out of 1 hunk FAILED -- saving rejects to file avr-mcus.def.rej (Stripping trailing CRs from patch; use --binary to disable.) patching file gen-avr-mmcu-specs.c Hunk #1 succeeded at 215 (offset 5 lines). (Stripping trailing CRs from patch; use --binary to disable.) patching file specs.h Hunk #1 succeeded at 58 (offset 1 line). Hunk #2 succeeded at 66 (offset 1 line). There are changes in avr-mcus.def after this patch is submitted. Now, I have incorporated the changes and attached the resolved patch. Regards, Pitchumani gcc/ChangeLog 2016-11-09 Pitchumani Sivanupandi <pitchuman...@atmel.com> * config/avr/avr-arch.h (avr_mcu_t): Add flash_size member. * config/avr/avr-devices.c(avr_mcu_types): Add flash size info. * config/avr/avr-mcu.def: Likewise. * config/avr/gen-avr-mmcu-specs.c (print_mcu): Remove hard-coded prefix check to find wrap-around value, instead use MCU flash size. For 8k flash devices, update link_pmem_wrap spec string to add --pmem-wrap-around=8k. * config/avr/specs.h: Remove link_pmem_wrap from LINK_RELAX_SPEC and add to linker specs (LINK_SPEC) directly. Committed. It looks like only avr-mcus.def and ChangeLog are committed. Without the other changes trunk build is broken. Regards, Pitchumani
Re: [patch,avr] Add support for devices with flash accessible by LD.
On Friday 09 June 2017 03:59 PM, Georg-Johann Lay wrote:
Hi,
This patch adds support for devices that can access flash memory
by LD* instructions, hence there is no need to put .rodata in RAM.
The default linker script for the new multilib versions already
supports this feature, it's similar to avrtiny, cf.
https://sourceware.org/PR21472
This patch does the following:
* Add multilib variants avrxmega3 and avrxmega3/short-calls.
* Add new option -mshort-calls for multilib selection between
devices with <= 8KiB flash and > 8KiB flash.
* Add specs handling for -mshort-calls: The compiler knows
if this option is needed or not appropriate (similar to -msp8).
* Add new ISA feature AVR_ISA_RCALL for multilib selection
via -mshort-calls.
* Add a new row to architecture description that contains the
start address of flash memory in the RAM address range.
(The actual value is not needed).
* For devices with flash in RAM space, don't let .rodata
objects trigger need for __do_copy_data.
* Add some devices.
* Add configure test for Binutils PR21472.
* Adjust documentation etc.
Even the smaller devices with flash <= 8KiB support JMP+CALL,
but we get better code by assuming RJMP+RCALL: Jump tables
are more efficient and insn length computation is more exact
(CALL -> RCALL relaxation would need -mrelax and insn size
would still be 4 bytes).
Moreover, avr-libc uses __AVR_HAVE_JMP_CALL__ to determine
vector entry size, and libgcc uses that macro for flash size
estimation.
Hi Johann,
+ AVR_ISA_RCALL = 0x10 /* Use RJMP / RCALL even though RJMP / RCALL
+are available (-mshort-calls). */
I think you meant "even though JMP/ CALL are avariable".
+AVR_MCU ("attiny817",ARCH_AVRXMEGA3, AVR_ISA_RCALL,
"__AVR_ATtiny817__", 0x3e00, 0x0, 0x2000)
+AVR_MCU ("attiny1616", ARCH_AVRXMEGA3, AVR_ISA_NONE,
"__AVR_ATtiny1616__", 0x3800, 0x0, 0x4000)
Add attiny1614 device.
AVR_MCU ("attiny1614", ARCH_AVRXMEGA3, AVR_ISA_NONE,
"__AVR_ATtiny1614__", 0x3800, 0x0, 0x4000)
Regards,
Pitchumani
