public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] gold testsuite: conditionalize -fmerge-constants use, clean up test
@ 2013-10-11 20:24 Roland McGrath
  2013-10-11 21:45 ` Cary Coutant
  0 siblings, 1 reply; 4+ messages in thread
From: Roland McGrath @ 2013-10-11 20:24 UTC (permalink / raw)
  To: binutils

Clang does not grok the -fmerge-constants switch, but it always emits
strings in SHF_MERGE sections anyway.  So I conditionalized the use of
the flag but did not conditionalize the tests that rely on the behavior.
The test files generate -Wmissing-prototypes warnings as C; we elide that
warning option for C++, and the code looks like it was intended as C++
rather than C anyway (using () rather than (void) in prototypes), so the
renaming was the simplest way to avoid the warnings.

Note that the patch omits changes to generated files, and also omits the
file-renaming diff (included in git format, but no actual diffs).

OK for trunk and 2.24?


Thanks,
Roland


gold/
	* configure.ac (MERGE_CONSTANTS_CFLAGS): New check.
    	* configure: Regenerate.
	* Makefile.in: Regenerate.
	* testsuite/merge_string_literals_1.c: Renamed to have .cc suffix.
	* testsuite/merge_string_literals_2.c: Likewise.
	* testsuite/Makefile.am
	(merge_string_literals_1.o, merge_string_literals_2.o): Update deps.
	(AM_CFLAGS, AM_CXXFLAGS): Use $(MERGE_CONSTANTS_FLAGS) in place of
	literal -fmerge-constants.
	* testsuite/Makefile.in: Regenerate.

--- a/gold/configure.ac
+++ b/gold/configure.ac
@@ -338,6 +338,19 @@ dnl Whether we can test -mcmodel=medium.
 AM_CONDITIONAL(MCMODEL_MEDIUM,
 [test "$target_cpu" = "x86_64" -a "$have_mcmodel_medium" = "yes" -a
"$gold_cv_prog_gcc41" = "yes"])

+AC_CACHE_CHECK([whether $CC supports -fmerge-constants],
+	       [gold_cv_merge_constants], [
+save_CFLAGS="$CFLAGS"
+CFLAGS="$CFLAGS -fmerge-constants"
+AC_COMPILE_IFELSE([const char *s = "foo";],
+		  [have_merge_constants=yes],
+		  [have_merge_constants=no])
+CFLAGS="$save_CFLAGS"])
+AC_SUBST([MERGE_CONSTANTS_CFLAGS])
+AS_IF([test "$gold_cv_merge_constants" = yes],
+      [MERGE_CONSTANTS_CFLAGS=-fmerge-constants],
+      [MERGE_CONSTANTS_CFLAGS=])
+
 dnl Test for __thread support.
 AC_CACHE_CHECK([for thread support], [gold_cv_c_thread],
 [AC_COMPILE_IFELSE([__thread int i = 1;],
--- a/gold/testsuite/Makefile.am
+++ b/gold/testsuite/Makefile.am
@@ -8,10 +8,10 @@
 AUTOMAKE_OPTIONS = foreign -Wno-portability

 # The two_file_test tests -fmerge-constants, so we simply always turn
-# it on.  This may need to be controlled by a configure option
-# eventually.
-AM_CFLAGS = $(WARN_CFLAGS) $(LFS_CFLAGS) -fmerge-constants
-AM_CXXFLAGS = $(WARN_CXXFLAGS) $(LFS_CFLAGS) -fmerge-constants
+# it on.  For compilers that do not support the command-line option,
+# we assume they just always emit SHF_MERGE sections unconditionally.
+AM_CFLAGS = $(WARN_CFLAGS) $(LFS_CFLAGS) $(MERGE_CONSTANTS_FLAGS)
+AM_CXXFLAGS = $(WARN_CXXFLAGS) $(LFS_CFLAGS) $(MERGE_CONSTANTS_FLAGS)

 AM_CPPFLAGS = \
 	-I$(srcdir) -I$(srcdir)/.. -I$(srcdir)/../../include \
@@ -337,9 +337,9 @@ large_symbol_alignment_LDADD =
 check_SCRIPTS += merge_string_literals.sh
 check_DATA += merge_string_literals.stdout
 MOSTLYCLEANFILES += merge_string_literals
-merge_string_literals_1.o: merge_string_literals_1.c
+merge_string_literals_1.o: merge_string_literals_1.cc
 	$(CXXCOMPILE) -O2 -c -fPIC -g -o $@ $<
-merge_string_literals_2.o: merge_string_literals_2.c
+merge_string_literals_2.o: merge_string_literals_2.cc
 	$(CXXCOMPILE) -O2 -c -fPIC -g -o $@ $<
 merge_string_literals: merge_string_literals_1.o
merge_string_literals_2.o gcctestdir/ld
 	$(CXXLINK) -Bgcctestdir/ merge_string_literals_1.o
merge_string_literals_2.o -O2 -shared -nostdlib
diff --git a/gold/testsuite/merge_string_literals_1.c
b/gold/testsuite/merge_string_literals_1.cc
similarity index 100%
rename from gold/testsuite/merge_string_literals_1.c
rename to gold/testsuite/merge_string_literals_1.cc
diff --git a/gold/testsuite/merge_string_literals_2.c
b/gold/testsuite/merge_string_literals_2.cc
similarity index 100%
rename from gold/testsuite/merge_string_literals_2.c
rename to gold/testsuite/merge_string_literals_2.cc

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] gold testsuite: conditionalize -fmerge-constants use, clean up test
  2013-10-11 20:24 [PATCH] gold testsuite: conditionalize -fmerge-constants use, clean up test Roland McGrath
@ 2013-10-11 21:45 ` Cary Coutant
  2013-10-11 21:57   ` Roland McGrath
  0 siblings, 1 reply; 4+ messages in thread
From: Cary Coutant @ 2013-10-11 21:45 UTC (permalink / raw)
  To: Roland McGrath; +Cc: binutils

> gold/
>         * configure.ac (MERGE_CONSTANTS_CFLAGS): New check.
>         * configure: Regenerate.
>         * Makefile.in: Regenerate.
>         * testsuite/merge_string_literals_1.c: Renamed to have .cc suffix.
>         * testsuite/merge_string_literals_2.c: Likewise.
>         * testsuite/Makefile.am
>         (merge_string_literals_1.o, merge_string_literals_2.o): Update deps.
>         (AM_CFLAGS, AM_CXXFLAGS): Use $(MERGE_CONSTANTS_FLAGS) in place of
>         literal -fmerge-constants.
>         * testsuite/Makefile.in: Regenerate.

You've got two different spellings here: MERGE_CONSTANTS_FLAGS and
MERGE_CONSTANTS_CFLAGS. How about just "MERGE_CONSTANTS_FLAG"?

OK with that change.

-cary

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] gold testsuite: conditionalize -fmerge-constants use, clean up test
  2013-10-11 21:45 ` Cary Coutant
