<div dir="ltr"><br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
> You could, but unless there's a compelling reason I'd rather not. It seems like we'd be passing an implementation detail around rather than something that gives us access to what we want.<br>
><br>
<br>
Eric, do you prefer the STI cache in patch 0005?<br></blockquote><div><br></div><div>I do, yes. At least as a temporary workaround.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
I agree with Rafael that it's pretty ugly if it's not actually<br>
necessary (although I haven't confirmed that every callsite is okay<br>
with only the feature bits, it seems likely).<br>
<br>
Is `FeatureBitset` really that much of an implementation detail?<br>
Every `MCSubtargetInfo` has one in its public API...<br>
<br></blockquote><div><br></div><div>I mean, we could pass around FeatureBitset instead of TargetSubtargetInfo through the backend as well. Save a lot of space.</div><div><br></div><div>I jest, a little, but it's the same thing (quite literally in a lot of cases). Just because our interfaces are pretty crappy doesn't mean we should just avoid them.</div><div><br></div><div>-eric</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> -eric<br>
><br>
> ><br>
> > On 10 July 2015 at 09:42, Rafael Espíndola <<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>> wrote:<br>
> >> patch 4 LGTM<br>
> >><br>
> >> On 10 July 2015 at 09:37, Rafael Espíndola <<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>> wrote:<br>
> >>> patch 3 LGTM.<br>
> >>><br>
> >>> On 10 July 2015 at 09:29, Rafael Espíndola <<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>> wrote:<br>
> >>>> Patch 2 LGTM.<br>
> >>>><br>
> >>>> On 10 July 2015 at 09:27, Rafael Espíndola <<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>> wrote:<br>
> >>>>> Patch 1 LGTM.<br>
> >>>>><br>
> >>>>><br>
> >>>>> On 8 July 2015 at 20:30, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>> wrote:<br>
> >>>>>> Last I checked my `-g -flto` workload, `MCRelaxableFragment` was taking<br>
> >>>>>> up 4.7% of the heap during LTO.  Since these aren't debug info related,<br>
> >>>>>> the relative cost is even higher without `-g` (I figure 10% or so?).<br>
> >>>>>> The struct layout contained a shock: over half of<br>
> >>>>>> `sizeof(MCRelaxableFragment)` comes from a local copy (!?) of<br>
> >>>>>> `MCSubtargetInfo`.<br>
> >>>>>><br>
> >>>>>> It was more roundabout than I expected to fix, but this patch series<br>
> >>>>>> does it.  0001 removes the copy of `MCSchedModel` from<br>
> >>>>>> `MCSubtargetInfo`, 0002-0004 change the `MCSubtargetInfo` API to prove<br>
> >>>>>> that only the feature bits get mutated, and 0005 moves the copies of<br>
> >>>>>> `MCSubtargetInfo` to a cache in `MCContext`.<br>
> >>>>>><br>
> >>>>>> I'd love a pre-commit sanity check from someone for two of these:<br>
> >>>>>>  - 0001 (the object for `MCSchedModel::GetDefaultMCSubtarget()` doesn't<br>
> >>>>>>    need to be defined in the header for some reason, does it?) and<br>
> >>>>>>  - 0003 (I'm touching *all* the targets...)<br>
> >>>>>><br>
> >>>>>> and I've provided the rest of the series (and a combined all.patch) for<br>
> >>>>>> context.<br>
> >>>>>><br>
> >>>>>><br>
><br>
<br>
</blockquote></div></div>