Finally: Mac Intel patches

Mikael Pettersson mikpe@REDACTED
Wed Aug 23 18:42:52 CEST 2006


Joel Reymont writes:
 > Apply to http://www.it.uu.se/research/group/hipe/snapshots/ 
 > otp-0804.tar.gz with Mikael's patches. Sorry it took me so long but I  
 > didn't use version control when porting HiPE earlier.
 > 
 > All tests pass! Apply as follows:
 > 
 > cd otp-0804 && patch -p1 < patch-otp-0804-darwin-x86

I've reviewed your changes and they look mostly OK.
I've changed some things (see commentary below) and split
the patch into three logical groups. Download them from
the URL above as:

patch-otp-0804-5-darwin-x86-fp-exceptions
patch-otp-0804-6-darwin-x86-asm-fixes
patch-otp-0804-7-darwin-x86-sigaction

You need all four earlier patches too, including patch-otp-0804-3-freebsd-fixes
which you appear to not have applied.

This works on Linux/x86 and Solaris/x86.
Let me know if they still work for you or if I broke anything.

Comments about your SSE2 exception handling changes:

>diff -rN -u old-otp-0804-1/erts/configure new-otp-0804/erts/configure
>--- old-otp-0804-1/erts/configure	2006-08-22 23:25:17.000000000 +0100
>+++ new-otp-0804/erts/configure	2006-08-22 23:25:22.000000000 +0100
>@@ -6881,7 +6881,7 @@
>     __asm__ __volatile__("ldmxcsr %0" : : "m"(mxcsr));
> }
> 
>-#if defined(__x86_64__)
>+#if defined(__x86_64__) || defined(__DARWIN__)
> static inline int cpu_has_sse2(void) { return 1; }
> #else /* !__x86_64__ */
> /*

I don't think there's any guarantee that Darwin only runs on
SSE2-capable x86-32 processors, so omitting the test seems dangerous.
The runtime test should work on all x86 processors, so I'm going to
let Darwin on x86-32 test at runtime.

>+#elif defined(__DARWIN__) && defined(__i386__)
>+    mcontext_t mc = uc->uc_mcontext;
>+    i386_float_state_t fpstate = mc->fs;
>+    fpstate.fpu_mxcsr = 0x1F80;
>+    *((unsigned short *)&fpstate.fpu_fsw) &= ~0xFF;
> #elif defined(__DARWIN__)
>     mcontext_t mc = uc->uc_mcontext;
>     mc->ss.srr0 += 4;

You're copying the float state into fpstate and updating the copy
without flushing the changes back to the original. This cannot be
right. fpstate should be a pointer to mc->fs.

What is the concrete type of the fpu_fsw field? I really don't
want to update it via a pointer cast like this.

>diff -rN -u old-otp-0804-1/erts/emulator/sys/unix/sys_float.c new-otp-0804/erts/emulator/sys/unix/sys_float.c
>--- old-otp-0804-1/erts/emulator/sys/unix/sys_float.c	2006-08-22 23:25:17.000000000 +0100
>+++ new-otp-0804/erts/emulator/sys/unix/sys_float.c	2006-08-22 23:25:23.000000000 +0100
>@@ -481,6 +491,13 @@
>     regs[PT_NIP] += 4;
>     regs[PT_FPSCR] = 0x80|0x40|0x10;	/* VE, OE, ZE; not UE or XE */
> #endif
>+#elif defined(__DARWIN__) && defined(__i386__)
>+    mcontext_t mc = uc->uc_mcontext;
>+	  if (mc->fs.fpu_mxcsr & 0x000F) {
>+			mc->fs.fpu_mxcsr &= ~(0x003F|0x0680);
>+			skip_sse2_insn(mc);
>+		}
>+  	*((unsigned short *)&mc->fs.fpu_fsw) &= ~0xFF;
> #elif defined(__DARWIN__)
>     mcontext_t mc = uc->uc_mcontext;
>     mc->ss.srr0 += 4;

The mxcsr mask should be 0x000D.

Comments about your HiPE/x86 changes (apart from sigaction/mach_override):

