[PATCH] Quick look-up for block in loop

[Wan, Xiaofei]

Hi

Any further comments on the updated patch? Thanks.
http://llvm-reviews.chandlerc.com/D2014

Thanks
Wan Xiaofei

-----Original Message-----
From: Hal Finkel [mailto:hfinkel at anl.gov] 
Sent: Thursday, October 24, 2013 10:14 PM
To: Wan, Xiaofei
Cc: llvm-commits at cs.uiuc.edu; atrick at apple.com; reviews+D2014+public+61410cad53427477 at llvm-reviews.chandlerc.com; Nadav Rotem; Arnold Schwaighofer
Subject: Re: [PATCH] Quick look-up for block in loop

----- Original Message -----
> How do you think only exposing two write interfaces for block vector 
> (remove block & add block, it is enough); then the DenseSet could be 
> cached and maintained easily.

I suspect that is sufficient. It looks like the only user of getBlocksVector is the Loop Vectorizer. Nadav, Arnold, thoughts?

 -Hal

> 
> It could introduce some extra performance benefit if the DenseSet 
> needn't be constructed each time.
> Take 458.sjeng as example, the time in LoopInfo is about ~4% without 
> this patch, now it is ~1% with this patch, so it may introduce ~1% 
> extra performance gain if DenseSet is cached.
> 
> Thanks
> Wan Xiaofei
> 
> -----Original Message-----
> From: Hal Finkel [mailto:hfinkel at anl.gov]
> Sent: Thursday, October 24, 2013 9:55 PM
> To: reviews+D2014+public+61410cad53427477 at llvm-reviews.chandlerc.com
> Cc: llvm-commits at cs.uiuc.edu; atrick at apple.com; Wan, Xiaofei
> Subject: Re: [PATCH] Quick look-up for block in loop
> 
> ----- Original Message -----
> > 
> >   Hal:
> > 
> >   Thanks for your comments, I don't think the DenseSet could be
> >   cached since the blocks in loop may be changed.
> 
> 
> I think that, normally, blocks can't be added or removed from a loop 
> without either invalidating the LoopInfo, or updating it (calling 
> LoopInfo::removeBlock, changeLoopFor, etc.). I think that to get a 
> mutable copy of the blocks array directly, getBlocksVector needs to be 
> called (and it could invalidate the cache, and prevent further caching 
> after it is called).
> 
> > 
> >   Finally, when you post patches to llvm-reviews, please generate
> >   them with -U999999 so that we can see the full context in the web
> >   interface.
> >   ~~~~What diff tool do you usually use, it seems svn diff has no
> >   option -U999999, thanks.
> 
> http://llvm.org/docs/Phabricator.html says:
> 
> svn diff --diff-cmd=diff -x -U999999
> 
> Personally, I use git-svn, so it is a bit easier.
> 
> Thanks,
> Hal
> 
> > 
> > 
> >   Thanks
> >   Wan Xiaofei
> > 
> > http://llvm-reviews.chandlerc.com/D2014
> > 
> 
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
> 

--
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory