[GOLD] PowerPC64 identical code folding

Alan Modra amodra@gmail.com
Tue Mar 12 13:31:00 GMT 2013


On Mon, Mar 11, 2013 at 06:21:05PM -0700, Andrew Pinski wrote:
> Actually keeping around different function descriptors seems better
> and then for PowerPC64 you can just turn ICF by default and not
> violate C/C++ rules about function pointers being different.

Yes, good idea.  --icf=safe can do the same as --icf=all on ppc64.

This patch enables --icf=safe mode for powerpc, and finally gives a
clean gold testsuite for ppc and ppc64 (unless you happen to be
affected by PR14675).  I changed the icf tests that check for folding
to check sections rather than symbols.  This avoids the problem that
folded functions on ppc64 remain with distinct addresses.  The tests
that check for no folding continue to use symbols.  OK to apply?

	* powerpc.cc (is_branch_reloc): Forward declare.
	(Target_powerpc::do_can_check_for_function_pointers): New predicate.
	(Target_powerpc::Scan::local_reloc_may_be_function_pointer): Return
	false for 64-bit, true for 32-bit non-branch relocs.
	(Target_powerpc::Scan::global_reloc_may_be_function_pointer): Likewise.
	* testsuite/Makefile.am (icf_test): Use linker map file instead of
	nm output.
	(icf_safe_test): Generate linker map file as well as nm output.
	(icf_safe_so_test): Likewise.
	* testsuite/Makefile.in: Regenerate.
	* testsuite/icf_test.sh: Parse linker map file to determine
	section folding.
	* testsuite/icf_safe_test.sh: Likewise.  Expect folding for PowerPC.
	* testsuite/icf_safe_so_test.sh: Likewise.
	(X86_32_or_ARM_specific_safe_fold): Merge into..
	(arch_specific_safe_fold): ..this.
	(X86_64_specific_safe_fold): Delete unused function.

Index: gold/powerpc.cc
===================================================================
RCS file: /cvs/src/src/gold/powerpc.cc,v
retrieving revision 1.86
diff -u -p -r1.86 powerpc.cc
--- gold/powerpc.cc	10 Mar 2013 23:08:18 -0000	1.86
+++ gold/powerpc.cc	12 Mar 2013 12:47:21 -0000
@@ -62,6 +62,9 @@ class Output_data_glink;
 template<int size, bool big_endian>
 class Stub_table;
 
+inline bool
+is_branch_reloc(unsigned int r_type);
+
 template<int size, bool big_endian>
 class Powerpc_relobj : public Sized_relobj_file<size, big_endian>
 {
@@ -555,6 +558,10 @@ class Target_powerpc : public Sized_targ
   void
   do_function_location(Symbol_location*) const;
 
+  bool
+  do_can_check_for_function_pointers() const
+  { return true; }
+
   // Relocate a section.
   void
   relocate_section(const Relocate_info<size, big_endian>*,
@@ -863,9 +870,18 @@ class Target_powerpc : public Sized_targ
 					unsigned int ,
 					Output_section* ,
 					const elfcpp::Rela<size, big_endian>& ,
-					unsigned int ,
+					unsigned int r_type,
 					const elfcpp::Sym<size, big_endian>&)
-    { return false; }
+    {
+      // PowerPC64 .opd is not folded, so any identical function text
+      // may be folded and we'll still keep function addresses distinct.
+      // That means no reloc is of concern here.
+      if (size == 64)
+	return false;
+      // For 32-bit, conservatively assume anything but calls to
+      // function code might be taking the address of the function.
+      return !is_branch_reloc(r_type);
+    }
 
     inline bool
     global_reloc_may_be_function_pointer(Symbol_table* , Layout* ,
@@ -873,10 +889,15 @@ class Target_powerpc : public Sized_targ
 					 Sized_relobj_file<size, big_endian>* ,
 					 unsigned int ,
 					 Output_section* ,
-					 const elfcpp::Rela<size,
-							    big_endian>& ,
-					 unsigned int , Symbol*)
-    { return false; }
+					 const elfcpp::Rela<size, big_endian>& ,
+					 unsigned int r_type,
+					 Symbol*)
+    {
+      // As above.
+      if (size == 64)
+	return false;
+      return !is_branch_reloc(r_type);
+    }
 
   private:
     static void
