[llvm] r346621 - [GCRoot] Remove some unneccessary complexity
Philip Reames via llvm-commits
llvm-commits at lists.llvm.org
Sun Nov 11 13:13:09 PST 2018
Author: reames
Date: Sun Nov 11 13:13:09 2018
New Revision: 346621
URL: http://llvm.org/viewvc/llvm-project?rev=346621&view=rev
Log:
[GCRoot] Remove some unneccessary complexity
The GCStrategy provides three configuration options were are largely redundant.
1) Support for conditionally lowering gcread and gcwrite to loads and stores. This is redundant since any GC which wished to use these abstractions would lower them out of existance before the built in lowering anyways. As such, there's no need to have the lowering being conditional.
2) Conditional initialization for allocas marked via gcroot. Semantically, roots have to be initialized before first potential use. Arguably, the frontend really should have responsibility for that, but the old API allowed the frontend to ignore this detail. Only one builtin GC used the non-initializing mode. Since no one to my knowledge actually uses the ErlangGC strategy, I decide the slight pessimization was worth the simplicity. If that turns out to be problematic, we can always improve the insertion algorithm to detect more existing initializing stores.
Modified:
llvm/trunk/include/llvm/CodeGen/GCStrategy.h
llvm/trunk/lib/CodeGen/BuiltinGCs.cpp
llvm/trunk/lib/CodeGen/GCRootLowering.cpp
Modified: llvm/trunk/include/llvm/CodeGen/GCStrategy.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/GCStrategy.h?rev=346621&r1=346620&r2=346621&view=diff
==============================================================================
--- llvm/trunk/include/llvm/CodeGen/GCStrategy.h (original)
+++ llvm/trunk/include/llvm/CodeGen/GCStrategy.h Sun Nov 11 13:13:09 2018
@@ -89,10 +89,7 @@ protected:
/// anything but their default values.
unsigned NeededSafePoints = 0; ///< Bitmask of required safe points.
- bool CustomReadBarriers = false; ///< Default is to insert loads.
- bool CustomWriteBarriers = false; ///< Default is to insert stores.
bool CustomRoots = false; ///< Default is to pass through to backend.
- bool InitRoots= true; ///< If set, roots are nulled during lowering.
bool UsesMetadata = false; ///< If set, backend must emit metadata tables.
public:
@@ -103,16 +100,6 @@ public:
/// name string specified on functions which use this strategy.
const std::string &getName() const { return Name; }
- /// By default, write barriers are replaced with simple store
- /// instructions. If true, you must provide a custom pass to lower
- /// calls to \@llvm.gcwrite.
- bool customWriteBarrier() const { return CustomWriteBarriers; }
-
- /// By default, read barriers are replaced with simple load
- /// instructions. If true, you must provide a custom pass to lower
- /// calls to \@llvm.gcread.
- bool customReadBarrier() const { return CustomReadBarriers; }
-
/// Returns true if this strategy is expecting the use of gc.statepoints,
/// and false otherwise.
bool useStatepoints() const { return UseStatepoints; }
@@ -150,11 +137,6 @@ public:
/// calls to \@llvm.gcroot.
bool customRoots() const { return CustomRoots; }
- /// 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.
- bool initializeRoots() const { return InitRoots; }
-
/// If set, appropriate metadata tables must be emitted by the back-end
/// (assembler, JIT, or otherwise). For statepoint, this method is
/// currently unsupported. The stackmap information can be found in the
Modified: llvm/trunk/lib/CodeGen/BuiltinGCs.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/BuiltinGCs.cpp?rev=346621&r1=346620&r2=346621&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/BuiltinGCs.cpp (original)
+++ llvm/trunk/lib/CodeGen/BuiltinGCs.cpp Sun Nov 11 13:13:09 2018
@@ -28,7 +28,6 @@ namespace {
class ErlangGC : public GCStrategy {
public:
ErlangGC() {
- InitRoots = false;
NeededSafePoints = 1 << GC::PostCall;
UsesMetadata = true;
CustomRoots = false;
@@ -57,7 +56,6 @@ public:
class ShadowStackGC : public GCStrategy {
public:
ShadowStackGC() {
- InitRoots = true;
CustomRoots = true;
}
};
@@ -74,7 +72,6 @@ public:
UseStatepoints = true;
// These options are all gc.root specific, we specify them so that the
// gc.root lowering code doesn't run.
- InitRoots = false;
NeededSafePoints = 0;
UsesMetadata = false;
CustomRoots = false;
@@ -108,7 +105,6 @@ public:
UseStatepoints = true;
// These options are all gc.root specific, we specify them so that the
// gc.root lowering code doesn't run.
- InitRoots = false;
NeededSafePoints = 0;
UsesMetadata = false;
CustomRoots = false;
Modified: llvm/trunk/lib/CodeGen/GCRootLowering.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/GCRootLowering.cpp?rev=346621&r1=346620&r2=346621&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/GCRootLowering.cpp (original)
+++ llvm/trunk/lib/CodeGen/GCRootLowering.cpp Sun Nov 11 13:13:09 2018
@@ -38,7 +38,7 @@ namespace {
/// directed by the GCStrategy. It also performs automatic root initialization
/// and custom intrinsic lowering.
class LowerIntrinsics : public FunctionPass {
- bool PerformDefaultLowering(Function &F, GCStrategy &S);
+ bool DoLowering(Function &F, GCStrategy &S);
public:
static char ID;
@@ -102,13 +102,6 @@ void LowerIntrinsics::getAnalysisUsage(A
AU.addPreserved<DominatorTreeWrapperPass>();
}
-static bool NeedsDefaultLoweringPass(const GCStrategy &C) {
- // Default lowering is necessary only if read or write barriers have a default
- // action. The default for roots is no action.
- return !C.customWriteBarrier() || !C.customReadBarrier() ||
- C.initializeRoots();
-}
-
/// doInitialization - If this module uses the GC intrinsics, find them now.
bool LowerIntrinsics::doInitialization(Module &M) {
GCModuleInfo *MI = getAnalysisIfAvailable<GCModuleInfo>();
@@ -188,19 +181,17 @@ bool LowerIntrinsics::runOnFunction(Func
GCFunctionInfo &FI = getAnalysis<GCModuleInfo>().getFunctionInfo(F);
GCStrategy &S = FI.getStrategy();
- bool MadeChange = false;
-
- if (NeedsDefaultLoweringPass(S))
- MadeChange |= PerformDefaultLowering(F, S);
-
- return MadeChange;
+ return DoLowering(F, S);
}
-bool LowerIntrinsics::PerformDefaultLowering(Function &F, GCStrategy &S) {
- bool LowerWr = !S.customWriteBarrier();
- bool LowerRd = !S.customReadBarrier();
- bool InitRoots = S.initializeRoots();
-
+/// Lower barriers out of existance (if the associated GCStrategy hasn't
+/// already done so...), and insert initializing stores to roots as a defensive
+/// measure. Given we're going to report all roots live at all safepoints, we
+/// need to be able to ensure each root has been initialized by the point the
+/// first safepoint is reached. This really should have been done by the
+/// frontend, but the old API made this non-obvious, so we do a potentially
+/// redundant store just in case.
+bool LowerIntrinsics::DoLowering(Function &F, GCStrategy &S) {
SmallVector<AllocaInst *, 32> Roots;
bool MadeChange = false;
@@ -209,37 +200,33 @@ bool LowerIntrinsics::PerformDefaultLowe
if (IntrinsicInst *CI = dyn_cast<IntrinsicInst>(II++)) {
Function *F = CI->getCalledFunction();
switch (F->getIntrinsicID()) {
- case Intrinsic::gcwrite:
- if (LowerWr) {
- // Replace a write barrier with a simple store.
- Value *St =
- new StoreInst(CI->getArgOperand(0), CI->getArgOperand(2), CI);
- CI->replaceAllUsesWith(St);
- CI->eraseFromParent();
- }
+ default: break;
+ case Intrinsic::gcwrite: {
+ // Replace a write barrier with a simple store.
+ Value *St = new StoreInst(CI->getArgOperand(0),
+ CI->getArgOperand(2), CI);
+ CI->replaceAllUsesWith(St);
+ CI->eraseFromParent();
+ MadeChange = true;
break;
- case Intrinsic::gcread:
- if (LowerRd) {
- // Replace a read barrier with a simple load.
- Value *Ld = new LoadInst(CI->getArgOperand(1), "", CI);
- Ld->takeName(CI);
- CI->replaceAllUsesWith(Ld);
- CI->eraseFromParent();
- }
+ }
+ case Intrinsic::gcread: {
+ // Replace a read barrier with a simple load.
+ Value *Ld = new LoadInst(CI->getArgOperand(1), "", CI);
+ Ld->takeName(CI);
+ CI->replaceAllUsesWith(Ld);
+ CI->eraseFromParent();
+ MadeChange = true;
break;
- case Intrinsic::gcroot:
- if (InitRoots) {
- // Initialize the GC root, but do not delete the intrinsic. The
- // backend needs the intrinsic to flag the stack slot.
- Roots.push_back(
- cast<AllocaInst>(CI->getArgOperand(0)->stripPointerCasts()));
- }
+ }
+ case Intrinsic::gcroot: {
+ // Initialize the GC root, but do not delete the intrinsic. The
+ // backend needs the intrinsic to flag the stack slot.
+ Roots.push_back(
+ cast<AllocaInst>(CI->getArgOperand(0)->stripPointerCasts()));
break;
- default:
- continue;
}
-
- MadeChange = true;
+ }
}
}
}
More information about the llvm-commits
mailing list