[PATCH] Remove copies of MCSubtargetInfo from MCRelaxableFragment

[Eric Christopher]

>
> > 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.
> >
>
> Eric, do you prefer the STI cache in patch 0005?
>

I do, yes. At least as a temporary workaround.


>
> I agree with Rafael that it's pretty ugly if it's not actually
> necessary (although I haven't confirmed that every callsite is okay
> with only the feature bits, it seems likely).
>
> Is `FeatureBitset` really that much of an implementation detail?
> Every `MCSubtargetInfo` has one in its public API...
>
>
I mean, we could pass around FeatureBitset instead of TargetSubtargetInfo
through the backend as well. Save a lot of space.

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.

-eric


>
> > -eric
> >
> > >
> > > On 10 July 2015 at 09:42, Rafael Espíndola <rafael.espindola at gmail.com>
> wrote:
> > >> patch 4 LGTM
> > >>
> > >> On 10 July 2015 at 09:37, Rafael Espíndola <
> rafael.espindola at gmail.com> wrote:
> > >>> patch 3 LGTM.
> > >>>
> > >>> On 10 July 2015 at 09:29, Rafael Espíndola <
> rafael.espindola at gmail.com> wrote:
> > >>>> Patch 2 LGTM.
> > >>>>
> > >>>> On 10 July 2015 at 09:27, Rafael Espíndola <
> rafael.espindola at gmail.com> wrote:
> > >>>>> Patch 1 LGTM.
> > >>>>>
> > >>>>>
> > >>>>> On 8 July 2015 at 20:30, Duncan P. N. Exon Smith <
> dexonsmith at apple.com> wrote:
> > >>>>>> Last I checked my `-g -flto` workload, `MCRelaxableFragment` was
> taking
> > >>>>>> up 4.7% of the heap during LTO.  Since these aren't debug info
> related,
> > >>>>>> the relative cost is even higher without `-g` (I figure 10% or
> so?).
> > >>>>>> The struct layout contained a shock: over half of
> > >>>>>> `sizeof(MCRelaxableFragment)` comes from a local copy (!?) of
> > >>>>>> `MCSubtargetInfo`.
> > >>>>>>
> > >>>>>> It was more roundabout than I expected to fix, but this patch
> series
> > >>>>>> does it.  0001 removes the copy of `MCSchedModel` from
> > >>>>>> `MCSubtargetInfo`, 0002-0004 change the `MCSubtargetInfo` API to
> prove
> > >>>>>> that only the feature bits get mutated, and 0005 moves the copies
> of
> > >>>>>> `MCSubtargetInfo` to a cache in `MCContext`.
> > >>>>>>
> > >>>>>> I'd love a pre-commit sanity check from someone for two of these:
> > >>>>>>  - 0001 (the object for `MCSchedModel::GetDefaultMCSubtarget()`
> doesn't
> > >>>>>>    need to be defined in the header for some reason, does it?) and
> > >>>>>>  - 0003 (I'm touching *all* the targets...)
> > >>>>>>
> > >>>>>> and I've provided the rest of the series (and a combined
> all.patch) for
> > >>>>>> context.
> > >>>>>>
> > >>>>>>
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150711/8456705a/attachment.html>