Remove gc.root's findCustomSafePoints mechanism

Searching all of the existing gc.root implementations I'm aware of (all three of them), there was exactly one use of this mechanism, and that was to implement a performance improvement that should have been applied to the default lowering.

Having this function is requiring a dependency on a CodeGen class (MachineFunction), in a class which is otherwise completely independent of CodeGen. I could solve this differently, but given that I see absolutely no value in preserving this mechanism, I going to just get rid of it.

Note: Tis is the first time I'm intentionally breaking previously supported gc.root functionality. Given 3.6 has branched, I believe this is a good time to do this.

Differential Revision: http://reviews.llvm.org/D7004

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@226305 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/CodeGen/GCStrategy.h b/include/llvm/CodeGen/GCStrategy.h
index 0b0c3124c537c24179733c8dee6d2503ec66b195..bdd14463e003ba55a2ed51668f9afe78f0a86a46 100644 (file)
--- a/include/llvm/CodeGen/GCStrategy.h
+++ b/include/llvm/CodeGen/GCStrategy.h
@@ -76,8 +76,6 @@ namespace llvm {
     bool CustomReadBarriers;   ///< Default is to insert loads.
     bool CustomWriteBarriers;  ///< Default is to insert stores.
     bool CustomRoots;          ///< Default is to pass through to backend.
     bool CustomReadBarriers;   ///< Default is to insert loads.
     bool CustomWriteBarriers;  ///< Default is to insert stores.
     bool CustomRoots;          ///< Default is to pass through to backend.

-    bool CustomSafePoints;     ///< Default is to use NeededSafePoints
-                               ///< to find safe points.

     bool InitRoots;            ///< If set, roots are nulled during lowering.
     bool UsesMetadata;         ///< If set, backend must emit metadata tables.
     
     bool InitRoots;            ///< If set, roots are nulled during lowering.
     bool UsesMetadata;         ///< If set, backend must emit metadata tables.
     


@@ -124,7 +122,7 @@ namespace llvm {
     /// True if safe points of any kind are required. By default, none are
     /// recorded. 
     bool needsSafePoints() const {
     /// True if safe points of any kind are required. By default, none are
     /// recorded. 
     bool needsSafePoints() const {

-      return CustomSafePoints || NeededSafePoints != 0;
+      return NeededSafePoints != 0;

     }
     
     /// True if the given kind of safe point is required. By default, none are
     }
     
     /// True if the given kind of safe point is required. By default, none are


@@ -137,10 +135,6 @@ namespace llvm {
     /// stack map. If true, then performCustomLowering must delete them.
     bool customRoots() const { return CustomRoots; }
 
     /// stack map. If true, then performCustomLowering must delete them.
     bool customRoots() const { return CustomRoots; }
 

-    /// By default, the GC analysis will find safe points according to
-    /// NeededSafePoints. If true, then findCustomSafePoints must create them.
-    bool customSafePoints() const { return CustomSafePoints; }
-    

     /// If set, gcroot intrinsics should initialize their allocas to null
     /// before the first use. This is necessary for most GCs and is enabled by
     /// default. 
     /// If set, gcroot intrinsics should initialize their allocas to null
     /// before the first use. This is necessary for most GCs and is enabled by
     /// default. 


@@ -170,14 +164,6 @@ namespace llvm {
       llvm_unreachable("GCStrategy subclass specified a configuration which"
                        "requires a custom lowering without providing one");
     }
       llvm_unreachable("GCStrategy subclass specified a configuration which"
                        "requires a custom lowering without providing one");
     }

-    ///@}
-    /// Called if customSafepoints returns true, used only by gc.root
-    /// implementations. 
-    virtual bool findCustomSafePoints(GCFunctionInfo& FI, MachineFunction& MF) {
-      llvm_unreachable("GCStrategy subclass specified a configuration which"
-                       "requests custom safepoint identification without"
-                       "providing an implementation for such");
-    }

   };
 
   /// Subclasses of GCStrategy are made available for use during compilation by
   };
 
   /// Subclasses of GCStrategy are made available for use during compilation by

diff --git a/lib/CodeGen/ErlangGC.cpp b/lib/CodeGen/ErlangGC.cpp
index 85b089343ca14c6e5120625e5794d97cfb88c76c..5f2b3a0983262edc141890506e1bd73ce8e762a4 100644 (file)
--- a/lib/CodeGen/ErlangGC.cpp
+++ b/lib/CodeGen/ErlangGC.cpp
@@ -28,12 +28,8 @@ using namespace llvm;
 namespace {
 
   class ErlangGC : public GCStrategy {
 namespace {
 
   class ErlangGC : public GCStrategy {

-    MCSymbol *InsertLabel(MachineBasicBlock &MBB,
-                          MachineBasicBlock::iterator MI,
-                          DebugLoc DL) const;

   public:
     ErlangGC();
   public:
     ErlangGC();

-    bool findCustomSafePoints(GCFunctionInfo &FI, MachineFunction &MF) override;

   };
 
 }
   };
 
 }


