[PATCH] [LAA] Fix estimation of number of memchecks

Michael Zolotukhin mzolotukhin at apple.com
Thu Jun 4 11:53:34 PDT 2015


The patch LGTM.

Minor nitpicks:


================
Comment at: lib/Analysis/LoopAccessAnalysis.cpp:361
@@ -366,1 +360,3 @@
 
+  NeedRTCheck = !CanDoRT || RtCheck.needsAnyChecking(nullptr);
+
----------------
This looks a bit confusing (probably, because of the variable names) - why do we need a RT check if we can't add them? A comment would be helpful here.

================
Comment at: lib/Analysis/LoopAccessAnalysis.cpp:1135-1136
@@ -1143,4 +1134,4 @@
   // Check that we found the bounds for the pointer.
-  if (CanDoRT)
+  if (NeedRTCheck && CanDoRT)
     DEBUG(dbgs() << "LAA: We can perform a memory runtime check if needed.\n");
   else if (NeedRTCheck) {
----------------
The debug message no longer needs "if needed" part.

And by the way, why do we need this change? To me the original and changed versions seem equivalent except that we won't print any message if `NeedRTCheck==false` and `CanDoRT==true` .

http://reviews.llvm.org/D10217

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list