On Mon, May 12, 2014 at 01:19:37PM +0200, Georg-Johann Lay wrote:
> Am 04/18/2014 11:52 AM, schrieb Senthil Kumar Selvaraj:
> >
> >On Sat, Apr 12, 2014 at 06:36:01PM +0200, Georg-Johann Lay wrote:
> >>Senthil Kumar Selvaraj schrieb:
> >>>This patch modifies AVR target's ASM spec to pass -mlink-relax to the
> >>>assembler if -mrelax is passed to the compiler driver. This was already
> >>>being passed on to the linker, this patch merely makes the assembler
> >>>also aware of it.
> >>>
> >>>The corresponding patch in binutils to handle the -mlink-relax patch is
> >>>already committed in the binutils repo. I'm not sure how to manage a
> >>>running a newer gcc with an older version of binutils though - how is this
> >>>generally handled?
> >>
> >>The right place is gcc/configure.ac and have a macro defined depending on
> >>whether gas supports -mlink-relax.
> >>
> >>
> >>Same should be done for -mrmw, IMO, for similar reasons, e.g. something like
> >>
> >>case "$target" in
> >> ...
> >> avr-*-*)
> >> ...
> >> gcc_GAS_CHECK_FEATURE([-mrmw option], gcc_cv_as_avr_mrmw,,
> >> [-mrmw], [.text],,
> >> [AC_DEFINE(HAVE_AS_AVR_MRMW_OPTION, 1,
> >> [Define if your assembler supports -mrmw option.])])
> >>
> >>or
> >>
> >> gcc_GAS_CHECK_FEATURE([-mrmw option], gcc_cv_as_avr_mrmw,,
> >> [-mrmw], [.text],,,)
> >> if test x$gcc_cv_as_avr_mrmw = xyes; then
> >> AC_DEFINE(HAVE_AS_AVR_MRMW_OPTION, 1,
> >> [Define if your assembler supports the -mrmw option.])
> >>
> >
> >Thanks Johann. The below patch adds the configury check for -mlink-relax,
> >along with the change to ASM_SPEC to propagate the -mrelax flag. I
> >modified the original patch a bit to make it easier to handle
> >conditional additions to SPECs (inspired by the sparc target).
> >
> >>
> >>However, the gcc-4_9-branch has already been created...
> >
> >Yes, but I figured it would be useful anyway - if this eventually gets
> >backported to 4_9, for example.
> >
> >If the below patch looks ok, could someone commit please? I don't have
> >commit access.
> >
> >Regards
> >Senthil
> >
> >gcc/ChangeLog
> >
> >2014-04-18 Senthil Kumar Selvaraj <senthil_kumar.selva...@atmel.com>
> >
> > * config/avr/avr.h: Pass on mlink-relax to assembler.
> > * configure.ac: Test for mlink-relax support in assembler.
> > * configure: Regenerate.
> >
> >diff --git gcc/config/avr/avr.h gcc/config/avr/avr.h
> >index 78434ec..b4e3eb1 100644
> >--- gcc/config/avr/avr.h
> >+++ gcc/config/avr/avr.h
> >@@ -512,7 +512,28 @@ extern const char *avr_device_to_sp8 (int argc, const
> >char **argv);
> > %{!fenforce-eh-specs:-fno-enforce-eh-specs} \
> > %{!fexceptions:-fno-exceptions}"
> >
> >-#define ASM_SPEC "%:device_to_as(%{mmcu=*:%*}) "
> >+#ifdef HAVE_AS_RELAX_OPTION
> >+#define ASM_RELAX_SPEC "%{mrelax:-mlink-relax}"
> >+#else
> >+#define ASM_RELAX_SPEC ""
> >+#endif
> >+
> >+#define ASM_SPEC "%:device_to_as(%{mmcu=*:%*})\
> >+%(asm_relax)"
> >+
> >+/* This macro defines names of additional specifications to put in the specs
> >+ that can be used in various specifications like CC1_SPEC. Its definition
> >+ is an initializer with a subgrouping for each command option.
> >+
> >+ Each subgrouping contains a string constant, that defines the
> >+ specification name, and a string constant that used by the GCC driver
> >+ program.
> >+
> >+ Do not define this macro if it does not need to do anything. */
> >+
> >+#define EXTRA_SPECS \
> >+ { "asm_relax", ASM_RELAX_SPEC }
> >+
>
> Hi, wouldn't it be easier to add just a line to driver-avr.c:avr_device_to_as
> ?
Well, I couldn't figure out how to do it without passing in the nested spec and
then do argument checking inside avr_device_to_as. Something like
#define ASM_SPEC "%:device_to_as(%{mmcu=*:%*} %{mrelax:-mlink-relax})"
and then handle argc==0, 1 and 2 cases by strcmp'ing against
-mlink-relax if HAVE_AVR_AS_LINK_RELAX_OPTION.
Did I miss something?
>
> >
> > #define LINK_SPEC "\
> > %{mrelax:--relax\
> >diff --git gcc/configure gcc/configure
> >index bfb1525..7815038 100755
> >--- gcc/configure
> >+++ gcc/configure
> >@@ -24142,6 +24142,39 @@ $as_echo "#define HAVE_AS_JSRDIRECT_RELOCS 1"
> >>>confdefs.h
> > fi
> > ;;
> >
> >+ avr-*-*)
> >+ { $as_echo "$as_me:${as_lineno-$LINENO}: checking assembler for
> >-mlink-relax option" >&5
> >+$as_echo_n "checking assembler for -mlink-relax option... " >&6; }
> >+if test "${gcc_cv_as_avr_relax+set}" = set; then :
> >+ $as_echo_n "(cached) " >&6
> >+else
> >+ gcc_cv_as_avr_relax=no
> >+ if test x$gcc_cv_as != x; then
> >+ $as_echo '.text' > conftest.s
> >+ if { ac_try='$gcc_cv_as $gcc_cv_as_flags -mlink-relax -o conftest.o
> >conftest.s >&5'
> >+ { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
> >+ (eval $ac_try) 2>&5
> >+ ac_status=$?
> >+ $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
> >+ test $ac_status = 0; }; }
> >+ then
> >+ gcc_cv_as_avr_relax=yes
> >+ else
> >+ echo "configure: failed program was" >&5
> >+ cat conftest.s >&5
> >+ fi
> >+ rm -f conftest.o conftest.s
> >+ fi
> >+fi
> >+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $gcc_cv_as_avr_relax" >&5
> >+$as_echo "$gcc_cv_as_avr_relax" >&6; }
> >+if test $gcc_cv_as_avr_relax = yes; then
> >+
> >+$as_echo "#define HAVE_AS_RELAX_OPTION 1" >>confdefs.h
> >+
> >+fi
> >+ ;;
> >+
> > cris-*-*)
> > { $as_echo "$as_me:${as_lineno-$LINENO}: checking assembler for
> > -no-mul-bug-abort option" >&5
> > $as_echo_n "checking assembler for -no-mul-bug-abort option... " >&6; }
> >diff --git gcc/configure.ac gcc/configure.ac
> >index d7cae6c..cfa862d 100644
> >--- gcc/configure.ac
> >+++ gcc/configure.ac
> >@@ -3579,6 +3579,13 @@ case "$target" in
> > [Define if your assembler supports the lituse_jsrdirect relocation.])])
> > ;;
> >
> >+ avr-*-*)
> >+ gcc_GAS_CHECK_FEATURE([-mlink-relax option], gcc_cv_as_avr_relax,,
> >+ [-mlink-relax], [.text],,
> >+ [AC_DEFINE(HAVE_AS_RELAX_OPTION, 1,
>
> IMO this is very confusing naming
>
> - The gas option is not "relax" but "link-relax"
> - The option is avr specific, thus in order to avoid naming clashes
> the variable(s) should be named something like
>
> HAVE_AS_AVR_LINK_RELAX_OPTION
Ok, I'll resend the patch with the renamed define - I was following
whatever the sparc target was doing.
Regards
Senthil
