[llvm] r308417 - [PM/LCG] Follow-up fix to r308088 to handle deletion of library

Chandler Carruth via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 18 21:12:25 PDT 2017


Author: chandlerc
Date: Tue Jul 18 21:12:25 2017
New Revision: 308417

URL: http://llvm.org/viewvc/llvm-project?rev=308417&view=rev
Log:
[PM/LCG] Follow-up fix to r308088 to handle deletion of library
functions.

In the prior commit, we provide ordering to the LCG between functions
and library function definitions that they might begin to call through
transformations. But we still would delete these library functions from
the call graph if they became dead during inlining.

While this immediately crashed, it also exposed a loss of information.
We shouldn't remove definitions of library functions that can still
usefully participate in the LCG-powered CGSCC optimization process. If
new call edges are formed, we want to have definitions to be called.

We can still remove these functions if truly dead using global-dce, etc,
but removing them during the CGSCC walk is premature.

This fixes a crash in the new PM when optimizing some unusual libraries
that end up with "internal" lib functions such as the code in the "R"
language's libraries.

Modified:
    llvm/trunk/include/llvm/Analysis/LazyCallGraph.h
    llvm/trunk/lib/Analysis/LazyCallGraph.cpp
    llvm/trunk/lib/Transforms/IPO/Inliner.cpp
    llvm/trunk/test/Other/cgscc-libcall-update.ll

Modified: llvm/trunk/include/llvm/Analysis/LazyCallGraph.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/LazyCallGraph.h?rev=308417&r1=308416&r2=308417&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Analysis/LazyCallGraph.h (original)
+++ llvm/trunk/include/llvm/Analysis/LazyCallGraph.h Tue Jul 18 21:12:25 2017
@@ -971,7 +971,17 @@ public:
   ///
   /// These functions, because they are known to LLVM, can have calls
   /// introduced out of thin air from arbitrary IR.
-  ArrayRef<Function *> getLibFunctions() const { return LibFunctions; }
+  ArrayRef<Function *> getLibFunctions() const {
+    return LibFunctions.getArrayRef();
+  }
+
+  /// Test whether a function is a known and defined library function tracked by
+  /// the call graph.
+  ///
+  /// Because these functions are known to LLVM they are specially modeled in
+  /// the call graph and even when all IR-level references have been removed
+  /// remain active and reachable.
+  bool isLibFunction(Function &F) const { return LibFunctions.count(&F); }
 
   ///@{
   /// \name Pre-SCC Mutation API
@@ -1110,7 +1120,7 @@ private:
   /// Defined functions that are also known library functions which the
   /// optimizer can reason about and therefore might introduce calls to out of
   /// thin air.
-  SmallVector<Function *, 4> LibFunctions;
+  SmallSetVector<Function *, 4> LibFunctions;
 
   /// Helper to insert a new function, with an already looked-up entry in
   /// the NodeMap.