>     map_start = mmap(map_hint, map_bytes,
> 		     PROT_EXEC|PROT_READ|PROT_WRITE,
>-		     MAP_PRIVATE|MAP_ANONYMOUS
>+		     MAP_PRIVATE
>+#ifndef __DARWIN__
>+         |MAP_ANONYMOUS
>+#else
>+         |MAP_ANON
>+#endif 
> #ifdef __x86_64__
> 		     |MAP_32BIT
> #endif

This was fixed recently in a cleaner way by patch-otp-0804-3-freebsd-fixes.

>diff -rN -u old-otp-0804-1/erts/emulator/hipe/hipe_x86_asm.m4 new-otp-0804/erts/emulator/hipe/hipe_x86_asm.m4
>--- old-otp-0804-1/erts/emulator/hipe/hipe_x86_asm.m4	2006-08-22 23:25:17.000000000 +0100
>+++ new-otp-0804/erts/emulator/hipe/hipe_x86_asm.m4	2006-08-22 23:25:23.000000000 +0100
>@@ -17,6 +17,28 @@
> `#define LEAF_WORDS	'LEAF_WORDS
> 
> /*
>+ * Workarounds for Darwin.
>+ */
>+ifelse(OPSYS,darwin,``
>+/* Darwin */
>+#define TEXT        .text
>+#define JOIN(X,Y)       X##Y
>+#define CSYM(NAME)      JOIN(_,NAME)
>+#define ASYM(NAME)      CSYM(NAME)
>+#define GLOBAL(NAME)	.globl NAME
>+#define SET_SIZE(NAME)	/*empty*/
>+#define TYPE_FUNCTION(NAME)	/*empty*/
>+'',``
>+/* Not Darwin */
>+#define TEXT        .section ".text"
>+#define CSYM(NAME)  NAME
>+#define ASYM(NAME)  NAME
>+#define GLOBAL(NAME)	.globl NAME
>+#define SET_SIZE(NAME)	.size NAME,.-NAME
>+#define TYPE_FUNCTION(NAME)	.type NAME,@REDACTED
>+'')dnl
>+
>+/*
>  * Reserved registers.
>  */
> `#define P	%ebp'

The current x86 assembly code uses .global not .globl.

The hipe_x86_bifs.m4 and hipe_x86_glue.S changes look good.

Comments about your HiPE/x86 sigaction/mach_override changes:

>diff -rN -u old-otp-0804-1/erts/emulator/Makefile.in new-otp-0804/erts/emulator/Makefile.in
>--- old-otp-0804-1/erts/emulator/Makefile.in	2006-08-22 23:25:17.000000000 +0100
>+++ new-otp-0804/erts/emulator/Makefile.in	2006-08-22 23:25:23.000000000 +0100
>@@ -705,7 +705,7 @@
> 		$(OBJDIR)/erl_mtrace_sys_wrap.o
> 
> HIPE_ultrasparc_OBJS=$(OBJDIR)/hipe_sparc.o $(OBJDIR)/hipe_sparc_glue.o $(OBJDIR)/hipe_sparc_bifs.o $(OBJDIR)/hipe_sparc_stack.o
>-HIPE_x86_OBJS=$(OBJDIR)/hipe_x86.o $(OBJDIR)/hipe_x86_glue.o $(OBJDIR)/hipe_x86_bifs.o $(OBJDIR)/hipe_x86_signal.o $(OBJDIR)/hipe_x86_stack.o
>+HIPE_x86_OBJS=$(OBJDIR)/hipe_x86.o $(OBJDIR)/hipe_x86_glue.o $(OBJDIR)/hipe_x86_bifs.o $(OBJDIR)/hipe_x86_signal.o $(OBJDIR)/hipe_x86_stack.o $(OBJDIR)/mach_override.o
> HIPE_amd64_OBJS=$(OBJDIR)/hipe_amd64.o $(OBJDIR)/hipe_amd64_glue.o $(OBJDIR)/hipe_amd64_bifs.o $(OBJDIR)/hipe_x86_signal.o $(OBJDIR)/hipe_x86_stack.o
> HIPE_ppc_OBJS=$(OBJDIR)/hipe_ppc.o $(OBJDIR)/hipe_ppc_glue.o $(OBJDIR)/hipe_ppc_bifs.o $(OBJDIR)/hipe_ppc_stack.o
> HIPE_ppc64_OBJS=$(HIPE_ppc_OBJS)

Adding mach_override.o needs to be conditional on OPSYS=darwin.
I introduced an auxiliary make variable to express that.

>diff -rN -u old-otp-0804-1/erts/emulator/hipe/hipe_x86_signal.c new-otp-0804/erts/emulator/hipe/hipe_x86_signal.c
...
>+#ifdef __DARWIN__
>+
>+static int (*__next_sigaction)(int, const struct sigaction*, struct sigaction*);
>+static int (*__orig_sigaction)(int, const struct sigaction*, struct sigaction*);
>+#define init_done()	(__next_sigaction != 0)
>+/*#define INIT()	do_init();*/
>+#define _NSIG NSIG
>+#define INIT()	do { if( !init_done() ) do_init(); } while(0)

__orig_sigaction is unused. I removed it.

>+#ifdef __DARWIN__
>+
>+#include "mach_override.h"
>+#include <dlfcn.h>
>+

The reason you use mach_override.h is that the dlfcn functionality
doesn't appear to work on Darwin as it does on Solaris or Linux.
You don't use dlsym() etc, so why include dlfcn.h?

I rearranged the hipe_x86_signal.c changes to make them smaller.

/Mikael



More information about the erlang-questions mailing list