[PATCH] D90285: [CallGraphUpdater][Attributor][FIX] Allow standalone function replacementNOTE: This is not a working patch. We need to revisit the ways we update the new PM callgraph, potentially the things we want to do are just not possible.

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert created this revision.
jdoerfert added reviewers: aeubanks, asbirlea.
Herald added subscribers: okura, kuter, uenoku, bollu, hiraditya.
Herald added a reviewer: uenoku.
Herald added a reviewer: homerdin.
Herald added a project: LLVM.
jdoerfert requested review of this revision.
Herald added a reviewer: sstefan1.
Herald added a reviewer: baziotis.
Herald added a subscriber: bbn.

We did mix replacing a function and deleting the old one but that should
be two different operations so we can reuse them individually.

This is supposed to fix the creation of deep-wrappers which currently
delete the original function but it actually will cause a crash in the
way we update the new PM call graph.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90285

Files:
  llvm/lib/Transforms/IPO/Attributor.cpp
  llvm/lib/Transforms/Utils/CallGraphUpdater.cpp
  llvm/unittests/Analysis/CGSCCPassManagerTest.cpp


Index: llvm/unittests/Analysis/CGSCCPassManagerTest.cpp
===================================================================
--- llvm/unittests/Analysis/CGSCCPassManagerTest.cpp
+++ llvm/unittests/Analysis/CGSCCPassManagerTest.cpp
@@ -1640,7 +1640,7 @@
         CallGraphUpdater CGU;
         CGU.initialize(CG, C, AM, UR);
         ASSERT_NO_FATAL_FAILURE(CGU.replaceFunctionWith(*FnF, *FnewF));
-        ASSERT_TRUE(FnF->isDeclaration());
+        ASSERT_FALSE(FnF->isDeclaration());
         ASSERT_EQ(FnF->getNumUses(), 0U);
       }));
 
Index: llvm/lib/Transforms/Utils/CallGraphUpdater.cpp
===================================================================
--- llvm/lib/Transforms/Utils/CallGraphUpdater.cpp
+++ llvm/lib/Transforms/Utils/CallGraphUpdater.cpp
@@ -134,7 +134,6 @@
     LazyCallGraph::Node &OldLCGN = LCG->get(OldFn);
     SCC->getOuterRefSCC().replaceNodeFunction(OldLCGN, NewFn);
   }
-  removeFunction(OldFn);
 }
 
 bool CallGraphUpdater::replaceCallSite(CallBase &OldCS, CallBase &NewCS) {
Index: llvm/lib/Transforms/IPO/Attributor.cpp
===================================================================
--- llvm/lib/Transforms/IPO/Attributor.cpp
+++ llvm/lib/Transforms/IPO/Attributor.cpp
@@ -1829,6 +1829,7 @@
 
     // Replace the function in the call graph (if any).
     CGUpdater.replaceFunctionWith(*OldFn, *NewFn);
+    CGUpdater.removeFunction(*OldFn);
 
     // If the old function was modified and needed to be reanalyzed, the new one
     // does now.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D90285.301179.patch
Type: text/x-patch
Size: 1489 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201028/4ffd9712/attachment.bin>