<div dir="ltr"><br><div class="gmail_extra"><div class="gmail_quote">On Wed, Jun 5, 2013 at 8:34 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Wed, Jun 5, 2013 at 8:06 PM, Eli Friedman <span dir="ltr"><<a href="mailto:eli.friedman@gmail.com" target="_blank">eli.friedman@gmail.com</a>></span> wrote:<br>
</div></div><div class="gmail_quote"><div><div class="h5"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr">Patch attached; fixes PR16144.<div><br></div><div>Currently, the data in TypeLocs consists of a bunch of tightly packed structures, and the structures can become misaligned because there isn't any padding.</div>


<div><br></div><div>There are basically three possible approaches to fixing the alignment issues in TypeLocs:</div><div><br></div><div><div>1) Force every piece of the TypeLoc's data to have alignment 8.</div><div>2) Perform dynamic alignment adjustments.</div>


<div>3) Use #pragma pack to let the compiler know the data is intentionally misaligned.</div></div><div><br></div><div>(1) has a substantial impact on memory usage (something like 1% on Cocoa.h), so I'd like to avoid it if possible.  (2) is the attached patch; it avoids both misaligned loads and unnecessary memory usage.  The primary downside is that TypeLocBuilder becomes a lot more complicated, because it doesn't know in advance where it needs to insert padding.  (3) keeps around to misaligned data: there's a potential performance penalty, it requires being careful not to introduce incorrect accesses to the data, and it's just plain ugly.<br>


</div><div><br></div><div><br></div><div>Two questions to focus on for review: would (3) be a better approach?</div></div></blockquote><div><br></div></div></div><div>I've tried this. We expose pointers into the type source info block in a couple of places (for instance, the array of ParmVarDecl*s on a FunctionTypeLoc) and the misalignment is then exposed to quite a large body of code. Maybe a MisalignedArrayRef<...> would help, but the damage is still not very contained.</div>
</div></blockquote><div><br></div><div>Okay, then I'll stick with (2).</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_quote"><div class="im">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>And is there any way to make the TypeLocBuilder implementation a bit less ugly?</div>
</div></blockquote></div></div><br><div>Perhaps we could remove the guarantee that the child locations of a TypeLoc produced by push<T> are valid, or require some explicit action to fix them?</div>
</blockquote></div><br></div><div class="gmail_extra">A quick run of the regression tests appears to show only one place in the code where we depend on this, and it's very easy to fix there.  That said, I'm not really comfortable making this assumption without completely eliminating temporary TypeLocs from the TypeLocBuilder API: I found another place that depends on this, but the relevant code pattern didn't show up in the regression tests.  I'm thinking I'll commit this patch with the current TypeLocBuilder implementation, and clean up the API in a followup patch.</div>
<div class="gmail_extra"><br></div><div class="gmail_extra">-Eli</div></div>