Modified: llvm/trunk/lib/Analysis/LazyCallGraph.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/LazyCallGraph.cpp?rev=308417&r1=308416&r2=308417&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/LazyCallGraph.cpp (original)
+++ llvm/trunk/lib/Analysis/LazyCallGraph.cpp Tue Jul 18 21:12:25 2017
@@ -144,7 +144,7 @@ LazyCallGraph::LazyCallGraph(Module &M,
     // synthesize reference edges to it to model the fact that LLVM can turn
     // arbitrary code into a library function call.
     if (isKnownLibFunction(F, TLI))
-      LibFunctions.push_back(&F);
+      LibFunctions.insert(&F);
 
     if (F.hasLocalLinkage())
       continue;
@@ -1608,6 +1608,11 @@ void LazyCallGraph::removeDeadFunction(F
   assert(F.use_empty() &&
          "This routine should only be called on trivially dead functions!");
 
+  // We shouldn't remove library functions as they are never really dead while
+  // the call graph is in use -- every function definition refers to them.
+  assert(!isLibFunction(F) &&
+         "Must not remove lib functions from the call graph!");
+
   auto NI = NodeMap.find(&F);
   if (NI == NodeMap.end())
     // Not in the graph at all!

Modified: llvm/trunk/lib/Transforms/IPO/Inliner.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/Inliner.cpp?rev=308417&r1=308416&r2=308417&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/IPO/Inliner.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/Inliner.cpp Tue Jul 18 21:12:25 2017
@@ -909,7 +909,7 @@ PreservedAnalyses InlinerPass::run(LazyC
         // To check this we also need to nuke any dead constant uses (perhaps
         // made dead by this operation on other functions).
         Callee.removeDeadConstantUsers();
-        if (Callee.use_empty()) {
+        if (Callee.use_empty() && !CG.isLibFunction(Callee)) {
           Calls.erase(
               std::remove_if(Calls.begin() + i + 1, Calls.end(),
                              [&Callee](const std::pair<CallSite, int> &Call) {

Modified: llvm/trunk/test/Other/cgscc-libcall-update.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Other/cgscc-libcall-update.ll?rev=308417&r1=308416&r2=308417&view=diff
==============================================================================
--- llvm/trunk/test/Other/cgscc-libcall-update.ll (original)
+++ llvm/trunk/test/Other/cgscc-libcall-update.ll Tue Jul 18 21:12:25 2017
@@ -1,10 +1,12 @@
 ; Make sure that the CGSCC pass manager can handle when instcombine simplifies
 ; one libcall into an unrelated libcall and update the call graph accordingly.
 ;
-; RUN: opt -passes='cgscc(function(instcombine))' -S < %s | FileCheck %s
+; Also check that it can handle inlining *removing* a libcall entirely.
+;
+; RUN: opt -passes='cgscc(inline,function(instcombine))' -S < %s | FileCheck %s
 
 define i8* @wibble(i8* %arg1, i8* %arg2) {
-; CHECK-LABLE: define @wibble(
+; CHECK-LABEL: define i8* @wibble(
 bb:
   %tmp = alloca [1024 x i8], align 16
   %tmp2 = getelementptr inbounds [1024 x i8], [1024 x i8]* %tmp, i64 0, i64 0
@@ -20,7 +22,7 @@ bb:
 ; CHECK:         ret
 }
 
-define i8* @strncpy(i8* %arg1, i8* %arg2, i64 %size) {
+define i8* @strncpy(i8* %arg1, i8* %arg2, i64 %size) noinline {
 bb:
   %result = call i8* @my_special_strncpy(i8* %arg1, i8* %arg2, i64 %size)
   ret i8* %result
@@ -33,3 +35,27 @@ declare i64 @llvm.objectsize.i64.p0i8(i8
 declare i8* @__strncpy_chk(i8*, i8*, i64, i64)
 
 declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture writeonly, i8* nocapture readonly, i64, i32, i1)
+
+; Check that even when we completely remove a libcall we don't get the call
+; graph wrong once we handle libcalls in the call graph specially to address
+; the above case.
+define i32 @hoge(i32* %arg1) {
+; CHECK-LABEL: define i32 @hoge(
+bb:
+  %tmp41 = load i32*, i32** null
+  %tmp6 = load i32, i32* %arg1
+  %tmp7 = call i32 @ntohl(i32 %tmp6)
+; CHECK-NOT: call i32 @ntohl
+  ret i32 %tmp7
+; CHECK: ret i32
+}
+
+; Even though this function is not used, it should be retained as it may be
+; used when doing further libcall transformations.
+define internal i32 @ntohl(i32 %x) {
+; CHECK-LABEL: define internal i32 @ntohl(
+entry:
+  %and2 = lshr i32 %x, 8
+  %shr = and i32 %and2, 65280
+  ret i32 %shr
+}




More information about the llvm-commits mailing list