[Gc] [PATCH] better thread support on Darwin (PPC)

Fergus Henderson fjh at cs.mu.oz.au
Tue Dec 16 14:26:00 PST 2003


Here's a few comments from a quick glance at the code in your patch
(I didn't review it in detail).

On 16-Dec-2003, Andrew Begel <abegel at eecs.berkeley.edu> wrote:
> --- gc6.3alpha2/darwin_stop_world.c	Thu Dec 11 03:18:05 2003
> +++ gc6.3alpha2-andy/darwin_stop_world.c	Mon Dec 15 19:09:37 2003
> @@ -16,101 +14,224 @@
>  */
>  #define PPC_RED_ZONE_SIZE 224
>  
> +/* I have no idea how big this should be. Or if I386 uses a red zone at all. */
> +#define I386_RED_ZONE_SIZE 224

I don't think the i386 uses a red zone at all...
so I think the right value here is zero.

> +unsigned int FindTopOfStack(unsigned int stack_start) {
> +  StackFrame	*frame;
> +  
> +  if (stack_start == 0) {
> +    __asm__ volatile("lwz	%0,0(r1)" : "=r" (frame));
> +  } else {
> +    frame = (StackFrame *)stack_start;
> +  }
> +
> +  do {
> +    frame = (StackFrame*)frame->savedSP;
> +  } while (frame != NULL && 
> +	   (!(((frame->savedLR & ~3) == 0) || ((~(frame->savedLR) & ~3) == 0))));

That is a very complicated condition.
I would suggest writing it as

  } while (frame != NULL
	   && (frame->savedLR & ~3) != 0
	   && (frame->savedLR & ~3) != ~3);

and adding a comment explaining when the second and third conditions
can occur and what they mean.

> +#warning This is completely untested and likely will not work

"#warning" is not standard C.

You could use "#error" here instead, I suppose.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
The University of Melbourne         |  of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- the last words of T. S. Garp.


More information about the Gc mailing list