[llvm] r244154 - Record whether the weights on out-edges from a MBB are normalized.

Xinliang David Li via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 6 10:33:04 PDT 2015


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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150806/e923bb8f/attachment.html>


More information about the llvm-commits mailing list