@ 2013-10-11 21:57   ` Roland McGrath
  2013-10-31 18:40     ` Cary Coutant
  0 siblings, 1 reply; 4+ messages in thread
From: Roland McGrath @ 2013-10-11 21:57 UTC (permalink / raw)
  To: Cary Coutant; +Cc: binutils

On Fri, Oct 11, 2013 at 2:45 PM, Cary Coutant <ccoutant@google.com> wrote:
> You've got two different spellings here: MERGE_CONSTANTS_FLAGS and
> MERGE_CONSTANTS_CFLAGS. How about just "MERGE_CONSTANTS_FLAG"?

Oops.  The former was a typo, I was going for consistency with other
*_CFLAGS variables.  But _FLAG is fine by me.

> OK with that change.

Done and committed.


Thanks,
Roland

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] gold testsuite: conditionalize -fmerge-constants use, clean up test
  2013-10-11 21:57   ` Roland McGrath
@ 2013-10-31 18:40     ` Cary Coutant
  0 siblings, 0 replies; 4+ messages in thread
From: Cary Coutant @ 2013-10-31 18:40 UTC (permalink / raw)
  To: Roland McGrath; +Cc: binutils

>> You've got two different spellings here: MERGE_CONSTANTS_FLAGS and
>> MERGE_CONSTANTS_CFLAGS. How about just "MERGE_CONSTANTS_FLAG"?
>
> Oops.  The former was a typo, I was going for consistency with other
> *_CFLAGS variables.  But _FLAG is fine by me.
>
>> OK with that change.
>
> Done and committed.

Sorry, I missed another case where you had two different spellings.
I've committed the following patch.

-cary


2013-10-31  Cary Coutant  <ccoutant@google.com>

        * gold/configure.ac: Fix check for -fmerge-constants.
        * gold/configure.ac: Regenerate.

diff --git a/gold/configure.ac b/gold/configure.ac
index c23117b..82ad11e 100644
--- a/gold/configure.ac
+++ b/gold/configure.ac
@@ -343,8 +343,8 @@ AC_CACHE_CHECK([whether $CC supports -fmerge-constants],
 save_CFLAGS="$CFLAGS"
 CFLAGS="$CFLAGS -fmerge-constants"
 AC_COMPILE_IFELSE([const char *s = "foo";],
-  [have_merge_constants=yes],
-  [have_merge_constants=no])
+  [gold_cv_merge_constants=yes],
+  [gold_cv_merge_constants=no])
 CFLAGS="$save_CFLAGS"])
 AC_SUBST([MERGE_CONSTANTS_FLAG])
 AS_IF([test "$gold_cv_merge_constants" = yes],

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2013-10-31 18:40 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-11 20:24 [PATCH] gold testsuite: conditionalize -fmerge-constants use, clean up test Roland McGrath
2013-10-11 21:45 ` Cary Coutant
2013-10-11 21:57   ` Roland McGrath
2013-10-31 18:40     ` Cary Coutant

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).