[llvm-commits] [PATCH] Memory footprint improvement of pass GlobalOpt (refined version)
Andrew Trick
atrick at apple.com
Sun Dec 2 23:41:41 PST 2012
On Dec 2, 2012, at 11:17 PM, Zhou Sheng <zhousheng00 at gmail.com> wrote:
> Hi nicholas,
>
> Refined the patch according to Nick & Andy's feedback.
Thanks for the comments.
I assume you can't write a strong unit test because the dead initializers are removed later in the pass either way. Is that true? If you're relying on existing unit tests to exercise this code rather than adding your own, please indicate that in the checkin comment.
Your patch looks reasonable. I'm not familiar with this pass though, so there may be a better way. Please get another reviewer to sign off on it too.
-Andy
> The attached patch is to improve the memory footprint of pass GlobalOpt. Please kindly let me know if the patch is good to commit.
> I created some case (attached in my previous email) to repeat the issue, on which 'opt -globalopt' consumes 3.2GB memory and is stuck for long time.
> With my patch the memory footprint is reduced to 300MB and GlobalOpt transform finishes in 3mins. (My machine info: 16 Intel(R) Xeon(R) CPU E5620 at 2.40GHz, 16GB)
>
> The big memory footprint cause is that current GlobalOpt one by one hoists and stores the leaf element constant into the global array, in each iteration, it recreates the global array initializer constant and leave the old initializer alone. This may result in many obsolete constants left.
> For example: we have global array @rom = global [16 x i32] zeroinitializer
> After the first element value is hoisted and installed: @rom = global [16 x i32] [ 1, 0, 0, ... ]
> After the second element value is installed: @rom = global [16 x 32] [ 1, 2, 0, 0, ... ] // here the previous initializer is obsolete
> ...
> When the transform is done, we have 15 obsolete initializers left useless.
>
> My solution is to use a container to record those obsolete constants created in each iteration in EvaluateStoreInto(),
> and then destroy them in the end of CommitValueTo() which is the caller of EvaluateStoreInto().
>
> Note here I choose to use SetVector as container because we need to keep the insertion order of the obsolete constants, as the global variable initializer is created in hierachical way which is bottom up. Therefore, we need to delete the obsolete ones in topdown way. For example:
>
> @rom = global [16 x [1024 x i32]] [[1, 2, 3, ...,1024 ] [2, 4, ...2048] ... ]
>
> For the above global variable, we also generate 16 constants [1024 x i32] before we generate the initializer every time, so when we delete the obsolete initializer, we need to make sure we delete the 16 [1024xi32] after [16 x [1024 x i32]] is deleted, otherwise there will be non-use-empty issue.
>
> http://llvm-reviews.chandlerc.com/D155
>
> Files:
> lib/Transforms/IPO/GlobalOpt.cpp
>
> Index: lib/Transforms/IPO/GlobalOpt.cpp
> ===================================================================
> --- lib/Transforms/IPO/GlobalOpt.cpp
> +++ lib/Transforms/IPO/GlobalOpt.cpp
> @@ -36,6 +36,7 @@
> #include "llvm/ADT/DenseMap.h"
> #include "llvm/ADT/SmallPtrSet.h"
> #include "llvm/ADT/SmallVector.h"
> +#include "llvm/ADT/SetVector.h"
> #include "llvm/ADT/Statistic.h"
> #include "llvm/ADT/STLExtras.h"
> #include <algorithm>
> @@ -2398,7 +2399,8 @@
> /// initializer. This returns 'Init' modified to reflect 'Val' stored into it.
> /// At this point, the GEP operands of Addr [0, OpNo) have been stepped into.
> static Constant *EvaluateStoreInto(Constant *Init, Constant *Val,
> - ConstantExpr *Addr, unsigned OpNo) {
> + ConstantExpr *Addr, unsigned OpNo,
> + SetVector<Constant*> &Obsolete) {
> // Base case of the recursion.
> if (OpNo == Addr->getNumOperands()) {
> assert(Val->getType() == Init->getType() && "Type mismatch!");
> @@ -2415,7 +2417,9 @@
> ConstantInt *CU = cast<ConstantInt>(Addr->getOperand(OpNo));
> unsigned Idx = CU->getZExtValue();
> assert(Idx < STy->getNumElements() && "Struct index out of range!");
> - Elts[Idx] = EvaluateStoreInto(Elts[Idx], Val, Addr, OpNo+1);
> + if (Elts[Idx]->getType()->isAggregateType())
> + Obsolete.insert(Elts[Idx]);
> + Elts[Idx] = EvaluateStoreInto(Elts[Idx], Val, Addr, OpNo+1, Obsolete);
>
> // Return the modified struct.
> return ConstantStruct::get(STy, Elts);
> @@ -2435,8 +2439,11 @@
> Elts.push_back(Init->getAggregateElement(i));
>
> assert(CI->getZExtValue() < NumElts);
> + Constant *OrigElem = Elts[CI->getZExtValue()];
> + if (OrigElem->getType()->isAggregateType())
> + Obsolete.insert(OrigElem);
> Elts[CI->getZExtValue()] =
> - EvaluateStoreInto(Elts[CI->getZExtValue()], Val, Addr, OpNo+1);
> + EvaluateStoreInto(OrigElem, Val, Addr, OpNo+1, Obsolete);
>
> if (Init->getType()->isArrayTy())
> return ConstantArray::get(cast<ArrayType>(InitTy), Elts);
> @@ -2452,9 +2459,33 @@
> return;
> }
>
> + // Collect obsolete constants created during EvaluateStoreInto() invoke,
> + // and then destroy them in the end of this function.
> + // This is to improve the memory footprint, because current algorithm is to
> + // one by one hoist and store the leaf element constant into the global array,
> + // in each iteration, it recreates the global array initializer constant and
> + // leave the old initializer alone. This may result in many obsolete constants
> + // left still. In some case, with big global array:
> + // @rom = [16 x [2048 x [5 x i32]]];
> + // it will consume over 3.2GB memory, and the transform will get stuck.
> + //
> + // Here we choose to use SetVector as container because we need to keep
> + // the insertion order of the obsolete constants as the global variable
> + // initializer is created in hierachical way which is bottom up. So we
> + // need to delete them in topdown way to avoid non-use-empty issue.
> + SetVector<Constant*> Obsolete;
> ConstantExpr *CE = cast<ConstantExpr>(Addr);
> GlobalVariable *GV = cast<GlobalVariable>(CE->getOperand(0));
> - GV->setInitializer(EvaluateStoreInto(GV->getInitializer(), Val, CE, 2));
> + Constant *OrigInit = GV->getInitializer();
> + if (OrigInit->getType()->isAggregateType())
> + Obsolete.insert(OrigInit);
> + Constant *Init = EvaluateStoreInto(OrigInit, Val, CE, 2, Obsolete);
> + GV->setInitializer(Init);
> +
> + for (unsigned i = 0, e = Obsolete.size(); i != e; ++i) {
> + if (Obsolete[i]->use_empty())
> + Obsolete[i]->destroyConstant();
> + }
> }
>
> namespace {
> <D155.1.patch>_______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list