@@ -48,35 +44,4 @@ ErlangGC::ErlangGC() {
   NeededSafePoints = 1 << GC::PostCall;
   UsesMetadata = true;
   CustomRoots = false;
   NeededSafePoints = 1 << GC::PostCall;
   UsesMetadata = true;
   CustomRoots = false;

-  CustomSafePoints = true;
-}
-
-MCSymbol *ErlangGC::InsertLabel(MachineBasicBlock &MBB,
-                                MachineBasicBlock::iterator MI,
-                                DebugLoc DL) const {
-  const TargetInstrInfo *TII = MBB.getParent()->getSubtarget().getInstrInfo();
-  MCSymbol *Label = MBB.getParent()->getContext().CreateTempSymbol();
-  BuildMI(MBB, MI, DL, TII->get(TargetOpcode::GC_LABEL)).addSym(Label);
-  return Label;
-}
-
-bool ErlangGC::findCustomSafePoints(GCFunctionInfo &FI, MachineFunction &MF) {
-  for (MachineFunction::iterator BBI = MF.begin(), BBE = MF.end(); BBI != BBE;
-       ++BBI)
-    for (MachineBasicBlock::iterator MI = BBI->begin(), ME = BBI->end();
-         MI != ME; ++MI)
-
-      if (MI->getDesc().isCall()) {
-
-        // Do not treat tail call sites as safe points.
-        if (MI->getDesc().isTerminator())
-          continue;
-
-        /* Code copied from VisitCallPoint(...) */
-        MachineBasicBlock::iterator RAI = MI; ++RAI;
-        MCSymbol* Label = InsertLabel(*MI->getParent(), RAI, MI->getDebugLoc());
-        FI.addSafePoint(GC::PostCall, Label, MI->getDebugLoc());
-      }
-
-  return false;

 }
 }

diff --git a/lib/CodeGen/GCRootLowering.cpp b/lib/CodeGen/GCRootLowering.cpp
index 9907a3022534bd25edd2524822dfdc1d4fa41ff1..9bda7b8493ff25e11a9126a7553af3efd6eb5045 100644 (file)
--- a/lib/CodeGen/GCRootLowering.cpp
+++ b/lib/CodeGen/GCRootLowering.cpp
@@ -329,8 +329,15 @@ void GCMachineCodeAnalysis::FindSafePoints(MachineFunction &MF) {
        ++BBI)
     for (MachineBasicBlock::iterator MI = BBI->begin(), ME = BBI->end();
          MI != ME; ++MI)
        ++BBI)
     for (MachineBasicBlock::iterator MI = BBI->begin(), ME = BBI->end();
          MI != ME; ++MI)

-      if (MI->isCall())
+      if (MI->isCall()) {
+        // Do not treat tail or sibling call sites as safe points.  This is
+        // legal since any arguments passed to the callee which live in the
+        // remnants of the callers frame will be owned and updated by the
+        // callee if required.
+        if (MI->isTerminator())
+          continue;

         VisitCallPoint(MI);
         VisitCallPoint(MI);

+      }

 }
 
 void GCMachineCodeAnalysis::FindStackOffsets(MachineFunction &MF) {
 }
 
 void GCMachineCodeAnalysis::FindStackOffsets(MachineFunction &MF) {


@@ -366,11 +373,7 @@ bool GCMachineCodeAnalysis::runOnMachineFunction(MachineFunction &MF) {
   FI->setFrameSize(MF.getFrameInfo()->getStackSize());
 
   // Find all safe points.
   FI->setFrameSize(MF.getFrameInfo()->getStackSize());
 
   // Find all safe points.

-  if (FI->getStrategy().customSafePoints()) {
-    FI->getStrategy().findCustomSafePoints(*FI, MF);
-  } else {
-    FindSafePoints(MF);
-  }
+  FindSafePoints(MF);

 
   // Find the stack offsets for all roots.
   FindStackOffsets(MF);
 
   // Find the stack offsets for all roots.
   FindStackOffsets(MF);

diff --git a/lib/CodeGen/GCStrategy.cpp b/lib/CodeGen/GCStrategy.cpp
index 05db8c44a5d6009da682f51d5da7f19195058645..2b687d9dd20e976779b8e2130141604ee6852b4b 100644 (file)
--- a/lib/CodeGen/GCStrategy.cpp
+++ b/lib/CodeGen/GCStrategy.cpp
@@ -18,5 +18,5 @@ using namespace llvm;
 
 GCStrategy::GCStrategy()
     : UseStatepoints(false), NeededSafePoints(0), CustomReadBarriers(false),
 
 GCStrategy::GCStrategy()
     : UseStatepoints(false), NeededSafePoints(0), CustomReadBarriers(false),

-      CustomWriteBarriers(false), CustomRoots(false), CustomSafePoints(false),
+      CustomWriteBarriers(false), CustomRoots(false),

       InitRoots(true), UsesMetadata(false) {}
       InitRoots(true), UsesMetadata(false) {}

diff --git a/lib/CodeGen/StatepointExampleGC.cpp b/lib/CodeGen/StatepointExampleGC.cpp
index 802cf132b34df30bf1bb30e340f833804a9a2c43..09b74ca1d2dd267c302d091b3e3a524d3a116c82 100644 (file)
--- a/lib/CodeGen/StatepointExampleGC.cpp
+++ b/lib/CodeGen/StatepointExampleGC.cpp
@@ -33,7 +33,6 @@ public:
     NeededSafePoints = 0;
     UsesMetadata = false;
     CustomRoots = false;
     NeededSafePoints = 0;
     UsesMetadata = false;
     CustomRoots = false;

-    CustomSafePoints = false;

   }
   Optional<bool> isGCManagedPointer(const Value *V) const override {
     // Method is only valid on pointer typed values.
   }
   Optional<bool> isGCManagedPointer(const Value *V) const override {
     // Method is only valid on pointer typed values.