[PATCH][Review Requested][Compilation Time] Small Vector for Machine Operand

Thu Feb 14 14:10:10 PST 2013

Hi BW,

Thanks for the review and the examples. I'm still learning LLVM so I found them quite helpful.

ArrayRef<> - This uses a const pointer to the array. Unfortunately much of LLVMs ScheduleDAG, and its various priority queues were not implemented with this in mind. I looked into changing the constness of the SUnits and SNodes, however, it looks like a lot of work. There is the added problem that when a SmallVector is modified the array may be destroyed. This would leave the ArrayRefs hanging. I found this to occur when I tried to use an ArrayRef for ScheduleDAGTopologicalSort.

SmallVectorImpl - I changed many of the SmallVectors to SmallVectorImpls.

Performance - The performance numbers in the previous email were a little confusing, but they did show an improvement. I ran them again on the updated patch and made the results easier to read (see below). They are measuring the time LLC takes to turn some bc files into machine code. The bc files were sampled from several benchmarks including SPEC.

'.reserve()' - Thanks yes we have several other patches some of which adjust reserve sizes, but std::vector is slower than SmallVector so we have mostly been changing types.

Tyler

Benchmark		% improvement
401.bzip2		1.54%
429.mcf		1.91%
433.milc		0.96%
444.namd		1.35%
450.soplex		2.32%
456.hmmer		1.85%
458.sjeng		1.75%
464.h264ref		1.28%
470.lbm		0.07%
471.omnetpp		2.31%
bitmnp01		2.31%
cjpegv2data6		1.41%
idctrn01		0.92%
libquake_portable	1.83%
linpack			1.03%
matrix01		1.32%
nbench			1.31%
tblook01		0.51%
ttsprk01		0.88%

geomean		1.42%

-----Original Message-----
From: Bill Wendling [mailto:wendling at apple.com] 
Sent: Monday, February 04, 2013 4:11 PM
To: Nowicki, Tyler
Cc: Jakub Staszak; llvm-commits at cs.uiuc.edu; Jakub Staszak
Subject: Re: [PATCH][Review Requested][Compilation Time] Small Vector for Machine Operand

Hi Tyler,

When passing around vectors that you're not going to modify, please use an 'ArrayRef<>' instead. The numbers for the SPEC tests seemed to increase. Is that a good thing? I.e., what are those numbers measuring? Have you considered using a '.reserve()' call on the std::vectors to limit the amount of reallocating and copying it does?

-bw

P.S. If you're going to pass a SmallVector around that you plan on modifying, you should use this:

	void foo(SmallVectorImpl<SomeType> &fnord) {}

You can use the same when looping:

	for (SmallVectorImpl<SomeType>::iterator I = ...)

That way you don't have to worry about the SmallVector count.

On Feb 4, 2013, at 12:22 PM, "Nowicki, Tyler" <tyler.nowicki at intel.com> wrote:

> Could someone please review this patch.
>  
> Thanks,
>  
> Tyler
>  
> From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-bounces at cs.uiuc.edu] On Behalf Of Nowicki, Tyler
> Sent: Thursday, January 31, 2013 5:10 PM
> To: Jakub Staszak
> Cc: llvm-commits at cs.uiuc.edu; Jakub Staszak
> Subject: RE: [PATCH][Review Requested][Compilation Time] Small Vector for Machine Operand
>  
> Oops, thanks.
>  
> Here is the patch...
>  
> Tyler
>  
> From: Jakub Staszak [mailto:kubastaszak at gmail.com] 
> Sent: Thursday, January 31, 2013 5:01 PM
> To: Nowicki, Tyler
> Cc: Jakub Staszak; llvm-commits at cs.uiuc.edu
> Subject: Re: [PATCH][Review Requested][Compilation Time] Small Vector for Machine Operand
>  
> Looks like you forgot to attach the patch :)
>  
> - Kuba
>  
> On Jan 31, 2013, at 7:58 PM, "Nowicki, Tyler" <tyler.nowicki at intel.com> wrote:
>  
> 
> This patch aims to improve compile time performance by changing when the SchedulerDAG object is created and destroyed, and replacing std::vector with SmallVector. Currently, ScheduleDAG is deleted after each BB which causes frequent SUnits vector clearance, and std::vector causes frequent element copies the vector size grows. Please see attached performance data.
>  
> This patch is part of a series of compile time improvements. Although these were originally produced by our colleague Wan Xiaofei, our team consisting of preston.gurd at intel.com; sriram.murali at intel.com and myself, are assuming all responsibility for this work.
>  
> PLEASE REVIEW. Thanks!
>  
> Tyler Nowicki
> Intel
>  
> Benchmark
> Trunk
> SmallVec.SU
> 401.bzip2
> 74.21
> 72.48
> 403.gcc
> 73.88
> 71.65
> 429.mcf
> 72.80
> 71.33
> 433.milc
> 78.78
> 78.14
> 444.namd
> 94.73
> 93.45
> 445.gobmk
> 36.28
> 35.51
> 450.soplex
> 71.41
> 69.64
> 456.hmmer
> 86.80
> 84.86
> 458.sjeng
> 96.38
> 94.27
> 464.h264ref
> 87.61
> 86.36
> 470.lbm
> 68.95
> 68.09
> 471.omnetpp
> 89.07
> 86.57
> bitmnp01
> 84.06
> 81.60
> cjpegv2data6
> 59.70
> 58.84
> idctrn01
> 40.18
> 39.79
> libquake2
> 48.48
> 47.54
> libquake_portable
> 63.54
> 62.38
> libxcsoar
> 47.44
> 46.81
> linpack
> 142.14
> 141.63
> matrix01
> 24.75
> 24.40
> nbench
> 108.04
> 107.04
> tblook01
> 44.03
> 43.71
> ttsprk01
> 39.23
> 38.54
> Geomean
> 65.85
> 64.72
> 401.bzip2
> 100.00
> 102.39
> 403.gcc
> 100.00
> 103.11
> 429.mcf
> 100.00
> 102.06
> 433.milc
> 100.00
> 100.82
> 444.namd
> 100.00
> 101.37
> 445.gobmk
> 100.00
> 102.17
> 450.soplex
> 100.00
> 102.54
> 456.hmmer
> 100.00
> 102.29
> 458.sjeng
> 100.00
> 102.24
> 464.h264ref
> 100.00
> 101.45
> 470.lbm
> 100.00
> 101.26
> 471.omnetpp
> 100.00
> 102.89
> bitmnp01
> 100.00
> 103.01
> cjpegv2data6
> 100.00
> 101.46
> idctrn01
> 100.00
> 100.98
> libquake2
> 100.00
> 101.98
> libquake_portable
> 100.00
> 101.86
> libxcsoar
> 100.00
> 101.35
> linpack
> 100.00
> 100.36
> matrix01
> 100.00
> 101.43
> nbench
> 100.00
> 100.93
> tblook01
> 100.00
> 100.73
> ttsprk01
> 100.00
> 101.79
> Geomean
> 100.00
> 101.76
>  
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>  
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
A non-text attachment was scrubbed...
Name: SmallVector2-svn.patch
Type: application/octet-stream
Size: 14797 bytes
Desc: SmallVector2-svn.patch
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130214/9d6869d4/attachment.obj>