[llvm] r244154 - Record whether the weights on out-edges from a MBB are normalized.
Cong Hou via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 6 11:14:57 PDT 2015
Sorry about breaking your builds. I will revert it now, and start to
investigate it soon.
thanks,
Cong
On Thu, Aug 6, 2015 at 10:33 AM, Xinliang David Li <xinliangli at gmail.com> wrote:
> yes, please revert first and investigate the root cause.
>
> David
>
> On Thu, Aug 6, 2015 at 10:09 AM, Hal Finkel via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
>>
>> Hi Cong,
>>
>> This caused PR24377 (which is completely breaking my builds because it
>> causes Clang to assert when building libomp). If this will not be quick to
>> fix, please revert in the mean time.
>>
>> Thanks in advance,
>> Hal
>>
>> ----- Original Message -----
>> > From: "Cong Hou" <congh at google.com>
>> > To: llvm-commits at lists.llvm.org
>> > Sent: Wednesday, August 5, 2015 5:01:20 PM
>> > Subject: [llvm] r244154 - Record whether the weights on out-edges from a
>> > MBB are normalized.
>> >
>> > Author: conghou
>> > Date: Wed Aug 5 17:01:20 2015
>> > New Revision: 244154
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=244154&view=rev
>> > Log:
>> > Record whether the weights on out-edges from a MBB are normalized.
>> >
>> > 1. Create a utility function normalizeEdgeWeights() in
>> > MachineBranchProbabilityInfo that normalizes a list of edge weights
>> > so that the sum of then can fit in uint32_t.
>> > 2. Provide an interface in MachineBasicBlock to normalize its
>> > successors' weights.
>> > 3. Add a flag in MachineBasicBlock that tracks whether its
>> > successors' weights are normalized.
>> > 4. Provide an overload of getSumForBlock that accepts a non-const
>> > pointer to a MBB so that it can force normalizing this MBB's
>> > successors' weights.
>> > 5. Update several uses of getSumForBlock() by eliminating the once
>> > needed weight scale.
>> >
>> > Differential Revision: http://reviews.llvm.org/D11442
>> >
>> >
>> > Modified:
>> > llvm/trunk/include/llvm/CodeGen/MachineBasicBlock.h
>> > llvm/trunk/include/llvm/CodeGen/MachineBranchProbabilityInfo.h
>> > llvm/trunk/lib/CodeGen/IfConversion.cpp
>> > llvm/trunk/lib/CodeGen/MachineBasicBlock.cpp
>> > llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp
>> > llvm/trunk/lib/CodeGen/MachineBranchProbabilityInfo.cpp
>> >
>> > Modified: llvm/trunk/include/llvm/CodeGen/MachineBasicBlock.h
>> > URL:
>> >
>> > http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/MachineBasicBlock.h?rev=244154&r1=244153&r2=244154&view=diff
>> >
>> > ==============================================================================
>> > --- llvm/trunk/include/llvm/CodeGen/MachineBasicBlock.h (original)
>> > +++ llvm/trunk/include/llvm/CodeGen/MachineBasicBlock.h Wed Aug 5
>> > 17:01:20 2015
>> > @@ -65,6 +65,10 @@ class MachineBasicBlock : public ilist_n
>> > Instructions Insts;
>> > const BasicBlock *BB;
>> > int Number;
>> > +
>> > + /// A flag tracking whether the weights of all successors are
>> > normalized.
>> > + bool AreSuccWeightsNormalized;
>> > +
>> > MachineFunction *xParent;
>> >
>> > /// Keep track of the predecessor / successor basicblocks.
>> > @@ -129,6 +133,9 @@ public:
>> > const MachineFunction *getParent() const { return xParent; }
>> > MachineFunction *getParent() { return xParent; }
>> >
>> > + /// Return whether all weights of successors are normalized.
>> > + bool areSuccWeightsNormalized() const { return
>> > AreSuccWeightsNormalized; }
>> > +
>> > /// MachineBasicBlock iterator that automatically skips over MIs
>> > that are
>> > /// inside bundles (i.e. walk top level MIs only).
>> > template<typename Ty, typename IterTy>
>> > @@ -384,6 +391,12 @@ public:
>> > /// Set successor weight of a given iterator.
>> > void setSuccWeight(succ_iterator I, uint32_t weight);
>> >
>> > + /// Normalize all succesor weights so that the sum of them does
>> > not exceed
>> > + /// UINT32_MAX. Return true if the weights are modified and false
>> > otherwise.
>> > + /// Note that weights that are modified after calling this
>> > function are not
>> > + /// guaranteed to be normalized.
>> > + bool normalizeSuccWeights();
>> > +
>> > /// Remove successor from the successors list of this
>> > MachineBasicBlock. The
>> > /// Predecessors list of succ is automatically updated.
>> > void removeSuccessor(MachineBasicBlock *succ);
>> >
>> > Modified:
>> > llvm/trunk/include/llvm/CodeGen/MachineBranchProbabilityInfo.h
>> > URL:
>> >
>> > http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/MachineBranchProbabilityInfo.h?rev=244154&r1=244153&r2=244154&view=diff
>> >
>> > ==============================================================================
>> > --- llvm/trunk/include/llvm/CodeGen/MachineBranchProbabilityInfo.h
>> > (original)
>> > +++ llvm/trunk/include/llvm/CodeGen/MachineBranchProbabilityInfo.h
>> > Wed Aug 5 17:01:20 2015
>> > @@ -59,6 +59,10 @@ public:
>> > // adjustment. Any edge weights used with the sum should be
>> > divided by Scale.
>> > uint32_t getSumForBlock(const MachineBasicBlock *MBB, uint32_t
>> > &Scale) const;
>> >
>> > + // Get sum of the block successors' weights, and force normalizing
>> > the
>> > + // successors' weights of MBB so that their sum fit within
>> > 32-bits.
>> > + uint32_t getSumForBlock(MachineBasicBlock *MBB) const;
>> > +
>> > // A 'Hot' edge is an edge which probability is >= 80%.
>> > bool isEdgeHot(const MachineBasicBlock *Src,
>> > const MachineBasicBlock *Dst) const;
>> > @@ -82,8 +86,34 @@ public:
>> > raw_ostream &printEdgeProbability(raw_ostream &OS,
>> > const MachineBasicBlock *Src,
>> > const MachineBasicBlock *Dst)
>> > const;
>> > +
>> > + // Normalize a list of weights by scaling them down so that the
>> > sum of them
>> > + // doesn't exceed UINT32_MAX. Return the scale.
>> > + template <class WeightList>
>> > + static uint32_t normalizeEdgeWeights(WeightList &Weights);
>> > };
>> >
>> > +template <class WeightList>
>> > +uint32_t
>> > +MachineBranchProbabilityInfo::normalizeEdgeWeights(WeightList
>> > &Weights) {
>> > + assert(Weights.size() < UINT32_MAX && "Too many weights in the
>> > list!");
>> > + // First we compute the sum with 64-bits of precision.
>> > + uint64_t Sum = std::accumulate(Weights.begin(), Weights.end(),
>> > uint64_t(0));
>> > +
>> > + // If the computed sum fits in 32-bits, we're done.
>> > + if (Sum <= UINT32_MAX)
>> > + return 1;
>> > +
>> > + // Otherwise, compute the scale necessary to cause the weights to
>> > fit, and
>> > + // re-sum with that scale applied.
>> > + assert((Sum / UINT32_MAX) < UINT32_MAX &&
>> > + "The sum of weights exceeds UINT32_MAX^2!");
>> > + uint32_t Scale = (Sum / UINT32_MAX) + 1;
>> > + for (auto &W : Weights)
>> > + W /= Scale;
>> > + return Scale;
>> > +}
>> > +
>> > }
>> >
>> >
>> >
>> > Modified: llvm/trunk/lib/CodeGen/IfConversion.cpp
>> > URL:
>> >
>> > http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/IfConversion.cpp?rev=244154&r1=244153&r2=244154&view=diff
>> >
>> > ==============================================================================
>> > --- llvm/trunk/lib/CodeGen/IfConversion.cpp (original)
>> > +++ llvm/trunk/lib/CodeGen/IfConversion.cpp Wed Aug 5 17:01:20 2015
>> > @@ -1232,15 +1232,17 @@ bool IfConverter::IfConvertTriangle(BBIn
>> >
>> > bool HasEarlyExit = CvtBBI->FalseBB != nullptr;
>> > uint64_t CvtNext = 0, CvtFalse = 0, BBNext = 0, BBCvt = 0,
>> > SumWeight = 0;
>> > - uint32_t WeightScale = 0;
>> >
>> > if (HasEarlyExit) {
>> > // Get weights before modifying CvtBBI->BB and BBI.BB.
>> > + // Explictly normalize the weights of all edges from CvtBBI->BB
>> > so that we
>> > + // are aware that the edge weights obtained below are
>> > normalized.
>> > + CvtBBI->BB->normalizeSuccWeights();
>> > CvtNext = MBPI->getEdgeWeight(CvtBBI->BB, NextBBI->BB);
>> > CvtFalse = MBPI->getEdgeWeight(CvtBBI->BB, CvtBBI->FalseBB);
>> > BBNext = MBPI->getEdgeWeight(BBI.BB, NextBBI->BB);
>> > BBCvt = MBPI->getEdgeWeight(BBI.BB, CvtBBI->BB);
>> > - SumWeight = MBPI->getSumForBlock(CvtBBI->BB, WeightScale);
>> > + SumWeight = MBPI->getSumForBlock(CvtBBI->BB);
>> > }
>> >
>> > if (CvtBBI->BB->pred_size() > 1) {
>> > @@ -1277,8 +1279,8 @@ bool IfConverter::IfConvertTriangle(BBIn
>> > // New_Weight(BBI.BB, CvtBBI->FalseBB) =
>> > // Weight(BBI.BB, CvtBBI->BB) * Weight(CvtBBI->BB,
>> > CvtBBI->FalseBB)
>> >
>> > - uint64_t NewNext = BBNext * SumWeight + (BBCvt * CvtNext) /
>> > WeightScale;
>> > - uint64_t NewFalse = (BBCvt * CvtFalse) / WeightScale;
>> > + uint64_t NewNext = BBNext * SumWeight + BBCvt * CvtNext;
>> > + uint64_t NewFalse = BBCvt * CvtFalse;
>> > // We need to scale down all weights of BBI.BB to fit uint32_t.
>> > // Here BBI.BB is connected to CvtBBI->FalseBB and will fall
>> > through to
>> > // the next block.
>> >
>> > Modified: llvm/trunk/lib/CodeGen/MachineBasicBlock.cpp
>> > URL:
>> >
>> > http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineBasicBlock.cpp?rev=244154&r1=244153&r2=244154&view=diff
>> >
>> > ==============================================================================
>> > --- llvm/trunk/lib/CodeGen/MachineBasicBlock.cpp (original)
>> > +++ llvm/trunk/lib/CodeGen/MachineBasicBlock.cpp Wed Aug 5 17:01:20
>> > 2015
>> > @@ -16,6 +16,7 @@
>> > #include "llvm/ADT/SmallString.h"
>> > #include "llvm/CodeGen/LiveIntervalAnalysis.h"
>> > #include "llvm/CodeGen/LiveVariables.h"
>> > +#include "llvm/CodeGen/MachineBranchProbabilityInfo.h"
>> > #include "llvm/CodeGen/MachineDominators.h"
>> > #include "llvm/CodeGen/MachineFunction.h"
>> > #include "llvm/CodeGen/MachineInstrBuilder.h"
>> > @@ -39,8 +40,9 @@ using namespace llvm;
>> > #define DEBUG_TYPE "codegen"
>> >
>> > MachineBasicBlock::MachineBasicBlock(MachineFunction &mf, const
>> > BasicBlock *bb)
>> > - : BB(bb), Number(-1), xParent(&mf), Alignment(0),
>> > IsLandingPad(false),
>> > - AddressTaken(false), CachedMCSymbol(nullptr) {
>> > + : BB(bb), Number(-1), AreSuccWeightsNormalized(false),
>> > xParent(&mf),
>> > + Alignment(0), IsLandingPad(false), AddressTaken(false),
>> > + CachedMCSymbol(nullptr) {
>> > Insts.Parent = this;
>> > }
>> >
>> > @@ -481,8 +483,10 @@ void MachineBasicBlock::addSuccessor(Mac
>> > if (weight != 0 && Weights.empty())
>> > Weights.resize(Successors.size());
>> >
>> > - if (weight != 0 || !Weights.empty())
>> > + if (weight != 0 || !Weights.empty()) {
>> > Weights.push_back(weight);
>> > + AreSuccWeightsNormalized = false;
>> > + }
>> >
>> > Successors.push_back(succ);
>> > succ->addPredecessor(this);
>> > @@ -1096,7 +1100,25 @@ uint32_t MachineBasicBlock::getSuccWeigh
>> > void MachineBasicBlock::setSuccWeight(succ_iterator I, uint32_t
>> > weight) {
>> > if (Weights.empty())
>> > return;
>> > - *getWeightIterator(I) = weight;
>> > + auto WeightIter = getWeightIterator(I);
>> > + uint32_t OldWeight = *WeightIter;
>> > + *WeightIter = weight;
>> > + if (weight > OldWeight)
>> > + AreSuccWeightsNormalized = false;
>> > +}
>> > +
>> > +/// Normalize all succesor weights so that the sum of them does not
>> > exceed
>> > +/// UINT32_MAX. Return true if the weights are modified and false
>> > otherwise.
>> > +/// Note that weights that are modified after calling this function
>> > are not
>> > +/// guaranteed to be normalized.
>> > +bool MachineBasicBlock::normalizeSuccWeights() {
>> > + if (!AreSuccWeightsNormalized) {
>> > + uint32_t Scale =
>> > + MachineBranchProbabilityInfo::normalizeEdgeWeights(Weights);
>> > + AreSuccWeightsNormalized = true;
>> > + return Scale != 1;
>> > + }
>> > + return false;
>> > }
>> >
>> > /// getWeightIterator - Return wight iterator corresonding to the I
>> > successor
>> >
>> > Modified: llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp
>> > URL:
>> >
>> > http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp?rev=244154&r1=244153&r2=244154&view=diff
>> >
>> > ==============================================================================
>> > --- llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp (original)
>> > +++ llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp Wed Aug 5
>> > 17:01:20 2015
>> > @@ -361,8 +361,7 @@ MachineBlockPlacement::selectBestSuccess
>> > // improve the MBPI interface to efficiently support query
>> > patterns such as
>> > // this.
>> > uint32_t BestWeight = 0;
>> > - uint32_t WeightScale = 0;
>> > - uint32_t SumWeight = MBPI->getSumForBlock(BB, WeightScale);
>> > + uint32_t SumWeight = MBPI->getSumForBlock(BB);
>> > DEBUG(dbgs() << "Attempting merge from: " << getBlockName(BB) <<
>> > "\n");
>> > for (MachineBasicBlock *Succ : BB->successors()) {
>> > if (BlockFilter && !BlockFilter->count(Succ))
>> > @@ -378,7 +377,7 @@ MachineBlockPlacement::selectBestSuccess
>> > }
>> >
>> > uint32_t SuccWeight = MBPI->getEdgeWeight(BB, Succ);
>> > - BranchProbability SuccProb(SuccWeight / WeightScale, SumWeight);
>> > + BranchProbability SuccProb(SuccWeight, SumWeight);
>> >
>> > // If we outline optional branches, look whether Succ is
>> > unavoidable, i.e.
>> > // dominates all terminators of the MachineFunction. If it does,
>> > other
>> > @@ -675,8 +674,7 @@ MachineBlockPlacement::findBestLoopExit(
>> > // FIXME: Due to the performance of the probability and weight
>> > routines in
>> > // the MBPI analysis, we use the internal weights and manually
>> > compute the
>> > // probabilities to avoid quadratic behavior.
>> > - uint32_t WeightScale = 0;
>> > - uint32_t SumWeight = MBPI->getSumForBlock(MBB, WeightScale);
>> > + uint32_t SumWeight = MBPI->getSumForBlock(MBB);
>> > for (MachineBasicBlock *Succ : MBB->successors()) {
>> > if (Succ->isLandingPad())
>> > continue;
>> > @@ -705,7 +703,7 @@ MachineBlockPlacement::findBestLoopExit(
>> > BlocksExitingToOuterLoop.insert(MBB);
>> > }
>> >
>> > - BranchProbability SuccProb(SuccWeight / WeightScale,
>> > SumWeight);
>> > + BranchProbability SuccProb(SuccWeight, SumWeight);
>> > BlockFrequency ExitEdgeFreq = MBFI->getBlockFreq(MBB) *
>> > SuccProb;
>> > DEBUG(dbgs() << " exiting: " << getBlockName(MBB) << " -> "
>> > << getBlockName(Succ) << " [L:" << SuccLoopDepth
>> > << "] (";
>> >
>> > Modified: llvm/trunk/lib/CodeGen/MachineBranchProbabilityInfo.cpp
>> > URL:
>> >
>> > http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineBranchProbabilityInfo.cpp?rev=244154&r1=244153&r2=244154&view=diff
>> >
>> > ==============================================================================
>> > --- llvm/trunk/lib/CodeGen/MachineBranchProbabilityInfo.cpp
>> > (original)
>> > +++ llvm/trunk/lib/CodeGen/MachineBranchProbabilityInfo.cpp Wed Aug
>> > 5 17:01:20 2015
>> > @@ -28,36 +28,35 @@ char MachineBranchProbabilityInfo::ID =
>> >
>> > void MachineBranchProbabilityInfo::anchor() { }
>> >
>> > -uint32_t MachineBranchProbabilityInfo::
>> > -getSumForBlock(const MachineBasicBlock *MBB, uint32_t &Scale) const
>> > {
>> > - // First we compute the sum with 64-bits of precision, ensuring
>> > that cannot
>> > - // overflow by bounding the number of weights considered.
>> > Hopefully no one
>> > - // actually needs 2^32 successors.
>> > - assert(MBB->succ_size() < UINT32_MAX);
>> > - uint64_t Sum = 0;
>> > - Scale = 1;
>> > +uint32_t
>> > +MachineBranchProbabilityInfo::getSumForBlock(MachineBasicBlock *MBB)
>> > const {
>> > + // Normalize the weights of MBB's all successors so that the sum
>> > is guaranteed
>> > + // to be no greater than UINT32_MAX.
>> > + MBB->normalizeSuccWeights();
>> > +
>> > + SmallVector<uint32_t, 8> Weights;
>> > for (MachineBasicBlock::const_succ_iterator I = MBB->succ_begin(),
>> > - E = MBB->succ_end(); I != E; ++I) {
>> > - uint32_t Weight = getEdgeWeight(MBB, I);
>> > - Sum += Weight;
>> > - }
>> > -
>> > - // If the computed sum fits in 32-bits, we're done.
>> > - if (Sum <= UINT32_MAX)
>> > - return Sum;
>> > -
>> > - // Otherwise, compute the scale necessary to cause the weights to
>> > fit, and
>> > - // re-sum with that scale applied.
>> > - assert((Sum / UINT32_MAX) < UINT32_MAX);
>> > - Scale = (Sum / UINT32_MAX) + 1;
>> > - Sum = 0;
>> > + E = MBB->succ_end();
>> > + I != E; ++I)
>> > + Weights.push_back(getEdgeWeight(MBB, I));
>> > +
>> > + return std::accumulate(Weights.begin(), Weights.end(), 0u);
>> > +}
>> > +
>> > +uint32_t
>> > +MachineBranchProbabilityInfo::getSumForBlock(const MachineBasicBlock
>> > *MBB,
>> > + uint32_t &Scale) const
>> > {
>> > + SmallVector<uint32_t, 8> Weights;
>> > for (MachineBasicBlock::const_succ_iterator I = MBB->succ_begin(),
>> > - E = MBB->succ_end(); I != E; ++I) {
>> > - uint32_t Weight = getEdgeWeight(MBB, I);
>> > - Sum += Weight / Scale;
>> > - }
>> > - assert(Sum <= UINT32_MAX);
>> > - return Sum;
>> > + E = MBB->succ_end();
>> > + I != E; ++I)
>> > + Weights.push_back(getEdgeWeight(MBB, I));
>> > +
>> > + if (MBB->areSuccWeightsNormalized())
>> > + Scale = 1;
>> > + else
>> > + Scale =
>> > MachineBranchProbabilityInfo::normalizeEdgeWeights(Weights);
>> > + return std::accumulate(Weights.begin(), Weights.end(), 0u);
>> > }
>> >
>> > uint32_t MachineBranchProbabilityInfo::
>> >
>> >
>> > _______________________________________________
>> > llvm-commits mailing list
>> > llvm-commits at lists.llvm.org
>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>> >
>>
>> --
>> Hal Finkel
>> Assistant Computational Scientist
>> Leadership Computing Facility
>> Argonne National Laboratory
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
More information about the llvm-commits
mailing list