[Review]: Sink multiple instructions per iteration in InstCombine

[Aditya Nandakumar]

Thanks Chandler, Philip
I will submit this after fixing the {}.

Thanks
Aditya
> On Jul 10, 2014, at 4:34 PM, Chandler Carruth <chandlerc at google.com> wrote:
> 
> 
> On Thu, Jul 10, 2014 at 4:26 PM, Philip Reames <listmail at philipreames.com> wrote:
> On 07/10/2014 04:12 PM, Chandler Carruth wrote:
>> Sorry it's taken me a while to get back to this.
>> 
>> I looked at the patch, and I'm still feeling like there has to be a more direct way to use the instcombine worklist to address this problem rather than adding another worklist on top. Having a feeling isn't a very good code review though. =D So I grabbed your patch and tried to build a benchmark just as you suggested to see what the performance difference was and whether there was another way to achieve it.
> Chandler - Given what you said here, it really sounds like you're not looking at an up to date version of the patch.  There is now no new worklist.  
> 
> Are you looking at Aditya's email from 6/30 10:45 am?
> 
> Whoops! Thanks for pointing that out, I had definitely missed that email. Really sorry. *This* patch makes perfect sense to me, LGTM with a tiny nit:
> 
> +            for (Use &U : I->operands()) {
> +              if (Instruction *OpI = dyn_cast<Instruction>(U.get())) {
> +                Worklist.Add(OpI);
> +              }
> +            }
> 
> I'd drop the {}s from both the for and if here...

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140711/93a4eb03/attachment.html>