[PATCH] Improve performance of calculateDbgValueHistory

[David Blaikie]

Best to split it out at least and we can review it separately. Thanks!

(While this may sound like it creates a lot of busy work, reviewing smaller
patches is exponentially easier so can result in faster turn around :))
On Aug 5, 2014 12:19 PM, "Frédéric Riss" <friss at apple.com> wrote:

> Oh, you’re right, this hunk isn’t explained. It’s just a general cleanup
> that I did while reading the code. If I’m not mistaken, the IsInEpilogue
> removal is functionally equivalent to the old version. I just used an early
> exit instead of continuing iterating the loop with a true IsInEpilogue
> variable. I also added !MI.isDebugValue() as this function checks for MIs
> that clobber registers, which can’t be the case of DebugValues I think. I
> also changed the comment to reflect what the function really does.
>
> This can be split out in a separate cleanup patch if needed, or dropped
> entirely if I missed something…
>
> Fred
>
> On 05 Aug 2014, at 21:03, David Blaikie <dblaikie at gmail.com> wrote:
>
> > (+Alexey who made the recent changes)
> >
> > Frédéric - This looks like general goodness except I'm a bit confused
> > about the other changes you made to collectChangingRegs (removing
> > IsEpilogue and associated changes). What was the purpose of those
> > changes? Could they be separated from this commit to keep the change
> > simpler and so it has a clear, singular purpose?
> >
> > - David
> >
> > On Tue, Aug 5, 2014 at 11:33 AM, Frédéric Riss <friss at apple.com> wrote:
> >> Hi,
> >>
> >> In r210492 the logic of calculateDbgValueHistory was improved to end
> register variable live ranges at the end of MBB conditionally on the fact
> that the register was or not clobbered by the function body. This requires
> 2 passes over all the operands of the function. The first pass computes the
> std::set of all clobbered registers of the function and in the second one
> we intersect the global set with the one recomputed for the current
> Instruction.
> >>
> >> The logic introduced in r210492 is necessarily a bit slower than before
> as it involves walking twice the full list of MachineOperands for the
> function. The new information is necessary, but the performance of debug
> info emission is degraded by more than 10% on some benchmarks.
> >>
> >> The biggest performance hit is due to the
> construction/iteration/destruction of std::set for each real
> MachineInstruction in the second pass. We can avoid the temporary sets by
> using a lambda that captures the global set and operates directly on that.
> I tried various other approaches and this one gets the best performance
> while still being quite readable.
> >>
> >> Here are some performance numbers (5 runs each time):
> >>
> >> Current mainstream
> >>  ---User Time---   --System Time--   --User+System--   ---Wall Time---
>  --- Name ---
> >>  0.0508 ( 71.0%)   0.0262 ( 50.3%)   0.0770 ( 62.3%)   0.0772 ( 62.3%)
>  Debug Info Emission
> >>  0.0515 ( 71.1%)   0.0267 ( 50.7%)   0.0782 ( 62.5%)   0.0780 ( 62.2%)
>  Debug Info Emission
> >>  0.0506 ( 71.3%)   0.0262 ( 50.5%)   0.0768 ( 62.5%)   0.0768 ( 62.5%)
>  Debug Info Emission
> >>  0.0511 ( 70.6%)   0.0266 ( 50.5%)   0.0778 ( 62.1%)   0.0779 ( 62.4%)
>  Debug Info Emission
> >>  0.0507 ( 71.1%)   0.0261 ( 50.7%)   0.0769 ( 62.5%)   0.0768 ( 62.5%)
>  Debug Info Emission
> >>
> >> With Patch
> >>  ---User Time---   --System Time--   --User+System--   ---Wall Time---
>  --- Name ---
> >>  0.0380 ( 64.2%)   0.0259 ( 50.4%)   0.0639 ( 57.8%)   0.0635 ( 57.7%)
>  Debug Info Emission
> >>  0.0377 ( 63.7%)   0.0266 ( 50.5%)   0.0643 ( 57.5%)   0.0645 ( 57.8%)
>  Debug Info Emission
> >>  0.0383 ( 64.2%)   0.0264 ( 50.4%)   0.0647 ( 57.7%)   0.0651 ( 58.1%)
>  Debug Info Emission
> >>  0.0383 ( 63.7%)   0.0272 ( 50.8%)   0.0656 ( 57.6%)   0.0664 ( 58.0%)
>  Debug Info Emission
> >>  0.0376 ( 64.3%)   0.0261 ( 50.3%)   0.0637 ( 57.7%)   0.0638 ( 57.7%)
>  Debug Info Emission
> >>
> >> Before r210492
> >>  ---User Time---   --System Time--   --User+System--   ---Wall Time---
>  --- Name ---
> >>  0.0341 ( 62.6%)   0.0261 ( 50.6%)   0.0602 ( 56.8%)   0.0600 ( 56.7%)
>  Debug Info Emission
> >>  0.0350 ( 62.7%)   0.0262 ( 50.4%)   0.0612 ( 56.7%)   0.0612 ( 56.7%)
>  Debug Info Emission
> >>  0.0347 ( 62.7%)   0.0259 ( 50.4%)   0.0607 ( 56.8%)   0.0610 ( 56.9%)
>  Debug Info Emission
> >>  0.0361 ( 62.3%)   0.0267 ( 50.6%)   0.0628 ( 56.7%)   0.0630 ( 56.7%)
>  Debug Info Emission
> >>  0.0354 ( 62.5%)   0.0271 ( 50.4%)   0.0624 ( 56.6%)   0.0625 ( 56.6%)
>  Debug Info Emission
> >>
> >> The patch passes check-all. It doesn’t contain any test case as it’s
> just a functionally equivalent refactoring of the same algorithm.
> >>
> >> Ok to check-in ?
> >>
> >> Thanks,
> >> Fred
> >>
> >>
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at cs.uiuc.edu
> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140805/eda87d46/attachment.html>