Re: [Patch ARM/ configury] Add fall-back check for gnu_unique_object
On Thu, 21 Jun 2012, Ramana Radhakrishnan wrote: > On 10 April 2012 10:11, Ramana Radhakrishnan > wrote: > >>> The patch with correct configure output is ok. > >> > >> Thanks - this is what I committed. > > > > Is this something that can be considered for backporting to release > > branches ? This patch technically doesn't fix a regression but brings > > in line behaviour of the normal bootstrap for %gnu_unique_object ? > > Ping - Is this ok to backport to the 4.7 and 4.6 branches ? I'd say yes. Thanks, Richard.
Re: [Patch ARM/ configury] Add fall-back check for gnu_unique_object
On 10 April 2012 10:11, Ramana Radhakrishnan wrote: >>> The patch with correct configure output is ok. >> >> Thanks - this is what I committed. > > Is this something that can be considered for backporting to release > branches ? This patch technically doesn't fix a regression but brings > in line behaviour of the normal bootstrap for %gnu_unique_object ? Ping - Is this ok to backport to the 4.7 and 4.6 branches ? Ramana
Re: [Patch ARM/ configury] Add fall-back check for gnu_unique_object
>> The patch with correct configure output is ok. > > Thanks - this is what I committed. Is this something that can be considered for backporting to release branches ? This patch technically doesn't fix a regression but brings in line behaviour of the normal bootstrap for %gnu_unique_object ? http://gcc.gnu.org/ml/gcc-patches/2012-03/msg01046.html regards, Ramana > > Ramana > > > > > > 2012-03-15 Ramana Radhakrishnan > > * config.gcc (target_type_format_char): New. Document it. Set it for > arm*-*-* . > * configure.ac (gnu_unique_option): Use target_type_format_char in > test. > Comment rationale. > * configure: Regenerate . > > > > Ramana > > >> >> Paolo
Re: [Patch ARM/ configury] Add fall-back check for gnu_unique_object
On 14 March 2012 15:52, Paolo Bonzini wrote: > Il 14/03/2012 16:37, Ramana Radhakrishnan ha scritto: >> Empirically I spotted this odd behaviour with >> gcc_GAS_CHECK_FEATURE and comments - Attached are the 2 alternate >> patches that I tried and the difference in the configure scripts >> themselves . I am no m4 expert but it does look like the m4_substr in >> gcc_GAS_CHECK_FEATURE doesn't really like that comment there. >> >> Any ideas anyone ? I must decline any knowledge of m4 at this point >> and I don't want to commit this until I understand what's going on >> here. > > Yes, the # comment is actually part of the macro argument. If you want > to write a "real" comment (i.e. at the m4 rather than shell level) use > "dnl" instead of "#". > > The patch with correct configure output is ok. Thanks - this is what I committed. Ramana 2012-03-15 Ramana Radhakrishnan * config.gcc (target_type_format_char): New. Document it. Set it for arm*-*-* . * configure.ac (gnu_unique_option): Use target_type_format_char in test. Comment rationale. * configure: Regenerate . Ramana > > Paolo Index: gcc/configure === --- gcc/configure (revision 185427) +++ gcc/configure (working copy) @@ -26081,7 +26081,7 @@ then gcc_cv_as_gnu_unique_object=yes fi elif test x$gcc_cv_as != x; then -$as_echo '.type foo, @gnu_unique_object' > conftest.s +$as_echo '.type foo, '$target_type_format_char'gnu_unique_object' > conftest.s if { ac_try='$gcc_cv_as $gcc_cv_as_flags -o conftest.o conftest.s >&5' { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5 (eval $ac_try) 2>&5 @@ -26100,7 +26100,8 @@ { $as_echo "$as_me:${as_lineno-$LINENO}: result: $gcc_cv_as_gnu_unique_object" >&5 $as_echo "$gcc_cv_as_gnu_unique_object" >&6; } if test $gcc_cv_as_gnu_unique_object = yes; then - # Also check for ld.so support, i.e. glibc 2.11 or higher. + # We need to unquote above to to use the definition from config.gcc. +# Also check for ld.so support, i.e. glibc 2.11 or higher. if test x$host = x$build -a x$host = x$target && ldd --version 2>/dev/null && glibcver=`ldd --version 2>/dev/null | sed 's/.* //;q'`; then Index: gcc/configure.ac === --- gcc/configure.ac(revision 185427) +++ gcc/configure.ac(working copy) @@ -4113,7 +4113,8 @@ esac], [gcc_GAS_CHECK_FEATURE([gnu_unique_object], gcc_cv_as_gnu_unique_object, [elf,2,19,52],, - [.type foo, @gnu_unique_object],, + [.type foo, '$target_type_format_char'gnu_unique_object],, +# We need to unquote above to to use the definition from config.gcc. # Also check for ld.so support, i.e. glibc 2.11 or higher. [[if test x$host = x$build -a x$host = x$target && ldd --version 2>/dev/null && Index: gcc/config.gcc === --- gcc/config.gcc (revision 185427) +++ gcc/config.gcc (working copy) @@ -182,6 +182,11 @@ # the --with-sysroot configure option or the # --sysroot command line option is used this # will be relative to the sysroot. +# target_type_format_char +# The default character to be used for formatting +# the attribute in a +# .type symbol_name, ${t_t_f_c} +# directive. # The following variables are used in each case-construct to build up the # outgoing variables: @@ -232,6 +237,7 @@ need_64bit_hwint= need_64bit_isa= native_system_header_dir=/usr/include +target_type_format_char='@' # Don't carry these over build->host->target. Please. xm_file= @@ -314,6 +320,7 @@ arm*-*-*) cpu_type=arm extra_headers="mmintrin.h arm_neon.h" + target_type_format_char='%' c_target_objs="arm-c.o" cxx_target_objs="arm-c.o" extra_options="${extra_options} arm/arm-tables.opt"
Re: [Patch ARM/ configury] Add fall-back check for gnu_unique_object
Paolo Bonzini writes: > Yes, the # comment is actually part of the macro argument. If you want > to write a "real" comment (i.e. at the m4 rather than shell level) use > "dnl" instead of "#". Actually both are part of the macro argument and act like comment introducers at the m4 level, but they differ in what they expand to. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Re: [Patch ARM/ configury] Add fall-back check for gnu_unique_object
Il 14/03/2012 16:37, Ramana Radhakrishnan ha scritto: > Empirically I spotted this odd behaviour with > gcc_GAS_CHECK_FEATURE and comments - Attached are the 2 alternate > patches that I tried and the difference in the configure scripts > themselves . I am no m4 expert but it does look like the m4_substr in > gcc_GAS_CHECK_FEATURE doesn't really like that comment there. > > Any ideas anyone ? I must decline any knowledge of m4 at this point > and I don't want to commit this until I understand what's going on > here. Yes, the # comment is actually part of the macro argument. If you want to write a "real" comment (i.e. at the m4 rather than shell level) use "dnl" instead of "#". The patch with correct configure output is ok. Paolo
Re: [Patch ARM/ configury] Add fall-back check for gnu_unique_object
On 12 March 2012 18:10, DJ Delorie wrote: > > Looks OK to me. I was about to commit this into my svn checkout and then realized the patch p4 didn't have the changes to configure - So I regenerated it again and then began a journey into the depths of m4 and autoconf for a bit before I gave up. Empirically I spotted this odd behaviour with gcc_GAS_CHECK_FEATURE and comments - Attached are the 2 alternate patches that I tried and the difference in the configure scripts themselves . I am no m4 expert but it does look like the m4_substr in gcc_GAS_CHECK_FEATURE doesn't really like that comment there. Any ideas anyone ? I must decline any knowledge of m4 at this point and I don't want to commit this until I understand what's going on here. Ramana - correct-configure-output.txt : Patch where everything looks sane. - incorrect-configure-output.txt: Patch where everything looks insane. - Diffs in the configure output : differences in configure files - for ease of viewing. Index: configure.ac === --- configure.ac(revision 185343) +++ configure.ac(working copy) @@ -4177,7 +4177,8 @@ esac], [gcc_GAS_CHECK_FEATURE([gnu_unique_object], gcc_cv_as_gnu_unique_object, [elf,2,19,52],, - [.type foo, @gnu_unique_object],, + [.type foo, '$target_type_format_char'gnu_unique_object],, +# We need to unquote above to to use the definition from config.gcc. # Also check for ld.so support, i.e. glibc 2.11 or higher. [[if test x$host = x$build -a x$host = x$target && ldd --version 2>/dev/null && Index: configure === --- configure (revision 185343) +++ configure (working copy) @@ -26167,7 +26167,7 @@ then gcc_cv_as_gnu_unique_object=yes fi elif test x$gcc_cv_as != x; then -$as_echo '.type foo, @gnu_unique_object' > conftest.s +$as_echo '.type foo, '$target_type_format_char'gnu_unique_object' > conftest.s if { ac_try='$gcc_cv_as $gcc_cv_as_flags -o conftest.o conftest.s >&5' { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5 (eval $ac_try) 2>&5 @@ -26186,7 +26186,8 @@ { $as_echo "$as_me:${as_lineno-$LINENO}: result: $gcc_cv_as_gnu_unique_object" >&5 $as_echo "$gcc_cv_as_gnu_unique_object" >&6; } if test $gcc_cv_as_gnu_unique_object = yes; then - # Also check for ld.so support, i.e. glibc 2.11 or higher. + # We need to unquote above to to use the definition from config.gcc. +# Also check for ld.so support, i.e. glibc 2.11 or higher. if test x$host = x$build -a x$host = x$target && ldd --version 2>/dev/null && glibcver=`ldd --version 2>/dev/null | sed 's/.* //;q'`; then Index: config.gcc === --- config.gcc (revision 185343) +++ config.gcc (working copy) @@ -182,6 +182,11 @@ # the --with-sysroot configure option or the # --sysroot command line option is used this # will be relative to the sysroot. +# target_type_format_char +# The default character to be used for formatting +# the attribute in a +# .type symbol_name, ${t_t_f_c} +# directive. # The following variables are used in each case-construct to build up the # outgoing variables: @@ -232,6 +237,7 @@ need_64bit_hwint= need_64bit_isa= native_system_header_dir=/usr/include +target_type_format_char='@' # Don't carry these over build->host->target. Please. xm_file= @@ -316,6 +322,7 @@ arm*-*-*) cpu_type=arm extra_headers="mmintrin.h arm_neon.h" + target_type_format_char='%' c_target_objs="arm-c.o" cxx_target_objs="arm-c.o" extra_options="${extra_options} arm/arm-tables.opt" Index: configure.ac === --- configure.ac(revision 185343) +++ configure.ac(working copy) @@ -4177,7 +4177,8 @@ esac], [gcc_GAS_CHECK_FEATURE([gnu_unique_object], gcc_cv_as_gnu_unique_object, [elf,2,19,52],, - [.type foo, @gnu_unique_object],, +# We need to unquote above to to use the definition from config.gcc. + [.type foo, '$target_type_format_char'gnu_unique_object],, # Also check for ld.so support, i.e. glibc 2.11 or higher. [[if test x$host = x$build -a x$host = x$target && ldd --version 2>/dev/null && Index: configure === --- configure (revision 185343) +++ configure (working copy) @@ -26167,7 +26167,7 @@ then gcc_cv_as_gnu_unique_object=yes fi elif test x$gcc_cv_as != x; then -$as_echo '.type foo, @gnu_unique_object' > conftest.s +$as_echo > conftest.s if { ac_try='$gcc_cv_as $gcc_cv_as_flags -o conftest.o conftest.s >&5' { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5 (eval
Re: [Patch ARM/ configury] Add fall-back check for gnu_unique_object
Looks OK to me.
Re: [Patch ARM/ configury] Add fall-back check for gnu_unique_object
On 10 March 2012 00:39, DJ Delorie wrote: > >> > Ping - http://gcc.gnu.org/ml/gcc-patches/2012-03/msg00549.html >> >> And now really add Paolo and DJ. > > + [.type foo, '$target_type_format_char'gnu_unique_object],, > > This un-quoting looks incorrect if you don't know what's going on > under the hood, but I don't see a clean way around it. A suitable > comment would be appropriate. Thanks for the quick review - I thought however it was kind of a "standard" workaround for this issue having seen this elsewhere in the same file - given this is used in multiple places/ > > +target_type_format_char="@" > + target_type_format_char="%" > > Since the string always has "special" characters, it's likely that > single quotes are more appropriate here. The two characters in the > patch don't care, but some future porter might naively do "$" and > wonder why (or worse, not wonder why) it doesn't work right. Fair point - done. > > Other than that it looks OK to me, assuming you tested it on all the > relevent targets (i.e. arm and not-arm). I tested x86_64-linux-gnu with a bootstrap and that showed identical auto-host.h to the previous run and thus that appeared to be fine. (This is a target that uses the default '@') .. On ARM I've done a full bootstrap and auto-host.h shows the appropriate macro defined ( and it does with this version of the patch as well). Are there any other targets you'd suggest ? Assuming all tests pass is this version better ? cheers Ramana 2012-03-11 Ramana Radhakrishnan * config.gcc (target_type_format_char): New. Document it. Set it for arm*-*-* . * configure.ac (gnu_unique_option): Use target_type_format_char in test. Comment rationale. * configure: Regenerate . diff --git a/gcc/config.gcc b/gcc/config.gcc index 99f0b47..a769d0c 100644 --- a/gcc/config.gcc +++ b/gcc/config.gcc @@ -184,6 +184,11 @@ # the --with-sysroot configure option or the # --sysroot command line option is used this # will be relative to the sysroot. +# target_type_format_char +# The default character to be used for formatting +# the attribute in a +# .type symbol_name, ${t_t_f_c} +# directive. # The following variables are used in each case-construct to build up the # outgoing variables: @@ -235,6 +240,7 @@ target_gtfiles= need_64bit_hwint= need_64bit_isa= native_system_header_dir=/usr/include +target_type_format_char='@' # Don't carry these over build->host->target. Please. xm_file= @@ -321,6 +327,7 @@ am33_2.0-*-linux*) arm*-*-*) cpu_type=arm extra_headers="mmintrin.h arm_neon.h" + target_type_format_char='%' c_target_objs="arm-c.o" cxx_target_objs="arm-c.o" extra_options="${extra_options} arm/arm-tables.opt" diff --git a/gcc/configure.ac b/gcc/configure.ac index 39302ad..4a534a1 100644 --- a/gcc/configure.ac +++ b/gcc/configure.ac @@ -4188,7 +4188,9 @@ Valid choices are 'yes' and 'no'.]) ;; esac], [gcc_GAS_CHECK_FEATURE([gnu_unique_object], gcc_cv_as_gnu_unique_object, [elf,2,19,52],, - [.type foo, @gnu_unique_object],, +#We have to unquote here to reuse the variable from +#config.gcc. + [.type foo, '$target_type_format_char'gnu_unique_object],, # Also check for ld.so support, i.e. glibc 2.11 or higher. [[if test x$host = x$build -a x$host = x$target && ldd --version 2>/dev/null &&
Re: [Patch ARM/ configury] Add fall-back check for gnu_unique_object
> > Ping - http://gcc.gnu.org/ml/gcc-patches/2012-03/msg00549.html > > And now really add Paolo and DJ. + [.type foo, '$target_type_format_char'gnu_unique_object],, This un-quoting looks incorrect if you don't know what's going on under the hood, but I don't see a clean way around it. A suitable comment would be appropriate. +target_type_format_char="@" + target_type_format_char="%" Since the string always has "special" characters, it's likely that single quotes are more appropriate here. The two characters in the patch don't care, but some future porter might naively do "$" and wonder why (or worse, not wonder why) it doesn't work right. Other than that it looks OK to me, assuming you tested it on all the relevent targets (i.e. arm and not-arm).
Re: [Patch ARM/ configury] Add fall-back check for gnu_unique_object
> Ping - http://gcc.gnu.org/ml/gcc-patches/2012-03/msg00549.html And now really add Paolo and DJ. Ramana > > regards, > Ramana > > >> >> regards, >> Ramana >> >> >> 2012-03-07 Ramana Radhakrishnan >> >> * config.gcc (target_type_format_char): New. Document it. Set it for >> arm*-*-* . >> * configure.ac (gnu_unique_option): Use target_type_format_char in >> test. >> * configure: Regenerate .
Re: [Patch ARM/ configury] Add fall-back check for gnu_unique_object
> > Ok ? Adjusting subject line and adding build machinery maintainer to comment - Ping - http://gcc.gnu.org/ml/gcc-patches/2012-03/msg00549.html regards, Ramana > > regards, > Ramana > > > 2012-03-07 Ramana Radhakrishnan > > * config.gcc (target_type_format_char): New. Document it. Set it for > arm*-*-* . > * configure.ac (gnu_unique_option): Use target_type_format_char in test. > * configure: Regenerate .
[Patch ARM/ configury] Add fall-back check for gnu_unique_object - Default to this for arm-linux-gnueabi
Hi, While investigating a report for odd behaviour with a C++ program, I discovered that the automatic checking for gnu_unique_object is broken during bootstrap for arm-linux-gnueabi targets because on ARM '@' is a comment character for the assembler. I do realize this can be worked around with the configure time option but it would be nice to fix this without having to resort to the workaround everytime. Fixed thusly - tested with bootstraps on both arm-linux-gnueabi and verified that auto-host.h has the right definition. Ok ? regards, Ramana 2012-03-07 Ramana Radhakrishnan * config.gcc (target_type_format_char): New. Document it. Set it for arm*-*-* . * configure.ac (gnu_unique_option): Use target_type_format_char in test. * configure: Regenerate . Index: gcc/config.gcc === --- gcc/config.gcc (revision 185057) +++ gcc/config.gcc (working copy) @@ -184,6 +184,11 @@ # the --with-sysroot configure option or the # --sysroot command line option is used this # will be relative to the sysroot. +# target_type_format_char +# The default character to be used for formatting +# the attribute in a +# .type symbol_name, ${t_t_f_c} +# directive. # The following variables are used in each case-construct to build up the # outgoing variables: @@ -235,6 +240,7 @@ need_64bit_hwint= need_64bit_isa= native_system_header_dir=/usr/include +target_type_format_char="@" # Don't carry these over build->host->target. Please. xm_file= @@ -321,6 +327,7 @@ arm*-*-*) cpu_type=arm extra_headers="mmintrin.h arm_neon.h" + target_type_format_char="%" c_target_objs="arm-c.o" cxx_target_objs="arm-c.o" extra_options="${extra_options} arm/arm-tables.opt" Index: gcc/configure === --- gcc/configure (revision 185057) +++ gcc/configure (working copy) @@ -26178,7 +26178,7 @@ then gcc_cv_as_gnu_unique_object=yes fi elif test x$gcc_cv_as != x; then -$as_echo '.type foo, @gnu_unique_object' > conftest.s +$as_echo '.type foo, '$target_type_format_char'gnu_unique_object' > conftest.s if { ac_try='$gcc_cv_as $gcc_cv_as_flags -o conftest.o conftest.s >&5' { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5 (eval $ac_try) 2>&5 Index: gcc/configure.ac === --- gcc/configure.ac(revision 185057) +++ gcc/configure.ac(working copy) @@ -4187,7 +4187,7 @@ esac], [gcc_GAS_CHECK_FEATURE([gnu_unique_object], gcc_cv_as_gnu_unique_object, [elf,2,19,52],, - [.type foo, @gnu_unique_object],, + [.type foo, '$target_type_format_char'gnu_unique_object],, # Also check for ld.so support, i.e. glibc 2.11 or higher. [[if test x$host = x$build -a x$host = x$target && ldd --version 2>/dev/null &&