Index: gold/testsuite/Makefile.am
===================================================================
RCS file: /cvs/src/src/gold/testsuite/Makefile.am,v
retrieving revision 1.208
diff -u -p -r1.208 Makefile.am
--- gold/testsuite/Makefile.am	4 Mar 2013 00:48:01 -0000	1.208
+++ gold/testsuite/Makefile.am	12 Mar 2013 12:47:22 -0000
@@ -201,14 +221,12 @@ pr14265.stdout: pr14265
 	$(TEST_NM) --format=bsd --numeric-sort $< > $@
 
 check_SCRIPTS += icf_test.sh
-check_DATA += icf_test.stdout
-MOSTLYCLEANFILES += icf_test
+check_DATA += icf_test.map
+MOSTLYCLEANFILES += icf_test icf_test.map
 icf_test.o: icf_test.cc 
 	$(CXXCOMPILE) -O0 -c -ffunction-sections -g -o $@ $<
-icf_test: icf_test.o gcctestdir/ld
-	$(CXXLINK) -Bgcctestdir/ -Wl,--icf=all icf_test.o
-icf_test.stdout: icf_test
-	$(TEST_NM) -C icf_test > icf_test.stdout
+icf_test icf_test.map: icf_test.o gcctestdir/ld
+	$(CXXLINK) -o icf_test -Bgcctestdir/ -Wl,--icf=all,-Map,icf_test.map icf_test.o
 
 check_SCRIPTS += icf_keep_unique_test.sh
 check_DATA += icf_keep_unique_test.stdout
@@ -218,31 +236,31 @@ icf_keep_unique_test.o: icf_keep_unique_
 icf_keep_unique_test: icf_keep_unique_test.o gcctestdir/ld
 	$(CXXLINK) -Bgcctestdir/ -Wl,--icf=all -Wl,--keep-unique,_Z11unique_funcv icf_keep_unique_test.o
 icf_keep_unique_test.stdout: icf_keep_unique_test
-	$(TEST_NM) -C icf_keep_unique_test > icf_keep_unique_test.stdout
+	$(TEST_NM) -C $< > $@
 
 check_SCRIPTS += icf_safe_test.sh
-check_DATA += icf_safe_test_1.stdout icf_safe_test_2.stdout
-MOSTLYCLEANFILES += icf_safe_test
+check_DATA += icf_safe_test_1.stdout icf_safe_test_2.stdout icf_safe_test.map
+MOSTLYCLEANFILES += icf_safe_test icf_safe_test.map
 icf_safe_test.o: icf_safe_test.cc
 	$(CXXCOMPILE) -O0 -c -ffunction-sections -g -o $@ $<
-icf_safe_test: icf_safe_test.o gcctestdir/ld
-	$(CXXLINK) -Bgcctestdir/ -Wl,--icf=safe icf_safe_test.o
+icf_safe_test icf_safe_test.map: icf_safe_test.o gcctestdir/ld
+	$(CXXLINK) -o icf_safe_test -Bgcctestdir/ -Wl,--icf=safe,-Map,icf_safe_test.map icf_safe_test.o
 icf_safe_test_1.stdout: icf_safe_test
-	$(TEST_NM) icf_safe_test > icf_safe_test_1.stdout
+	$(TEST_NM) $< > $@
 icf_safe_test_2.stdout: icf_safe_test
-	$(TEST_READELF) -h icf_safe_test > icf_safe_test_2.stdout
+	$(TEST_READELF) -h $< > $@
 
 check_SCRIPTS += icf_safe_so_test.sh
-check_DATA += icf_safe_so_test_1.stdout icf_safe_so_test_2.stdout
-MOSTLYCLEANFILES += icf_safe_so_test
+check_DATA += icf_safe_so_test_1.stdout icf_safe_so_test_2.stdout icf_safe_so_test.map
+MOSTLYCLEANFILES += icf_safe_so_test icf_safe_so_test.map
 icf_safe_so_test.o: icf_safe_so_test.cc
 	$(CXXCOMPILE) -O0 -c -ffunction-sections -fPIC -g -o $@ $<
-icf_safe_so_test: icf_safe_so_test.o gcctestdir/ld
-	$(CXXLINK) -Bgcctestdir/ -Wl,--icf=safe icf_safe_so_test.o -fPIC -shared
+icf_safe_so_test icf_safe_so_test.map: icf_safe_so_test.o gcctestdir/ld
+	$(CXXLINK) -o icf_safe_so_test -Bgcctestdir/ -Wl,--icf=safe,-Map,icf_safe_so_test.map icf_safe_so_test.o -fPIC -shared
 icf_safe_so_test_1.stdout: icf_safe_so_test
-	$(TEST_NM) icf_safe_so_test > icf_safe_so_test_1.stdout
+	$(TEST_NM) $< > $@
 icf_safe_so_test_2.stdout: icf_safe_so_test
