[PATCH] Remove copies of MCSubtargetInfo from MCRelaxableFragment

[Rafael Espíndola]

Duncan, what is the memory savings of the original patch? You still
store a MCSubtargetInfo reference and a the bits. Can't you at least
drop the MCSubtargetInfo and reconstruct it just before calling
encodeInstruction with some "central" MCSubtargetInfo?

On 10 July 2015 at 20:45, Eric Christopher <echristo at gmail.com> wrote:
>
>>
>> > 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.
>> > >>>>>>
>> > >>>>>>
>> >
>>
>