[llvm] r253192 - [GlobalOpt] Address post-commit review comments on r253168

James Molloy via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 16 02:16:23 PST 2015


Author: jamesm
Date: Mon Nov 16 04:16:22 2015
New Revision: 253192

URL: http://llvm.org/viewvc/llvm-project?rev=253192&view=rev
Log:
[GlobalOpt] Address post-commit review comments on r253168

Address Duncan Exon Smith's comments on D14148, which was added after the patch had been LGTM'd and committed:
  * clang-format one area where whitespace diffs occurred.
  * Add a threshold to limit the store/load dominance checks as they are quadratic.

Modified:
    llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp

Modified: llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp?rev=253192&r1=253191&r2=253192&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp Mon Nov 16 04:16:22 2015
@@ -87,9 +87,10 @@ namespace {
     bool ProcessInternalGlobal(GlobalVariable *GV,Module::global_iterator &GVI,
                                const GlobalStatus &GS);
     bool OptimizeEmptyGlobalCXXDtors(Function *CXAAtExitFn);
-    
-    bool isPointerValueDeadOnEntryToFunction(const Function *F, GlobalValue *GV);
-    
+
+    bool isPointerValueDeadOnEntryToFunction(const Function *F,
+                                             GlobalValue *GV);
+
     TargetLibraryInfo *TLI;
     SmallSet<const Comdat *, 8> NotDiscardableComdats;
   };
@@ -1770,6 +1771,19 @@ bool GlobalOpt::isPointerValueDeadOnEntr
   auto &DT = getAnalysis<DominatorTreeWrapperPass>(*const_cast<Function *>(F))
                  .getDomTree();
 
+  // The below check is quadratic. Check we're not going to do too many tests.
+  // FIXME: Even though this will always have worst-case quadratic time, we
+  // could put effort into minimizing the average time by putting stores that
+  // have been shown to dominate at least one load at the beginning of the
+  // Stores array, making subsequent dominance checks more likely to succeed
+  // early.
+  //
+  // The threshold here is fairly large because global->local demotion is a
+  // very powerful optimization should it fire.
+  const unsigned Threshold = 100;
+  if (Loads.size() * Stores.size() > Threshold)
+    return false;
+
   for (auto *L : Loads) {
     auto *LTy = L->getType();
     if (!std::any_of(Stores.begin(), Stores.end(), [&](StoreInst *S) {




More information about the llvm-commits mailing list