auto::alignptrs: Eliminate this config step

[jkeenan]

In TT #498, Andy Dougherty noted that the result of configuration step auto::alignptrs is used in only two PARROT_ASSERT statements in src/pmc/unmanagedstruct.pmc:

 127 static char * 
 128 char_offset_key(PARROT_INTERP, PMC *pmc, PMC *key, int *type) 
 129 {
 ...
 186             if (*type == enum_type_struct_ptr) {
 187                 /* that is either a pointer */
 188                 PARROT_ASSERT((PTR2INTVAL(p) & (PARROT_PTR_ALIGNMENT - 1)) ==      0);
 189                 VTABLE_set_pointer(interp, init, *(void**)p);
 190             }
 ...
 196         else if (init->vtable->base_type == enum_class_ManagedStruct
 197              && *type                    == enum_type_struct_ptr) {
 198             /* a nested struct pointer belonging to us
 199              * p is the location of the struct pointer in the
 200              * outer struct, the inner is at PMC_data(init) */
 201 
 202             PARROT_ASSERT((PTR2INTVAL(p) & (PARROT_PTR_ALIGNMENT - 1)) == 0);
 203             *(void **)p = VTABLE_get_pointer(interp, init);
 204         }
 205 
 206         return char_offset_key(interp, init, next, type);
 207     }

So tonight I created the noalignptrs branch to see what would happen if we did indeed eliminate this step. So far, it has built and passed make test.

[jkeenan]

Replying to jkeenan:

The only failure I got during make fulltest was the test of examples/pir/quine_ord.pir in t/examples/pir.t. The output of this test is massive, so I'm not going to post it. Investigating.

kid51

[jkeenan]

Replying to jkeenan:

Replying to jkeenan: The only failure I got during make fulltest was the test of examples/pir/quine_ord.pir in t/examples/pir.t.

Ah, not my fault! This is failing in trunk, observed in r43153.

[jkeenan]

diff between noalignptrs branch and trunk

[jkeenan]

I've attached a diff between branch and trunk which should be considered as a first draft of a patch implementing the removal of auto::alignptrs. So far, all I've done is to comment out the two PARROT_ASSERT lines.

Index: src/pmc/unmanagedstruct.pmc
===================================================================
--- src/pmc/unmanagedstruct.pmc (revision 43151)
+++ src/pmc/unmanagedstruct.pmc (revision 43153)
@@ -185,7 +185,9 @@
             /* now point ptr to the real data */
             if (*type == enum_type_struct_ptr) {
                 /* that is either a pointer */
+                /*
                 PARROT_ASSERT((PTR2INTVAL(p) & (PARROT_PTR_ALIGNMENT - 1)) == 0);
+                */
                 VTABLE_set_pointer(interp, init, *(void**)p);
             }
 
@@ -199,7 +201,9 @@
              * p is the location of the struct pointer in the
              * outer struct, the inner is at PMC_data(init) */
 
+            /*
             PARROT_ASSERT((PTR2INTVAL(p) & (PARROT_PTR_ALIGNMENT - 1)) == 0);
+            */
             *(void **)p = VTABLE_get_pointer(interp, init);
         }

I suspect that deleting those two lines, including their reference to PTR2INTVAL is probably not the best way to proceed, even if it works. Should there be some other PARROT_ASSERT there?

Suggestions?

Thank you very much.
kid51

[jkeenan]

Paraphrasing some comments from plobsing on #parrot:

The assertions at lines 188 and 202 of src/pmc/unmanagedstruct.pmc (trunk) check that the pointers are aligned to a 2**n char boundary (required on some obscure platform I'm sure).

(But it appears we have yet to encounter such a platform. -- kid51)

On the one hand, parrot doesn't check this elsewhere; on the other, UnManagedStruct is the closest users come to directly managing pointers, so extra checks might be good. We could say: "If you run a platform that breaks if you access pointers that aren't aligned and you break it using UnManagedStruct, that's your fault." In a way, UnManagedStruct is part of the escape hatch of the VM abstraction.

Seeking more comments from knowledgeable sources.

kid51

[doughera]

Replying to jkeenan:

Paraphrasing some comments from plobsing on #parrot: The assertions at lines 188 and 202 of src/pmc/unmanagedstruct.pmc (trunk) check that the pointers are aligned to a 2**n char boundary (required on some obscure platform I'm sure). (But it appears we have yet to encounter such a platform. -- kid51)

Actually it's a reasonably common requirement on RISC processors. SPARC is one well-known example, but there are many others. It's more that x86 doesn't require any such alignment constraints, so many programmers are unfamiliar with them.

I suspect that these checks are a holdover from when Parrot tried to manage alignment manually in many cases. It does less of that now. If pointers are allocated and used appropriately, then the compiler will guarantee that they are suitably aligned for their stated purpose, and no assert() checks are required.

I'd say "Just delete the lines."

[jkeenan]

Replying to doughera:

I'd say "Just delete the lines."

Thanks, Andy. I will do so and merge the noalignptrs branch into trunk after tomorrow's release.

kid51

[jkeenan]

Merged into trunk at r43490. I will close ticket no later than this weekend.

Thank you very much.

kid51

[jkeenan]

No complaints; closing ticket.

Thank you very much.

kid51