# 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]
