submit [PATCH] SimplifyCFG for code review

Sean Silva silvas at purdue.edu
Tue Jun 18 14:58:04 PDT 2013


Those regression tests seem really large. Is there any way you could reduce
them?

+/// If \param [in] BB has more than one predecessor that is a conditional
+/// branch, attempt to use parallel and/or for the branch condtion.
\returns
+/// true on success.

Spelling: s/condtion/condition/.
Also, typically doxygen commands are put on a line of their own instead of
inline with prose (I'm not sure that doxygen even accepts them inline like
you have them here).

+/// Before:
+///   ......
+///   %cmp10 = fcmp une float %tmp1, %tmp2
+///   br i1 %cmp1, label %if.then, label %lor.rhs

You want \code + \endcode <
http://www.stack.nl/~dimitri/doxygen/manual/commands.html#cmdcode>
enclosing this code display, so that doxygen will format it correctly.

+      if (UCond || !PP || (Preds.count(PP) == 0) ||
+`        (PTI->getNumSuccessors() != 1) ||
+`        Pred->hasAddressTaken())
+        return false;

Please don't use hard tabs. Your patch also introduces a bunch of trailing
whitespace. Also, there are a number of formatting issues; I recommend
using clang-format on the regions of code that you are adding to ensure
they comply with our preferred code formatting.

-- Sean Silva
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130618/04923c12/attachment.html>


More information about the llvm-commits mailing list