-	$(TEST_READELF) -h icf_safe_so_test > icf_safe_so_test_2.stdout
+	$(TEST_READELF) -h $< > $@
 
 check_SCRIPTS += final_layout.sh
 check_DATA += final_layout.stdout
Index: gold/testsuite/icf_test.sh
===================================================================
RCS file: /cvs/src/src/gold/testsuite/icf_test.sh,v
retrieving revision 1.1
diff -u -p -r1.1 icf_test.sh
--- gold/testsuite/icf_test.sh	5 Aug 2009 20:51:56 -0000	1.1
+++ gold/testsuite/icf_test.sh	12 Mar 2013 12:47:22 -0000
@@ -28,13 +28,19 @@
 
 check()
 {
-    func_addr_1=`grep $2 $1 | awk '{print $1}'`
-    func_addr_2=`grep $3 $1 | awk '{print $1}'`
-    if [ $func_addr_1 != $func_addr_2 ]
-    then
-        echo "Identical Code Folding failed to fold" $2 "and" $3
-	exit 1
-    fi
+    awk "
+BEGIN { discard = 0; }
+/^Discarded input/ { discard = 1; }
+/^Memory map/ { discard = 0; }
+/.*\\.text\\..*($2|$3).*/ { act[discard] = act[discard] \" \" \$0; }
+END {
+      # printf \"kept\" act[0] \"\\nfolded\" act[1] \"\\n\";
+      if (length(act[0]) == 0 || length(act[1]) == 0)
+	{
+	  printf \"Identical Code Folding did not fold $2 and $3\\n\"
+	  exit 1;
+	}
+    }" $1
 }
 
-check icf_test.stdout "folded_func" "kept_func"
+check icf_test.map "folded_func" "kept_func"
Index: gold/testsuite/icf_safe_test.sh
===================================================================
RCS file: /cvs/src/src/gold/testsuite/icf_safe_test.sh,v
retrieving revision 1.6
diff -u -p -r1.6 icf_safe_test.sh
--- gold/testsuite/icf_safe_test.sh	15 Sep 2012 17:11:28 -0000	1.6
+++ gold/testsuite/icf_safe_test.sh	12 Mar 2013 12:47:22 -0000
@@ -40,28 +40,34 @@ check_nofold()
 
 check_fold()
 {
-    func_addr_1=`grep $2 $1 | awk '{print $1}'`
-    func_addr_2=`grep $3 $1 | awk '{print $1}'`
-    if [ $func_addr_1 != $func_addr_2 ]
-    then
-        echo "Safe Identical Code Folding did not fold " $2 "and" $3
-	exit 1
-    fi
+    awk "
+BEGIN { discard = 0; }
+/^Discarded input/ { discard = 1; }
+/^Memory map/ { discard = 0; }
+/.*\\.text\\..*($2|$3).*/ { act[discard] = act[discard] \" \" \$0; }
+END {
+      # printf \"kept\" act[0] \"\\nfolded\" act[1] \"\\n\";
+      if (length(act[0]) == 0 || length(act[1]) == 0)
+	{
+	  printf \"Safe Identical Code Folding did not fold $2 and $3\\n\"
+	  exit 1;
+	}
+    }" $1
 }
 
 arch_specific_safe_fold()
 {
-    grep_x86=`grep -q -e "Advanced Micro Devices X86-64" -e "Intel 80386" -e "ARM" -e "TILE" $2`
+    grep_x86=`grep -q -e "Advanced Micro Devices X86-64" -e "Intel 80386" -e "ARM" -e "TILE" -e "PowerPC" $2`
     if [ $? -eq 0 ];
     then
-      check_fold $1 $3 $4
+      check_fold $3 $4 $5
     else
-      check_nofold $1 $3 $4
+      check_nofold $1 $4 $5
     fi
 }
 
 arch_specific_safe_fold icf_safe_test_1.stdout icf_safe_test_2.stdout \
- "kept_func_1" "kept_func_2"
-check_fold   icf_safe_test_1.stdout "_ZN1AD1Ev" "_ZN1AC1Ev"
+  icf_safe_test.map "kept_func_1" "kept_func_2"
+check_fold   icf_safe_test.map "_ZN1AD2Ev" "_ZN1AC2Ev"
 check_nofold icf_safe_test_1.stdout "kept_func_3" "kept_func_1"
 check_nofold icf_safe_test_1.stdout "kept_func_3" "kept_func_2"
Index: gold/testsuite/icf_safe_so_test.sh
===================================================================
RCS file: /cvs/src/src/gold/testsuite/icf_safe_so_test.sh,v
retrieving revision 1.5
diff -u -p -r1.5 icf_safe_so_test.sh
--- gold/testsuite/icf_safe_so_test.sh	21 Mar 2011 20:55:33 -0000	1.5
+++ gold/testsuite/icf_safe_so_test.sh	12 Mar 2013 12:47:22 -0000
@@ -67,41 +67,36 @@ check_fold()
       return 0
     fi
 
-    func_addr_1=`grep $2 $1 | awk '{print $1}'`
-    func_addr_2=`grep $3 $1 | awk '{print $1}'`
-    if [ $func_addr_1 != $func_addr_2 ];
-    then
-        echo "Safe Identical Code Folding did not fold " $2 "and" $3
-	exit 1
-    fi
+    awk "
+BEGIN { discard = 0; }
+/^Discarded input/ { discard = 1; }
+/^Memory map/ { discard = 0; }
+/.*\\.text\\..*($2|$3).*/ { act[discard] = act[discard] \" \" \$0; }
+END {
+      # printf \"kept\" act[0] \"\\nfolded\" act[1] \"\\n\";
+      if (length(act[0]) == 0 || length(act[1]) == 0)
+	{
+	  printf \"Safe Identical Code Folding did not fold $2 and $3\\n\"
+	  exit 1;
+	}
+    }" $4
 }
 
 arch_specific_safe_fold()
 {
-    if [ $1 -eq 0 ];
+    grep -e "Intel 80386" -e "ARM" -e "PowerPC" $1 > /dev/null 2>&1
+    if [ $? -eq 0 ];
     then
-      check_fold $2 $3 $4
+      check_fold $2 $4 $5 $3
     else
-      check_nofold $2 $3 $4
+      check_nofold $2 $4 $5
     fi
 }
 
-X86_32_or_ARM_specific_safe_fold()
-{
-    grep -e "Intel 80386" -e "ARM" $1 > /dev/null 2>&1
-    arch_specific_safe_fold $? $2 $3 $4
-}
-
-X86_64_specific_safe_fold ()
-{
-    grep -e "Advanced Micro Devices X86-64" $1 > /dev/null 2>&1
-    arch_specific_safe_fold $? $2 $3 $4
-}
-
-X86_32_or_ARM_specific_safe_fold icf_safe_so_test_2.stdout icf_safe_so_test_1.stdout "foo_prot" "foo_hidden"
-X86_32_or_ARM_specific_safe_fold icf_safe_so_test_2.stdout icf_safe_so_test_1.stdout "foo_prot" "foo_internal"
-X86_32_or_ARM_specific_safe_fold icf_safe_so_test_2.stdout icf_safe_so_test_1.stdout "foo_prot" "foo_static"
-X86_32_or_ARM_specific_safe_fold icf_safe_so_test_2.stdout icf_safe_so_test_1.stdout "foo_hidden" "foo_internal"
-X86_32_or_ARM_specific_safe_fold icf_safe_so_test_2.stdout icf_safe_so_test_1.stdout "foo_hidden" "foo_static"
-X86_32_or_ARM_specific_safe_fold icf_safe_so_test_2.stdout icf_safe_so_test_1.stdout "foo_internal" "foo_static"
+arch_specific_safe_fold icf_safe_so_test_2.stdout icf_safe_so_test_1.stdout icf_safe_so_test.map "foo_prot" "foo_hidden"
+arch_specific_safe_fold icf_safe_so_test_2.stdout icf_safe_so_test_1.stdout icf_safe_so_test.map "foo_prot" "foo_internal"
+arch_specific_safe_fold icf_safe_so_test_2.stdout icf_safe_so_test_1.stdout icf_safe_so_test.map "foo_prot" "foo_static"
+arch_specific_safe_fold icf_safe_so_test_2.stdout icf_safe_so_test_1.stdout icf_safe_so_test.map "foo_hidden" "foo_internal"
+arch_specific_safe_fold icf_safe_so_test_2.stdout icf_safe_so_test_1.stdout icf_safe_so_test.map "foo_hidden" "foo_static"
+arch_specific_safe_fold icf_safe_so_test_2.stdout icf_safe_so_test_1.stdout icf_safe_so_test.map "foo_internal" "foo_static"
 check_nofold icf_safe_so_test_1.stdout "foo_glob" "bar_glob"

-- 
Alan Modra
Australia Development Lab, IBM



More information about the Binutils mailing list