[PATCH] D30104: Refactor instruction simplification code in visitors. NFC.

[Easwaran Raman via Phabricator via llvm-commits]

eraman marked 5 inline comments as done.
eraman added a comment.

Thanks for the comments. Will update the revised patch in a bit.



================
Comment at: lib/Analysis/InlineCost.cpp:465
+  int N = 0;
+  for (Value *Op : I.operands()) {
+    Constant *COp = dyn_cast<Constant>(Op);
----------------
chandlerc wrote:
> What if there are more than two operands? Maybe you want to require one of th eexpression subclasses here? Or maybe just make COps be a small vector so this is generically correct? It should't be any less efficient, and I find the N++ bit below somewhat easy to miss anyways.
Changed it to use SmallVector


================
Comment at: lib/Analysis/InlineCost.cpp:507-509
+  auto Evaluate = [&](ArrayRef<Constant *> COps) -> Constant * {
+    return ConstantExpr::getPtrToInt(COps[0], I.getType());
+  };
----------------
chandlerc wrote:
> For all of these where you only use the lambda once, put the lambda in the arguments?
> 
> Also for all of these, can you let the return type be deduced? I always prefer that when it is unambiguous. This might be made easier by the routine accepting a generic callable.
Made the suggested change in all but one place where the body of the law is multiple lines and to me makes the surrounding if more readable.


https://reviews.llvm.org/D30104