[erlang-patches] Few HiPE patches

[Mikael Pettersson]

Martti Kuparinen writes:
 > You were absolutely right, those HiPE patches were not needed, I only
 > left these two. At least NetBSD does not have MAP_ANONYOUS but MAP_ANON
 > according to mmap(2). Happy with these?
 > 
 > 
 > --- erts/emulator/hipe/hipe_arm.c.orig  2010-06-24 07:25:07.000000000
 > +0300
 > +++ erts/emulator/hipe/hipe_arm.c       2010-06-24 07:25:43.000000000
 > +0300
 > @@ -73,6 +73,10 @@
 >  #define in_area(ptr,start,nbytes)      \
 >         ((unsigned long)((char*)(ptr) - (char*)(start)) < (nbytes))
 >  
 > +#if !defined(MAP_ANONYMOUS) && defined(MAP_ANON)
 > +#define MAP_ANONYOUS MAP_ANON
 > +#endif
 > +
 >  static void *new_code_mapping(void)
 >  {
 >      return mmap(0, SEGMENT_NRBYTES,
 > 
 > 

OK

 > --- erts/emulator/hipe/hipe_sparc.c.orig        2010-06-24
 > 07:26:50.000000000 +0
 > 300
 > +++ erts/emulator/hipe/hipe_sparc.c     2010-06-24 07:27:25.000000000
 > +0300
 > @@ -130,6 +130,10 @@
 >  #define ALLOC_CODE_STATS(X)    do{}while(0)
 >  #endif
 >  
 > +#if !defined(MAP_ANONYMOUS) && defined(MAP_ANON)
 > +#define MAP_ANONYMOUS MAP_ANON
 > +#endif
 > +
 >  static void morecore(unsigned int alloc_bytes)
 >  {
 >      unsigned int map_bytes;
 > 
 > 

OK

 > > reasonable (BSD-specific snippets for signal handling)
 > 
 > I looked at erts/emulator/hipe/hipe_x86_signal.c again and I have one
 > question: INIT is defined inside #ifdef...#endif but used without any
 > #ifdef..#endif check. Why? Without
 > 
 > 
 > @@ -326,7 +327,9 @@
 >      struct sigaction sa;
 >      int i;
 >  
 > +#ifndef __NetBSD__
 >      INIT();
 > +#endif
 >  
 >      hipe_sigaltstack_init();
 > 
 > 
 > I get an error like this
 > 
 > 
 > obj/i386-unknown-netbsdelf5.99.30/opt/smp/hipe_x86_signal.o: In function
 > `hipe_signal_init':
 > hipe_x86_signal.c:(.text+0x9): undefined reference to `INIT'

That's because the target in question should have selected one of the
#if .. #endif sections for its sigaction() wrapper implementation,
which would also define the INIT() macro.  By not selecting any such
section you don't get an INIT() macro definition, but you also don't
get any sigaction() wrapper, which is the whole point of this file.

The sigaction() wrapper to enforce SA_ONSTACK is required on x86/amd64.
If a port to a target omits it then that port is unsafe and unsupported.

 > > to questionable (why do you patch the BEAM code generator?).
 > 
 > I assume you are talking about the now-removed patch-ah
 > (http://pkgsrc-wip.cvs.sourceforge.net/viewvc/pkgsrc-wip/wip/erlang/patches/patch-ah?revision=1.3) which was
 > 
 > --- lib/compiler/src/v3_codegen.erl.orig	2010-06-11 18:30:11.000000000
 > +0300
 > +++ lib/compiler/src/v3_codegen.erl	2010-06-17 10:19:44.000000000 +0300
 > @@ -1520,6 +1520,8 @@
 >      Sizes = filter(fun({_,{integer,0}}) -> false;
 >  		      (_) -> true end, Sizes0),
 >      case Sizes of
 > +       [] ->
 > +           {bs_init2,[{integer,0}]};
 >  	[{1,_}|_] ->
 >  	    {bs_init_bits,cg_binary_bytes_to_bits(Sizes, [])};
 >  	[{8,_}|_] ->
 > 
 > 
 > I've removed this now.

I was.  Maybe it is a good change, but we need the original author's
comments about it and a review by the BEAM compiler folks.  And it's
not a HiPE patch so it should be handled separately.