[PATCH][Review Requested][Compilation Time] Using hashing in getVTList()

[Bill Wendling]

The patch LGTM. I'd be interested to know what speedups you're seeing.

A few stylistic comments:

* You can use 'SmallVectorImpl<..>::iterator' instead of 'SmallVector<..., 4>::iterator'. This saves having to rewrite all of the for loops if you change the initial size of the SmallVector later. E.g.:

	for (SmallVectorImpl<SDVTList>::reverse_iterator
		I = VTListSet[Slot].rbegin(),
		E = VTListSet[Slot].rend(); I != E; ++I) { ... }

* LLVM typically uses 'unsigned' whenever possible. So the 'VTListSize' should probably be this:

	const static unsigned VTListSize = 64;

I didn't see anywhere in the style guide saying that such things should be an 'enum', so I won't force you to make it one. :-)

-bw

On Feb 27, 2013, at 11:18 AM, "Nowicki, Tyler" <tyler.nowicki at intel.com> wrote:

> Sure, here it is.
> 
> Tyler
> 
> -----Original Message-----
> From: Bill Wendling [mailto:wendling at apple.com] 
> Sent: Wednesday, February 27, 2013 2:07 PM
> To: Nowicki, Tyler
> Cc: Chris Lattner; llvm-commits at cs.uiuc.edu
> Subject: Re: [PATCH][Review Requested][Compilation Time] Using hashing in getVTList()
> 
> Hi Tyler,
> 
> I don't see the patch here. Could you repost please?
> 
> -bw
> 
> On Feb 26, 2013, at 7:59 AM, "Nowicki, Tyler" <tyler.nowicki at intel.com> wrote:
> 
>> Could I get a review/comment on 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, February 14, 2013 6:44 PM
>> To: Chris Lattner
>> Cc: llvm-commits at cs.uiuc.edu
>> Subject: RE: [PATCH][Review Requested][Compilation Time] Using hashing in getVTList()
>> 
>> Hi Chris,
>> 
>> Thanks for the review. We have committed the const change.
>> 
>> FoldingSet - We have experimented with using FoldingSet on another compile-time optimization however we found that it was slower. Sriram Murali posted a message discussing our findings on this. See `RE: [PATCH][Review Requested][Compilation Time] Calculate hash   value and equality comparison within SCEV node itself'
>> 
>> Hashing - The patch uses bucket hashing. It is simple and fast. Can you suggest something better?
>> 
>> Tyler
>> 
>> From: Chris Lattner [mailto:clattner at apple.com] 
>> Sent: Wednesday, February 13, 2013 12:05 AM
>> To: Nowicki, Tyler
>> Cc: llvm-commits at cs.uiuc.edu
>> Subject: Re: [PATCH][Review Requested][Compilation Time] Using hashing in getVTList()
>> 
>> 
>> On Jan 31, 2013, at 2:54 PM, "Nowicki, Tyler" <tyler.nowicki at intel.com> wrote:
>> 
>> 
>> Hi,
>> 
>> This patch aims to improve compile time performance by replacing a sequential search over an std::vector in the function getVTList() with a look up into a SmallVector where the index is calculated by hashing the input VTs and a sequential search through a smaller VT bucket.
>> 
>> 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.
>> 
>> Hi Tyler,
>> 
>> Marking stuff const is obvious, please commit that.  For the actual algorithm here, have you considered using a FoldingSet or a hash table of some sort to unique the lists instead of switching the order of the linear scan?
>> 
>> -Chris
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 
> <Hashing-svn.patch>