[llvm] r185393 - Remove floating point computations form SpillPlacement.cpp.

Renato Golin renato.golin at linaro.org
Tue Jul 2 05:48:16 PDT 2013


Hi Jakob, Benjamin,

Just FYI, this commit broke Lencod on ARM again, though, if this is the
same problem as before, it seems that Manman and Stepan already have a
solution in mind.

I'm not sure the current problem is, again, due to spilling the wrong
offsets on the ByVal stack, but just to let you know, in case it doesn't,
we might have to investigate this commit further as well. ;)

cheers,
--renato


On 2 July 2013 00:19, Jakob Stoklund Olesen <stoklund at 2pi.dk> wrote:

> Author: stoklund
> Date: Mon Jul  1 18:19:39 2013
> New Revision: 185393
>
> URL: http://llvm.org/viewvc/llvm-project?rev=185393&view=rev
> Log:
> Remove floating point computations form SpillPlacement.cpp.
>
> Patch by Benjamin Kramer!
>
> Use the BlockFrequency class instead of floats in the Hopfield network
> computations. This rescales the node Bias field from a [-2;2] float
> range to two block frequencies BiasN and BiasP pulling in opposite
> directions. This construct has a more predictable behavior when block
> frequencies saturate.
>
> The per-node scaling factors are no longer necessary, assuming the block
> frequencies around a bundle are consistent.
>
> This patch can cause the register allocator to make different spilling
> decisions. The differences should be small.
>
> Modified:
>     llvm/trunk/include/llvm/Support/BlockFrequency.h
>     llvm/trunk/lib/CodeGen/SpillPlacement.cpp
>     llvm/trunk/lib/CodeGen/SpillPlacement.h
>
> Modified: llvm/trunk/include/llvm/Support/BlockFrequency.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/BlockFrequency.h?rev=185393&r1=185392&r2=185393&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/Support/BlockFrequency.h (original)
> +++ llvm/trunk/include/llvm/Support/BlockFrequency.h Mon Jul  1 18:19:39
> 2013
> @@ -36,6 +36,9 @@ public:
>    /// \brief Returns the frequency of the entry block of the function.
>    static uint64_t getEntryFrequency() { return ENTRY_FREQ; }
>
> +  /// \brief Returns the maximum possible prequency, the saturation value.
> +  static uint64_t getMaxFrequency() { return -1ULL; }
> +
>    /// \brief Returns the frequency as a fixpoint number scaled by the
> entry
>    /// frequency.
>    uint64_t getFrequency() const { return Frequency; }
>
> Modified: llvm/trunk/lib/CodeGen/SpillPlacement.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SpillPlacement.cpp?rev=185393&r1=185392&r2=185393&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SpillPlacement.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SpillPlacement.cpp Mon Jul  1 18:19:39 2013
> @@ -59,6 +59,10 @@ void SpillPlacement::getAnalysisUsage(An
>    MachineFunctionPass::getAnalysisUsage(AU);
>  }
>
> +/// Decision threshold. A node gets the output value 0 if the weighted
> sum of
> +/// its inputs falls in the open interval (-Threshold;Threshold).
> +static const BlockFrequency Threshold = 2;
> +
>  /// Node - Each edge bundle corresponds to a Hopfield node.
>  ///
>  /// The node contains precomputed frequency data that only depends on the
> CFG,
> @@ -69,23 +73,17 @@ void SpillPlacement::getAnalysisUsage(An
>  /// because all weights are positive.
>  ///
>  struct SpillPlacement::Node {
> -  /// Scale - Inverse block frequency feeding into[0] or out of[1] the
> bundle.
> -  /// Ideally, these two numbers should be identical, but inaccuracies in
> the
> -  /// block frequency estimates means that we need to normalize ingoing
> and
> -  /// outgoing frequencies separately so they are commensurate.
> -  float Scale[2];
> -
> -  /// Bias - Normalized contributions from non-transparent blocks.
> -  /// A bundle connected to a MustSpill block has a huge negative bias,
> -  /// otherwise it is a number in the range [-2;2].
> -  float Bias;
> +  /// BiasN - Sum of blocks that prefer a spill.
> +  BlockFrequency BiasN;
> +  /// BiasP - Sum of blocks that prefer a register.
> +  BlockFrequency BiasP;
>
>    /// Value - Output value of this node computed from the Bias and links.
>    /// This is always in the range [-1;1]. A positive number means the
> variable
>    /// should go in a register through this bundle.
> -  float Value;
> +  int Value;
>
> -  typedef SmallVector<std::pair<float, unsigned>, 4> LinkVector;
> +  typedef SmallVector<std::pair<BlockFrequency, unsigned>, 4> LinkVector;
>
>    /// Links - (Weight, BundleNo) for all transparent blocks connecting to
> other
>    /// bundles. The weights are all positive and add up to at most 2,
> weights
> @@ -94,6 +92,9 @@ struct SpillPlacement::Node {
>    /// connected basic blocks.
>    LinkVector Links;
>
> +  /// SumLinkWeights - Cached sum of the weights of all links + ThresHold.
> +  BlockFrequency SumLinkWeights;
> +
>    /// preferReg - Return true when this node prefers to be in a register.
>    bool preferReg() const {
>      // Undecided nodes (Value==0) go on the stack.
> @@ -102,28 +103,24 @@ struct SpillPlacement::Node {
>
>    /// mustSpill - Return True if this node is so biased that it must
> spill.
>    bool mustSpill() const {
> -    // Actually, we must spill if Bias < sum(weights).
> -    // It may be worth it to compute the weight sum here?
> -    return Bias < -2.0f;
> -  }
> -
> -  /// Node - Create a blank Node.
> -  Node() {
> -    Scale[0] = Scale[1] = 0;
> +    // We must spill if Bias < -sum(weights) or the MustSpill flag was
> set.
> +    // BiasN is saturated when MustSpill is set, make sure this still
> returns
> +    // true when the RHS saturates. Note that SumLinkWeights includes
> Threshold.
> +    return BiasN >= BiasP + SumLinkWeights;
>    }
>
>    /// clear - Reset per-query data, but preserve frequencies that only
> depend on
>    // the CFG.
>    void clear() {
> -    Bias = Value = 0;
> +    BiasN = BiasP = Value = 0;
> +    SumLinkWeights = Threshold;
>      Links.clear();
>    }
>
>    /// addLink - Add a link to bundle b with weight w.
> -  /// out=0 for an ingoing link, and 1 for an outgoing link.
> -  void addLink(unsigned b, float w, bool out) {
> -    // Normalize w relative to all connected blocks from that direction.
> -    w *= Scale[out];
> +  void addLink(unsigned b, BlockFrequency w) {
> +    // Update cached sum.
> +    SumLinkWeights += w;
>
>      // There can be multiple links to the same bundle, add them up.
>      for (LinkVector::iterator I = Links.begin(), E = Links.end(); I != E;
> ++I)
> @@ -135,21 +132,35 @@ struct SpillPlacement::Node {
>      Links.push_back(std::make_pair(w, b));
>    }
>
> -  /// addBias - Bias this node from an ingoing[0] or outgoing[1] link.
> -  /// Return the change to the total number of positive biases.
> -  void addBias(float w, bool out) {
> -    // Normalize w relative to all connected blocks from that direction.
> -    w *= Scale[out];
> -    Bias += w;
> +  /// addBias - Bias this node.
> +  void addBias(BlockFrequency freq, BorderConstraint direction) {
> +    switch (direction) {
> +    default:
> +      break;
> +    case PrefReg:
> +      BiasP += freq;
> +      break;
> +    case PrefSpill:
> +      BiasN += freq;
> +      break;
> +    case MustSpill:
> +      BiasN = BlockFrequency::getMaxFrequency();
> +      break;
> +    }
>    }
>
>    /// update - Recompute Value from Bias and Links. Return true when node
>    /// preference changes.
>    bool update(const Node nodes[]) {
>      // Compute the weighted sum of inputs.
> -    float Sum = Bias;
> -    for (LinkVector::iterator I = Links.begin(), E = Links.end(); I != E;
> ++I)
> -      Sum += I->first * nodes[I->second].Value;
> +    BlockFrequency SumN = BiasN;
> +    BlockFrequency SumP = BiasP;
> +    for (LinkVector::iterator I = Links.begin(), E = Links.end(); I != E;
> ++I) {
> +      if (nodes[I->second].Value == -1)
> +        SumN += I->first;
> +      else if (nodes[I->second].Value == 1)
> +        SumP += I->first;
> +    }
>
>      // The weighted sum is going to be in the range [-2;2]. Ideally, we
> should
>      // simply set Value = sign(Sum), but we will add a dead zone around 0
> for
> @@ -157,11 +168,10 @@ struct SpillPlacement::Node {
>      //  1. It avoids arbitrary bias when all links are 0 as is possible
> during
>      //     initial iterations.
>      //  2. It helps tame rounding errors when the links nominally sum to
> 0.
> -    const float Thres = 1e-4f;
>      bool Before = preferReg();
> -    if (Sum < -Thres)
> +    if (SumN >= SumP + Threshold)
>        Value = -1;
> -    else if (Sum > Thres)
> +    else if (SumP >= SumN + Threshold)
>        Value = 1;
>      else
>        Value = 0;
> @@ -178,23 +188,13 @@ bool SpillPlacement::runOnMachineFunctio
>    nodes = new Node[bundles->getNumBundles()];
>
>    // Compute total ingoing and outgoing block frequencies for all bundles.
> -  BlockFrequency.resize(mf.getNumBlockIDs());
> +  BlockFrequencies.resize(mf.getNumBlockIDs());
>    MachineBlockFrequencyInfo &MBFI =
> getAnalysis<MachineBlockFrequencyInfo>();
> -  float EntryFreq = BlockFrequency::getEntryFrequency();
>    for (MachineFunction::iterator I = mf.begin(), E = mf.end(); I != E;
> ++I) {
> -    float Freq = MBFI.getBlockFreq(I).getFrequency() / EntryFreq;
>      unsigned Num = I->getNumber();
> -    BlockFrequency[Num] = Freq;
> -    nodes[bundles->getBundle(Num, 1)].Scale[0] += Freq;
> -    nodes[bundles->getBundle(Num, 0)].Scale[1] += Freq;
> +    BlockFrequencies[Num] = MBFI.getBlockFreq(I);
>    }
>
> -  // Scales are reciprocal frequencies.
> -  for (unsigned i = 0, e = bundles->getNumBundles(); i != e; ++i)
> -    for (unsigned d = 0; d != 2; ++d)
> -      if (nodes[i].Scale[d] > 0)
> -        nodes[i].Scale[d] = 1 / nodes[i].Scale[d];
> -
>    // We never change the function.
>    return false;
>  }
> @@ -219,8 +219,10 @@ void SpillPlacement::activate(unsigned n
>    // connected blocks need to be interested before we consider expanding
> the
>    // region through the bundle. This helps compile time by limiting the
> number
>    // of blocks visited and the number of links in the Hopfield network.
> -  if (bundles->getBlocks(n).size() > 100)
> -    nodes[n].Bias = -0.0625f;
> +  if (bundles->getBlocks(n).size() > 100) {
> +    nodes[n].BiasP = 0;
> +    nodes[n].BiasN = (BlockFrequency::getEntryFrequency() / 16);
> +  }
>  }
>
>
> @@ -229,27 +231,20 @@ void SpillPlacement::activate(unsigned n
>  void SpillPlacement::addConstraints(ArrayRef<BlockConstraint> LiveBlocks)
> {
>    for (ArrayRef<BlockConstraint>::iterator I = LiveBlocks.begin(),
>         E = LiveBlocks.end(); I != E; ++I) {
> -    float Freq = getBlockFrequency(I->Number);
> -    const float Bias[] = {
> -      0,           // DontCare,
> -      1,           // PrefReg,
> -      -1,          // PrefSpill
> -      0,           // PrefBoth
> -      -HUGE_VALF   // MustSpill
> -    };
> +    BlockFrequency Freq = BlockFrequencies[I->Number];
>
>      // Live-in to block?
>      if (I->Entry != DontCare) {
>        unsigned ib = bundles->getBundle(I->Number, 0);
>        activate(ib);
> -      nodes[ib].addBias(Freq * Bias[I->Entry], 1);
> +      nodes[ib].addBias(Freq, I->Entry);
>      }
>
>      // Live-out from block?
>      if (I->Exit != DontCare) {
>        unsigned ob = bundles->getBundle(I->Number, 1);
>        activate(ob);
> -      nodes[ob].addBias(Freq * Bias[I->Exit], 0);
> +      nodes[ob].addBias(Freq, I->Exit);
>      }
>    }
>  }
> @@ -258,15 +253,15 @@ void SpillPlacement::addConstraints(Arra
>  void SpillPlacement::addPrefSpill(ArrayRef<unsigned> Blocks, bool Strong)
> {
>    for (ArrayRef<unsigned>::iterator I = Blocks.begin(), E = Blocks.end();
>         I != E; ++I) {
> -    float Freq = getBlockFrequency(*I);
> +    BlockFrequency Freq = BlockFrequencies[*I];
>      if (Strong)
>        Freq += Freq;
>      unsigned ib = bundles->getBundle(*I, 0);
>      unsigned ob = bundles->getBundle(*I, 1);
>      activate(ib);
>      activate(ob);
> -    nodes[ib].addBias(-Freq, 1);
> -    nodes[ob].addBias(-Freq, 0);
> +    nodes[ib].addBias(Freq, PrefSpill);
> +    nodes[ob].addBias(Freq, PrefSpill);
>    }
>  }
>
> @@ -286,9 +281,9 @@ void SpillPlacement::addLinks(ArrayRef<u
>        Linked.push_back(ib);
>      if (nodes[ob].Links.empty() && !nodes[ob].mustSpill())
>        Linked.push_back(ob);
> -    float Freq = getBlockFrequency(Number);
> -    nodes[ib].addLink(ob, Freq, 1);
> -    nodes[ob].addLink(ib, Freq, 0);
> +    BlockFrequency Freq = BlockFrequencies[Number];
> +    nodes[ib].addLink(ob, Freq);
> +    nodes[ob].addLink(ib, Freq);
>    }
>  }
>
>
> Modified: llvm/trunk/lib/CodeGen/SpillPlacement.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SpillPlacement.h?rev=185393&r1=185392&r2=185393&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SpillPlacement.h (original)
> +++ llvm/trunk/lib/CodeGen/SpillPlacement.h Mon Jul  1 18:19:39 2013
> @@ -30,6 +30,7 @@
>  #include "llvm/ADT/ArrayRef.h"
>  #include "llvm/ADT/SmallVector.h"
>  #include "llvm/CodeGen/MachineFunctionPass.h"
> +#include "llvm/Support/BlockFrequency.h"
>
>  namespace llvm {
>
> @@ -57,7 +58,7 @@ class SpillPlacement  : public MachineFu
>    SmallVector<unsigned, 8> RecentPositive;
>
>    // Block frequencies are computed once. Indexed by block number.
> -  SmallVector<float, 4> BlockFrequency;
> +  SmallVector<BlockFrequency, 4> BlockFrequencies;
>
>  public:
>    static char ID; // Pass identification, replacement for typeid.
> @@ -140,7 +141,9 @@ public:
>    /// getBlockFrequency - Return the estimated block execution frequency
> per
>    /// function invocation.
>    float getBlockFrequency(unsigned Number) const {
> -    return BlockFrequency[Number];
> +    // FIXME: Return the BlockFrequency directly.
> +    const float Scale = 1.0f / BlockFrequency::getEntryFrequency();
> +    return BlockFrequencies[Number].getFrequency() * Scale;
>    }
>
>  private:
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130702/5479ad2c/attachment.html>


More information about the llvm-commits mailing list