[llvm-commits] [llvm] r149468 - in /llvm/trunk: docs/ include/llvm-c/ include/llvm-c/Transforms/ include/llvm/ include/llvm/Transforms/ include/llvm/Transforms/IPO/ lib/Transforms/ lib/Transforms/IPO/ lib/Transforms/Vectorize/ test/Transforms/BBV

[Hal Finkel]

On Thu, 2012-02-09 at 00:17 -0600, Sebastian Pop wrote:
> On Tue, Jan 31, 2012 at 9:51 PM, Hal Finkel <hfinkel at anl.gov> wrote:
> > +namespace {
> > +  struct BBVectorize : public BasicBlockPass {
> > +    static char ID; // Pass identification, replacement for typeid
> > +    BBVectorize() : BasicBlockPass(ID) {
> > +      initializeBBVectorizePass(*PassRegistry::getPassRegistry());
> > +    }
> > +
> > +    typedef std::pair<Value *, Value *> ValuePair;
> > +    typedef std::pair<ValuePair, size_t> ValuePairWithDepth;
> > +    typedef std::pair<ValuePair, ValuePair> VPPair; // A ValuePair pair
> > +    typedef std::pair<std::multimap<Value *, Value *>::iterator,
> > +              std::multimap<Value *, Value *>::iterator> VPIteratorPair;
> > +    typedef std::pair<std::multimap<ValuePair, ValuePair>::iterator,
> > +              std::multimap<ValuePair, ValuePair>::iterator>
> > +                VPPIteratorPair;
> > +
> > +    AliasAnalysis *AA;
> > +    ScalarEvolution *SE;
> > +    TargetData *TD;
> > +
> > +    // FIXME: const correct?
> > +
> > +    bool vectorizePairs(BasicBlock &BB);
> > +
> > +    void getCandidatePairs(BasicBlock &BB,
> > +                       std::multimap<Value *, Value *> &CandidatePairs,
> > +                       std::vector<Value *> &PairableInsts);
> > +
> > +    void computeConnectedPairs(std::multimap<Value *, Value *> &CandidatePairs,
> > +                       std::vector<Value *> &PairableInsts,
> > +                       std::multimap<ValuePair, ValuePair> &ConnectedPairs);
> > +
> > +    void buildDepMap(BasicBlock &BB,
> > +                       std::multimap<Value *, Value *> &CandidatePairs,
> > +                       std::vector<Value *> &PairableInsts,
> > +                       DenseSet<ValuePair> &PairableInstUsers);
> > +
> > +    void choosePairs(std::multimap<Value *, Value *> &CandidatePairs,
> > +                        std::vector<Value *> &PairableInsts,
> > +                        std::multimap<ValuePair, ValuePair> &ConnectedPairs,
> > +                        DenseSet<ValuePair> &PairableInstUsers,
> > +                        DenseMap<Value *, Value *>& ChosenPairs);
> > +
> > +    void fuseChosenPairs(BasicBlock &BB,
> > +                     std::vector<Value *> &PairableInsts,
> > +                     DenseMap<Value *, Value *>& ChosenPairs);
> > +
> > +    bool isInstVectorizable(Instruction *I, bool &IsSimpleLoadStore);
> > +
> > +    bool areInstsCompatible(Instruction *I, Instruction *J,
> > +                       bool IsSimpleLoadStore);
> > +
> > +    bool trackUsesOfI(DenseSet<Value *> &Users,
> > +                      AliasSetTracker &WriteSet, Instruction *I,
> > +                      Instruction *J, bool UpdateUsers = true,
> > +                      std::multimap<Value *, Value *> *LoadMoveSet = 0);
> > +
> > +    void computePairsConnectedTo(
> > +                      std::multimap<Value *, Value *> &CandidatePairs,
> > +                      std::vector<Value *> &PairableInsts,
> > +                      std::multimap<ValuePair, ValuePair> &ConnectedPairs,
> > +                      ValuePair P);
> > +
> > +    bool pairsConflict(ValuePair P, ValuePair Q,
> > +                 DenseSet<ValuePair> &PairableInstUsers,
> > +                 std::multimap<ValuePair, ValuePair> *PairableInstUserMap = 0);
> > +
> > +    bool pairWillFormCycle(ValuePair P,
> > +                       std::multimap<ValuePair, ValuePair> &PairableInstUsers,
> > +                       DenseSet<ValuePair> &CurrentPairs);
> > +
> > +    void pruneTreeFor(
> > +                      std::multimap<Value *, Value *> &CandidatePairs,
> > +                      std::vector<Value *> &PairableInsts,
> > +                      std::multimap<ValuePair, ValuePair> &ConnectedPairs,
> > +                      DenseSet<ValuePair> &PairableInstUsers,
> > +                      std::multimap<ValuePair, ValuePair> &PairableInstUserMap,
> > +                      DenseMap<Value *, Value *> &ChosenPairs,
> > +                      DenseMap<ValuePair, size_t> &Tree,
> > +                      DenseSet<ValuePair> &PrunedTree, ValuePair J,
> > +                      bool UseCycleCheck);
> > +
> > +    void buildInitialTreeFor(
> > +                      std::multimap<Value *, Value *> &CandidatePairs,
> > +                      std::vector<Value *> &PairableInsts,
> > +                      std::multimap<ValuePair, ValuePair> &ConnectedPairs,
> > +                      DenseSet<ValuePair> &PairableInstUsers,
> > +                      DenseMap<Value *, Value *> &ChosenPairs,
> > +                      DenseMap<ValuePair, size_t> &Tree, ValuePair J);
> > +
> > +    void findBestTreeFor(
> > +                      std::multimap<Value *, Value *> &CandidatePairs,
> > +                      std::vector<Value *> &PairableInsts,
> > +                      std::multimap<ValuePair, ValuePair> &ConnectedPairs,
> > +                      DenseSet<ValuePair> &PairableInstUsers,
> > +                      std::multimap<ValuePair, ValuePair> &PairableInstUserMap,
> > +                      DenseMap<Value *, Value *> &ChosenPairs,
> > +                      DenseSet<ValuePair> &BestTree, size_t &BestMaxDepth,
> > +                      size_t &BestEffSize, VPIteratorPair ChoiceRange,
> > +                      bool UseCycleCheck);
> 
> Having so many parameters to this and other methods above seems wrong:
> you could try to refactor this by better structuring the data.  I
> could see at least the pair of vectorizable instructions and the tree
> of pairs split out as separate classes.  You can also add some of
> these parameters to BBVectorize to avoid to pass them from method to
> method.

You are absolutely right, this needs to be refactored. I will work on
this.

Thanks again,
Hal

> 
> Sebastian
> --
> Qualcomm Innovation Center, Inc is a member of Code Aurora Forum

-- 
Hal Finkel
Postdoctoral Appointee
Leadership Computing Facility
Argonne National Laboratory