[PATCH] Improve performance of calculateDbgValueHistory

Tue Aug 5 23:14:12 PDT 2014

Here’s the patch without the stylistic modifications. I kept the comment correction as I’m touching the corresponding function in the patch. I’ll submit the cleanup patch separately.

OK to commit?

Fred

On 05 Aug 2014, at 21:35, David Blaikie <dblaikie at gmail.com> wrote:

> 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/20140806/b0a1b4ee/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: rdar.17884104.patch
Type: application/octet-stream
Size: 4183 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140806/b0a1b4ee/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140806/b0a1b4ee/attachment-0001.html>