# New Ticket Created by Andy Dougherty # Please include the string: [perl #53264] # in the subject line of all future correspondence about this issue. # <URL: http://rt.perl.org/rt3/Ticket/Display.html?id=53264 >
On Tue, 22 Apr 2008, [EMAIL PROTECTED] wrote: > Log: > [src] The PMC struct and the Stack_Chunk_t struct aren't isomorphic enough on > non-x86 processors, so the stack chunk recycling scheme needs an explicit > reference count rather than re-using the first int slot in the PObj union. > This appears to fix RT #53170, reported by Seneca Cunningham. Odd. They look pretty similar. Is this something to do with the next_for_GC pointers? Anyway, I had two thoughts looking at this patch: 1. Now both "Buffers" and "Stacks" have ref counts. The ref count in the stack is an explicit member of the structure; the ref count for "Buffers" tags along just before the buffer in memory (see the picture in include/parrot/pobj.h). Would it make sense to make the Buffer "refcount" explicit as well? I wonder if that would simplify some of the header pool logic. I haven't thought deeply about this. 2. There are some casting and type-punning warnings that have, as their ultimate cause, the STACK_DATAP() macro. Getting rid of the type-punning warning gives rise to a cast alignment warning. Looking up a level, the only uses for that macro are to return Stack_Entry_t pointers. Why not be explicit about it in the definition in include/parrot/stacks.h? The attached patch tries to do just that. I haven't tested this, but it looks like it ought to work. diff -r -u parrot-current/include/parrot/stacks.h parrot-andy/include/parrot/stacks.h --- parrot-current/include/parrot/stacks.h 2008-04-23 13:58:26.000000000 -0400 +++ parrot-andy/include/parrot/stacks.h 2008-04-23 16:05:48.000000000 -0400 @@ -34,6 +34,7 @@ union { /* force appropriate alignment of 'data'. If alignment is necessary, assume double is good enough. 27-04-2007. */ void *data; + Stack_Entry_t *stdata; /* Used in src/stack*.c */ #if PARROT_PTR_ALIGNMENT > 1 double d_dummy; #endif @@ -158,7 +159,7 @@ PARROT_API PARROT_WARN_UNUSED_RESULT PARROT_CANNOT_RETURN_NULL -void* stack_prepare_pop(PARROT_INTERP, ARGMOD(Stack_Chunk_t **stack_p)) +Stack_Entry_t * stack_prepare_pop(PARROT_INTERP, ARGMOD(Stack_Chunk_t **stack_p)) __attribute__nonnull__(1) __attribute__nonnull__(2) FUNC_MODIFIES(*stack_p); @@ -166,7 +167,7 @@ PARROT_API PARROT_WARN_UNUSED_RESULT PARROT_CANNOT_RETURN_NULL -void* stack_prepare_push(PARROT_INTERP, ARGMOD(Stack_Chunk_t **stack_p)) +Stack_Entry_t * stack_prepare_push(PARROT_INTERP, ARGMOD(Stack_Chunk_t **stack_p)) __attribute__nonnull__(1) __attribute__nonnull__(2) FUNC_MODIFIES(*stack_p); diff -r -u parrot-current/src/stack_common.c parrot-andy/src/stack_common.c --- parrot-current/src/stack_common.c 2008-04-23 13:57:37.000000000 -0400 +++ parrot-andy/src/stack_common.c 2008-04-23 16:06:31.000000000 -0400 @@ -121,7 +121,7 @@ PARROT_API PARROT_WARN_UNUSED_RESULT PARROT_CANNOT_RETURN_NULL -void* +Stack_Entry_t * stack_prepare_push(PARROT_INTERP, ARGMOD(Stack_Chunk_t **stack_p)) { Stack_Chunk_t * const chunk = *stack_p; @@ -132,7 +132,7 @@ chunk->refcount++; - return STACK_DATAP(new_chunk); + return new_chunk->u.stdata; } @@ -149,7 +149,7 @@ PARROT_API PARROT_WARN_UNUSED_RESULT PARROT_CANNOT_RETURN_NULL -void* +Stack_Entry_t * stack_prepare_pop(PARROT_INTERP, ARGMOD(Stack_Chunk_t **stack_p)) { Stack_Chunk_t * const chunk = *stack_p; @@ -166,7 +166,7 @@ if ((*stack_p)->refcount < chunk->refcount) (*stack_p)->refcount = chunk->refcount; - return STACK_DATAP(chunk); + return chunk->u.stdata; } /* diff -r -u parrot-current/src/stacks.c parrot-andy/src/stacks.c --- parrot-current/src/stacks.c 2008-04-23 13:57:38.000000000 -0400 +++ parrot-andy/src/stacks.c 2008-04-23 15:59:28.000000000 -0400 @@ -60,7 +60,6 @@ mark_stack(PARROT_INTERP, ARGMOD(Stack_Chunk_t *chunk)) { for (; ; chunk = chunk->prev) { - void **entry_data; Stack_Entry_t *entry; pobject_lives(interp, (PObj *)chunk); @@ -68,8 +67,7 @@ if (chunk == chunk->prev) break; - entry_data = STACK_DATAP(chunk); - entry = (Stack_Entry_t *)entry_data; + entry = chunk->u.stdata; switch (entry->entry_type) { case STACK_ENTRY_PMC: @@ -150,7 +148,6 @@ stack_entry(PARROT_INTERP, ARGIN(Stack_Chunk_t *stack), INTVAL depth) { Stack_Chunk_t *chunk; - void **entry; size_t offset = (size_t)depth; /* For negative depths, look from the bottom of the stack up. */ @@ -175,9 +172,7 @@ if (chunk == chunk->prev) return NULL; - /* this looks bad, but it avoids a type-punning warning */ - entry = STACK_DATAP(chunk); - return (Stack_Entry_t *)entry; + return chunk->u.stdata; } /* -- Andy Dougherty [EMAIL PROTECTED]