[llvm-commits] [poolalloc] r107283 - in /poolalloc/trunk: include/poolalloc/PoolAllocate.h lib/PoolAllocate/PoolAllocate.cpp
John Criswell
criswell at uiuc.edu
Wed Jun 30 07:30:22 PDT 2010
Author: criswell
Date: Wed Jun 30 09:30:22 2010
New Revision: 107283
URL: http://llvm.org/viewvc/llvm-project?rev=107283&view=rev
Log:
Added more FIXME comments from code review.
Improved some of the formatting issues.
Modified:
poolalloc/trunk/include/poolalloc/PoolAllocate.h
poolalloc/trunk/lib/PoolAllocate/PoolAllocate.cpp
Modified: poolalloc/trunk/include/poolalloc/PoolAllocate.h
URL: http://llvm.org/viewvc/llvm-project/poolalloc/trunk/include/poolalloc/PoolAllocate.h?rev=107283&r1=107282&r2=107283&view=diff
==============================================================================
--- poolalloc/trunk/include/poolalloc/PoolAllocate.h (original)
+++ poolalloc/trunk/include/poolalloc/PoolAllocate.h Wed Jun 30 09:30:22 2010
@@ -88,6 +88,8 @@
/// DSNode. Note: This does not necessarily include pool arguments that are
/// passed in because of indirect function calls that are not used in the
/// function.
+ /// FIXME: This comment should clearly describe which pools are and are not
+ /// in this data structure.
std::map<const DSNode*, Value*> PoolDescriptors;
// Reverse mapping for PoolDescriptors, needed by TPPA
Modified: poolalloc/trunk/lib/PoolAllocate/PoolAllocate.cpp
URL: http://llvm.org/viewvc/llvm-project/poolalloc/trunk/lib/PoolAllocate/PoolAllocate.cpp?rev=107283&r1=107282&r2=107283&view=diff
==============================================================================
--- poolalloc/trunk/lib/PoolAllocate/PoolAllocate.cpp (original)
+++ poolalloc/trunk/lib/PoolAllocate/PoolAllocate.cpp Wed Jun 30 09:30:22 2010
@@ -216,6 +216,11 @@
}
}
+ //
+ // FIXME: Make name more descriptive and explain, in a comment here, what this
+ // code is trying to do (namely, avoid optimizations for performance
+ // overhead measurements?).
+ //
if (CurHeuristic->IsRealHeuristic())
MicroOptimizePoolCalls();
@@ -350,6 +355,8 @@
}
}
+/// FIXME: Should these be in the pooloptimize pass?
+///
/// MicroOptimizePoolCalls - Apply any microoptimizations to calls to pool
/// allocation function calls that we can. This runs after the whole program
/// has been transformed.
@@ -449,6 +456,7 @@
//
// Mark the returned node as needing to be passed in.
+ // FIXME: We should not do this for main().
//
if (DSNode *RetNode = G->getReturnNodeFor(F).getNode())
RetNode->markReachableNodes(MarkedNodes);
@@ -469,7 +477,7 @@
//
// FIXME:
// 1) PassAllArguments seems to be ignored here. Why is that?
- // 2) Why is the heap node check part of the PassAllArguments check?
+ // 2) Should the heap node check be part of the PassAllArguments check?
// 3) SAFECode probably needs to pass the pool even if it's not a heap node.
// We should probably just do what the heuristic tells us to do.
//
@@ -518,12 +526,15 @@
if (G->node_begin() == G->node_end()) return 0;
FuncInfo &FI = *getFuncInfo(F);
+
+ // No need to clone if no pools need to be passed in!
if (FI.ArgNodes.empty())
- return 0; // No need to clone if no pools need to be passed in!
+ return 0;
// Update statistics..
NumArgsAdded += FI.ArgNodes.size();
- if (MaxArgsAdded < FI.ArgNodes.size()) MaxArgsAdded = FI.ArgNodes.size();
+ if (MaxArgsAdded < FI.ArgNodes.size())
+ MaxArgsAdded = FI.ArgNodes.size();
++NumCloned;
//
@@ -539,7 +550,12 @@
// Create the new function prototype
FunctionType *FuncTy = FunctionType::get(OldFuncTy->getReturnType(), ArgTys,
OldFuncTy->isVarArg());
+
+ //
+ // FIXME: Can probably add new function to module during creation
+ //
// Create the new function...
+ //
Function *New = Function::Create(FuncTy, Function::InternalLinkage, F.getName());
New->copyAttributesFrom(&F);
F.getParent()->getFunctionList().insert(&F, New);
@@ -554,9 +570,15 @@
PoolDescriptors[FI.ArgNodes[i]] = NI;
}
+ //
// Map the existing arguments of the old function to the corresponding
// arguments of the new function, and copy over the names.
+ //
+ //
DenseMap<const Value*, Value*> ValueMap;
+ // FIXME: Remove use of SAFECodeEnabled flag
+ // FIXME: Is FI.ValueMap empty? We should put an assert to verify that it
+ // is.
if (SAFECodeEnabled)
for (std::map<const Value*, Value*>::iterator I = FI.ValueMap.begin(),
E = FI.ValueMap.end(); I != E; ++I)
@@ -581,6 +603,9 @@
NewToOldValueMap.insert(std::make_pair(I->second, I->first));
//
+ // FIXME: File a bug report for CloneFunctionInto; it should take care of
+ // this mess for us. Also check whether it does it correctly.
+ //
// The cloned function will have its function attributes set more or less
// correctly at this point. However, it will not have its parameter
// attributes set correctly. We need to go through each argument in the
@@ -619,8 +644,8 @@
//
// FIXME: Update comment
//
-// FIXME: Global pools should probably be initialized by a global ctor instead of by
-// main().
+// FIXME: Global pools should probably be initialized by a global ctor instead
+// of by main().
//
// SetupGlobalPools - Create global pools for all DSNodes in the globals graph
// which contain heap objects. If a global variable points to a piece of memory
@@ -867,6 +892,10 @@
I != E;
++I){
//
+ // FIXME: Don't do SAFECode specific behavior here; follow the heuristic.
+ // FIXME: Are there nodes which don't have the heap flag localally but have
+ // it set in the globals graph?
+ //
// Only the following nodes are pool allocated:
// 1) Heap nodes
// 2) Array nodes when bounds checking is enabled.
@@ -874,7 +903,8 @@
//
DSNode *N = I;
if ((N->isHeapNode()) || (BoundsChecksEnabled && (N->isArrayNode())) ||
- (GlobalsGraphNodeMapping.count(N) && GlobalsGraphNodeMapping[N].getNode()->isHeapNode())) {
+ (GlobalsGraphNodeMapping.count(N) &&
+ GlobalsGraphNodeMapping[N].getNode()->isHeapNode())) {
if (GlobalsGraphNodeMapping.count(N)) {
// If it is a global pool, set up the pool descriptor appropriately.
DSNode *GGN = GlobalsGraphNodeMapping[N].getNode();
More information about the llvm-commits
mailing list