Ping! > -----Original Message----- > From: avr-gcc-list-bounces+pitchumani.s=atmel....@nongnu.org [mailto:avr- > gcc-list-bounces+pitchumani.s=atmel....@nongnu.org] On Behalf Of S, > Pitchumani > Sent: Saturday, March 15, 2014 9:20 PM > To: Georg-Johann Lay > Cc: avr-gcc-list@nongnu.org > Subject: Re: [avr-gcc-list] Device specific ISA support in AVR > > > -----Original Message----- > > From: Georg-Johann Lay [mailto:a...@gjlay.de] > > Sent: Thursday, March 13, 2014 7:43 PM > > To: S, Pitchumani > > Cc: avr-gcc-list@nongnu.org > > Subject: Re: [avr-gcc-list] Device specific ISA support in AVR > > > > Am 03/12/2014 06:59 PM, schrieb S, Pitchumani: > > > > >>> Please review the patches and comment. > > >> > > >> Hi Pitchumani, > > >> > > >> some remarks on the work: > > >> > > >> 1) It might be useful to builtin-define macros so that user code can > > test > > >> for > > >> availability of these instructions, similar to __AVR_ERRATA_SKIP__ or > > >> __AVR_HAVE_MUL__. That way users can depend on the macro when > > implementing > > >> their inline assembler that might use RMW instructions. This macro > > should > > >> then > > >> be documented with the other macros. > > >> > > >> 2) Binutils' documentation should spell out "RMW" as read-modify- > store. > > >> It's > > >> always easier to remember an option if the resolution of some cryptic > > >> letter > > >> combination is known. > > >> > > >> 3) There are also MCU-specific ISA-like features in avr-mcus.def: > > >> > > >> *) ERRATA_SKIP (ISA with broken CPSE, SBRC/S, SBIC/S wrapping 32-bit > > insn) > > >> > > >> *) SHORT_SP (MCU has no SPH special function register) > > >> > > >> Maybe these 3 ISA properties can be merged into one field with > > respective > > >> flags. That way avr-mcus.def would be easier to read and the number > of > > >> fields > > >> would reduce. > > >> > > >> 4) I'd prefer ISA in front of the feature, not as suffix, i.e. > > >> > > >> AVR_ISA_RMW, AVR_ISA_SKIP_BUG, AVR_ISR_SP8, etc. instead of > > AVR_RMW_ISA. > > >> > > >> 5) There is already a Binutils PR, cf. PR15043 (with differently > named > > >> options, > > >> though). > > >> > > >> http://sourceware.org/PR15043 > > >> > > >> 6) The GCC release notes must mention that at least Binutils version > > xyz > > >> is needed. > > >> > > >> Typically, there is some time margin until GCC might assume that > > specific > > >> features are available in binutils. This is beacuse your work is not > a > > >> pure > > >> extension but affects existing devices. > > >> > > >> 7) Don't you skip the test conditions that you want to test in the > new > > GCC > > >> testsuite program? I.e. you skip -mmcu=atxmega32a4u. > > > > > > Hi Johann, > > > > > > I have updated avr-gcc patch as per your comments. Please review. > > > > Again some notes :-) > > > > The GNU coding rules limit lines to 80 characters; some lines in your > > changes > > are longer. (There are some files like avr-mcus.def where long lines are > > in > > order and accepted). > > > > Maybe the easiest way is to rename lengthy component name > > "dev_specific_feature" to "isa" which is clear enough, IMO. > > > > > > The test case is still not in order because of > > > > +/* { dg-options "-mmcu=atxmega128b1" } */ > > > > which collides with -mmcu= when the tests are run for a different > target. > > > > But there's no need for -mmcu= at all, we have __AVR_ISA_RMW__ so you > can > > factor out the asms by means of #ifdef __AVR_ISA_RMW__ :-) > > > > Johann > > > > > gcc/ChangeLog > > > 2014-03-12 Pitchumani Sivanupandi <pitchuman...@atmel.com> > > > > > > * config/avr/avr-arch.h (avr_mcu_t): Add dev_specific_feature > field > > > to have device specific ISA/ feature information. Remove short_sp > > > and errata_skip fields. > > > Add avr_device_specific_features enum to have device specific > info. > > > * config/avr/avr-c.c: use dev_specific_feature to check > > errata_skip. > > > > Function names are missing. > > > > > Add __AVR_ISA_RMW__ builtin macro if RMW ISA available. > > > * config/avr/avr-devices.c (avr_mcu_types): Update AVR_MCU macro > > for > > > updated device specific info. > > > * config/avr/avr-mcus.def: Merge device specific details to > > > dev_specific_feature field. > > > * config/avr/avr.c (avr_2word_insn_p): use dev_specific_feature > > field > > > to check errata_skip. > > > * config/avr/avr.h: same for short sp info. > > > > What objects / macros are affected? > > > > > * config/avr/driver-avr.c (avr_device_to_as): Pass -mrmw option > to > > > assembler if RMW isa supported by current device. > > > * config/avr/genmultilib.awk: Update as device info structure > > changed. > > > * doc/invoke.texi: Add info for __AVR_ISA_RMW__ builtin macro > > > > > > gcc/testsuite/ChangeLog > > > 2014-03-12 Pitchumani Sivanupandi <pitchuman...@atmel.com> > > > > > > * gcc.target/avr/ dev-specific-rmw.c: New test. > > > > Superfluous blank in the file name. > > Hi Johann, > > Thanks for the review :) > I have updated the patch and ChangeLog. > > Regards, > Pitchumani > > gcc/ChangeLog > > 2014-03-12 Pitchumani Sivanupandi <pitchuman...@atmel.com> > > * config/avr/avr-arch.h (avr_mcu_t): Add dev_attribute field to have > device > specific ISA/ feature information. Remove short_sp and errata_skip > fields. > Add avr_device_specific_features enum to have device specific info. > * config/avr/avr-c.c (avr_cpu_cpp_builtins): use dev_attribute to > check > errata_skip. Add __AVR_ISA_RMW__ builtin macro if RMW ISA available. > * config/avr/avr-devices.c (avr_mcu_types): Update AVR_MCU macro for > updated device specific info. > * config/avr/avr-mcus.def: Merge device specific details to > dev_attribute field. > * config/avr/avr.c (avr_2word_insn_p): use dev_attribute field to > check > errata_skip. > * config/avr/avr.h (AVR_HAVE_8BIT_SP): same for short sp info. > * config/avr/driver-avr.c (avr_device_to_as): Pass -mrmw option to > assembler if RMW isa supported by current device. > * config/avr/genmultilib.awk: Update as device info structure changed. > * doc/invoke.texi: Add info for __AVR_ISA_RMW__ builtin macro > > gcc/testsuite/ChangeLog > > 2014-03-12 Pitchumani Sivanupandi <pitchuman...@atmel.com> > > * gcc.target/avr/dev-specific-rmw.c: New test.
_______________________________________________ AVR-GCC-list mailing list AVR-GCC-list@nongnu.org https://lists.nongnu.org/mailman/listinfo/avr-gcc-list
