opus - Mar 2014 - [PATCH 1/2] Add separate labels for the start of public functions

This avoids having to use the public symbol name when jumping here,
on platforms where the public symbols have an underscore prefix.
---
This avoids having to add heuristics for adding prefixes to symbols
in jumps to local labels as well.
---
 celt/arm/celt_pitch_xcorr_arm.s | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/celt/arm/celt_pitch_xcorr_arm.s b/celt/arm/celt_pitch_xcorr_arm.s
index 598e45b..f96e0a8 100644
--- a/celt/arm/celt_pitch_xcorr_arm.s
+++ b/celt/arm/celt_pitch_xcorr_arm.s
@@ -42,6 +42,7 @@ IF OPUS_ARM_MAY_HAVE_NEON
 
 ; Compute sum[k]=sum(x[j]*y[j+k],j=0...len-1), k=0...3
 xcorr_kernel_neon PROC
+xcorr_kernel_neon_start
   ; input:
   ;   r3     = int         len
   ;   r4     = opus_val16 *x
@@ -181,7 +182,7 @@ celt_pitch_xcorr_neon_process4
   VEOR         q0, q0, q0
   ; xcorr_kernel_neon only modifies r4, r5, r12, and q0...q3.
   ; So we don't save/restore any other registers.
-  BL xcorr_kernel_neon
+  BL xcorr_kernel_neon_start
   SUBS         r6, r6, #4
   VST1.32      {q0}, [r2]!
   ; _y += 4
@@ -257,6 +258,7 @@ IF OPUS_ARM_MAY_HAVE_EDSP
 ; This will get used on ARMv7 devices without NEON, so it has been optimized
 ; to take advantage of dual-issuing where possible.
 xcorr_kernel_edsp PROC
+xcorr_kernel_edsp_start
   ; input:
   ;   r3      = int         len
   ;   r4      = opus_val16 *_x (must be 32-bit aligned)
@@ -416,7 +418,7 @@ celt_pitch_xcorr_edsp_process4
   MOV          r7, #0
   MOV          r8, #0
   MOV          r9, #0
-  BL xcorr_kernel_edsp  ; xcorr_kernel_edsp(_x, _y+i, xcorr+i, len)
+  BL xcorr_kernel_edsp_start  ; xcorr_kernel_edsp(_x, _y+i, xcorr+i, len)
   ; maxcorr = max(maxcorr, sum0, sum1, sum2, sum3)
   CMP          r0, r6
   ; _y+=4
-- 
1.8.5.2 (Apple Git-48)

This allows building the arm assembly for iOS.

This checks for the __APPLE__ preprocessor built-in define to
determine whether this extra handling should be enabled.
---
 Makefile.am         |  2 +-
 celt/arm/arm2gnu.pl | 23 +++++++++++++++++------
 configure.ac        |  8 ++++++++
 3 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index c39d803..e76b204 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -225,7 +225,7 @@ $(CELT_SOURCES_ARM_ASM:%.s=%-gnu.S):
$(top_srcdir)/celt/arm/arm2gnu.pl
 
 # convert ARM asm to GNU as format
 %-gnu.S: $(top_srcdir)/%.s
-	$(top_srcdir)/celt/arm/arm2gnu.pl < $< > $@
+	$(top_srcdir)/celt/arm/arm2gnu.pl @ARM2GNU_PARAMS@ < $< > $@
 # For autoconf-modified sources (e.g., armopts.s)
 %-gnu.S: %.s
 	$(top_srcdir)/celt/arm/arm2gnu.pl < $< > $@
diff --git a/celt/arm/arm2gnu.pl b/celt/arm/arm2gnu.pl
index e313904..46e7d0a 100755
--- a/celt/arm/arm2gnu.pl
+++ b/celt/arm/arm2gnu.pl
@@ -26,6 +26,8 @@
 
 my $bigend;  # little/big endian
 my $nxstack;
+my $apple = 0;
+my $symprefix = "";
 
 $nxstack = 0;
 
@@ -39,6 +41,11 @@ while ($ARGV[0] =~ /^-/) {
     $nflag++;
     next;
     }
+    if (/^-apple/) {
+        $apple = 1;
+        $symprefix = "_";
+        next;
+    }
     die "I don't recognize this switch: $_\\n";
 }
 $printit++ unless $nflag;
