This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [Patch, avr] Propagate -mrelax gcc driver flag to assembler


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.selvaraj@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 ?


  #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

+		[Define if your assembler supports -mlink-relax option.])])
+  ;;
+
    cris-*-*)
      gcc_GAS_CHECK_FEATURE([-no-mul-bug-abort option],
        gcc_cv_as_cris_no_mul_bug,[2,15,91],


Johann


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]