On Wed, Oct 14, 2009 at 2:10 AM, Wesley W. Terpstra <span dir="ltr"><<a href="mailto:wesley@terpstra.ca">wesley@terpstra.ca</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div>Ok, but it is definitely updateCrossMap that is doing the unaligned accesses.</div><br>gdb says the address of the trap is...<br>(gdb) break *(void*)0x0000000120d843ac<br>Breakpoint 1 at 0x120d843ac: file gc/generational.c, line 373.<br>
=> s->generationalMaps.crossMap[cardIndex] = (GC_crossMapElem)offset;<br></blockquote><div><br>... shockingly, gdb was correct. It took me some time to wrap my mind around this, but there is indeed a bug in that line. Or rather, that line became buggy when r4683 tried to quell a warning. The warning was harmless, but the "fix" wasn't.<br>
<br>Here's what is going on:<br> typedef uint8_t GC_crossMapStart __attribute__ ((aligned(4));<br>defines a type that is a uint8_t, which happens to be aligned on a 4-byte boundary. That's all fine and dandy. Make a pointer type to the object, well that pointer is 4-byte aligned. Swell.<br>
<br>Now deference that pointer at [1].<br><br>Uhm.<br><br>Well, the object type pointed to is aligned(4), so the (pointer+1) must be 4-byte aligned. I'm an alpha and loading a byte is expensive, but no problem, it's word aligned! Let's just load it using a word32 memory access. BAM! unaligned access.<br>
<br>Morale of the story? Don't mess around with __attribute__ ((aligned))! Two of the three places it is used in MLton are wrong and I'm not 100% sure the remaining use (pointer) is safe either. I assume that "pointer" is used only to refer to objects on the heap. That means that on an alpha it is actually 8-byte aligned. Telling the alpha it's less aligned than it actually is will either degrade performance or have no effect at all.<br>
</div></div><br>When you cast pointers you might get a warning, but gcc is also doing the 'right thing' for MLton's purposes. If I cast a char* to an int* gcc assumes that I know what I'm doing and that the char* pointer was really aligned to an int after all. That's why it issues a warning, to let you know it just increased its alignment assumptions. Telling gcc that the char* was 4-byte aligned quells the warning (on a 32-bit system) but doesn't improve the assembler. All it does is introduce potential bugs if you ever derefence that char* pointer at say [1] [2] or [3]. So there is absolutely nothing to gain (performance-wise) by declaring these types as aligned and everything to lose (correctness-wise).<br>
<br>As for the warnings, they aren't even warnings that are enabled by default in C! We turned them on with -Wcast-align. They are off by default because lots of correct C code (including ours) do casts like this. The align(4) hack also doesn't help on 64-bit ports... I get flooded with these warnings on ia64 and alpha.<br>
<br>I am going to unilaterally remove all __attribute__ ((align))s and quell this class of warning by removing -Wcast-align. After my rant I assume no one dares to object. :)<br><br>