@@ -79,7 +86,7 @@ while (<>) {
     s/\bINCLUDE[ \t]*([^ \t\n]+)/.include \"$1\"/;
     s/\bGET[ \t]*([^ \t\n]+)/.include \"${ my $x=$1; $x =~ s|\.s|-gnu.S|;
\$x }\"/;
     s/\bIMPORT\b/.extern/;
-    s/\bEXPORT\b/.global/;
+    s/\bEXPORT\b\s*/.global $symprefix/;
     s/^(\s+)\[/$1IF/;
     s/^(\s+)\|/$1ELSE/;
     s/^(\s+)\]/$1ENDIF/;
@@ -135,7 +142,7 @@ while (<>) {
             # won't match the original source file (we could use the .line
             # directive, which is documented to be obsolete, but then gdb will
             # show the wrong line in the translated source file).
-            s/$/;   .arch armv7-a\n   .fpu neon\n   .object_arch armv4t/;
+            s/$/;   .arch armv7-a\n   .fpu neon\n   .object_arch armv4t/ unless
($apple);
         }
     }
 
@@ -157,9 +164,13 @@ while (<>) {
         $prefix = "";
         if ($proc)
         {
-            $prefix = $prefix.sprintf("\t.type\t%s, %%function;
",$proc);
+            $prefix = $prefix.sprintf("\t.type\t%s, %%function;
",$proc) unless ($apple);
+            # Make sure we $prefix isn't empty here (for the $apple case).
+            # We handle mangling the label here, make sure it doesn't match
+            # the label handling below (if $prefix would be empty).
+            $prefix = "; ";
             push(@proc_stack, $proc);
-            s/^[A-Za-z_\.]\w+/$&:/;
+            s/^[A-Za-z_\.]\w+/$symprefix$&:/;
         }
         $prefix = $prefix."\t.thumb_func; " if ($thumb);
         s/\bPROC\b/@ $&/;
@@ -172,7 +183,7 @@ while (<>) {
         my $proc;
         s/\bENDP\b/@ $&/;
         $proc = pop(@proc_stack);
-        $_ = "\t.size $proc, .-$proc".$_ if ($proc);
+        $_ = "\t.size $proc, .-$proc".$_ if ($proc &&
!$apple);
     }
     s/\bSUBT\b/@ $&/;
     s/\bDATA\b/@ $&/;   # DATA directive is deprecated -- Asm guide, p.7-25
@@ -337,6 +348,6 @@ while (<>) {
 }
 #If we had a code section, mark that this object doesn't need an executable
 # stack.
-if ($nxstack) {
+if ($nxstack && !$apple) {
     printf ("   
.section\t.note.GNU-stack,\"\",\%\%progbits\n");
 }
diff --git a/configure.ac b/configure.ac
index 443362f..9fec105 100644
--- a/configure.ac
+++ b/configure.ac
@@ -317,6 +317,14 @@ AS_IF([test x"${enable_asm}" =
x"yes"],[
                     [rtcd_support=ARM"$rtcd_support"],
                     [rtcd_support="no"]
                 )
+                AC_MSG_CHECKING([for apple style tools])
+                AC_PREPROC_IFELSE([AC_LANG_PROGRAM([
+#ifndef __APPLE__
+#error 1
+#endif],[])],
+                    [AC_MSG_RESULT([yes]); ARM2GNU_PARAMS="-apple"],
+                    [AC_MSG_RESULT([no]); ARM2GNU_PARAMS=""])
+                AC_SUBST(ARM2GNU_PARAMS)
             ],
             [
                 AC_MSG_WARN(
-- 
1.8.5.2 (Apple Git-48)

Martin Storsjo wrote:
> +                    [AC_MSG_RESULT([yes]);
ARM2GNU_PARAMS="-apple"],
I'd prefer --apple instead of -apple, but otherwise these patches both 
look good to me.

Does Apple have equivalent machinery to any of the pieces you disabled?
Those were:
- object size calculations so debuggers can tell what function they're in
- architecture flags so an executable that gets linked with this object 
doesn't get tagged as requiring a later architecture than it really does 
(thanks to our runtime detection)
- the non-executable stack flags

Not insisting you add all those things in this patch if there is some 
way to do them, but it would be nice to know if they're impossible to 
do, unnecessary, or simply unimplemented for now.

Both patches applied (with s/-apple/--apple/ in the second one):
https://git.xiph.org/?p=opus.git;h=ac0e294b52ec;a=commitdiff
https://git.xiph.org/?p=opus.git;h=76e831d917ff;a=commitdiff