[PATCH] D12170: Const propagatin after hitting assume bugfix

Vedant Kumar via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 19 16:05:11 PDT 2015


vsk added a subscriber: vsk.
vsk added a comment.

A few inline comments --


================
Comment at: include/llvm/Transforms/Utils/Local.h:291
@@ -290,3 +290,3 @@
 /// \brief Replace each use of 'From' with 'To' if that use is dominated by 
 /// the given edge.  Returns the number of replacements made.
 unsigned replaceDominatedUsesWith(Value *From, Value *To, DominatorTree &DT,
----------------
"the given edge" is now a bit vague. Could you update the docstring?

================
Comment at: lib/IR/Dominators.cpp:151
@@ -151,1 +150,3 @@
+         "edges,"
+         " the callers can normally handle them more efficiently.");
 
----------------
IMO this is fine as a comment.

If you think it's a good idea to lift this explanation into the assert, I suggest shortening it a bit.

================
Comment at: lib/IR/Dominators.cpp:202
@@ -201,1 +201,3 @@
+         "edges,"
+         " the callers can normally handle them more efficiently.");
 
----------------
Same comment as above.


http://reviews.llvm.org/D12170





More information about the llvm-commits mailing list