On 06/11/2018 19:43, Richard Earnshaw (lists) wrote: > On 06/11/2018 18:18, Segher Boessenkool wrote: >> Hi Richard, >> >> On Tue, Nov 06, 2018 at 11:46:53AM +0000, Richard Earnshaw (lists) wrote: >>> On 06/11/2018 01:40, Segher Boessenkool wrote: >>>> Hi Richard, >>>> >>>> On Mon, Nov 05, 2018 at 10:09:30AM +0000, Richard Earnshaw (lists) wrote: >>>>>>>> Shouldn't you be able to do this per function at least? >>>>>>> >>>>>>> do what per function? track speculation? >>>>>> >>>>>> disable shrink-wrapping only when any speculation was there >>>>>> (this is about __bultin_speculation_safe_value, no?) >>>>> >>>>> Only indirectly. This is about the tracking code that tracks >>>>> conditional branches and propagates that information through call/return >>>>> sequences. Shrink wrapping messes with the prologue/epilogue sequences >>>>> after the speculation tracking pass has run and unknowingly deletes some >>>>> of the additional code that was previously inserted by the tracking pass. >>>> >>>> Do you have an example of this? Shrink-wrapping does not generally >>>> delete any code. >>>> >>> >>> Well it generates new 'light-weight' prologue and epilogue sequences for >>> the 'shrunk' code path that lack the establishment of the tracker >>> register and doesn't know how to move the existing sequence to the new >>> entry sequence. >> >> Ah, so the shrink-wrapping code is not deleting anything at all (just >> not adding it). Gotcha :-) > > Well.... you could argue that it deleted the tracker update for the case > where the branch was not taken, and it also deleted the part of the > prologue where the tracker state was restored into SP before the return.
Duh! epilogue, of course. R. > But I'm being picky... :-) > >> >> [ snip example code; thanks, that helped ] >> >>> I'm not asking that shrink wrapping be updated to handle all this; in >>> fact, I'm not sure it's that easy to do as the branch patterns and >>> simple-return patterns aren't set up to handle this. >> >> One thing you could do is make shrink-wrap aware what part of the code >> needs the speculation tracking parts of the prologue. You could do this >> by making a separate shrink-wrapping component for it, or you can do it >> by marking the places needing it as needing the full prologue, e.g. by >> emitting a fake call into it (and not outputting any code for that call). >> The latter does cause a stack frame to be emitted even when it wouldn't >> otherwise, unfortunately. The separate shrink-wrapping approach should >> work beautifully as far as I see. >> >> > > There are number of optimizations that are worth investigation with the > tracking support; but whether they'll notably improve performance I'm > not sure. Tracking just just expensive and the main problem is the > serialization of the state, which limits the core's ability to reorder > stuff internally. > > R. > >