[Review]: Sink multiple instructions per iteration in InstCombine

[Chandler Carruth]

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/20140710/f6b32de0/attachment.html>