[PATCH] D136277: [LoopInterchange] Simplify DepMatrix to a dependency vector.

[Bardia Mahjour via Phabricator via llvm-commits]

bmahjour requested changes to this revision.
bmahjour added a comment.
This revision now requires changes to proceed.

This would collapse two dimensions of the dependency matrix into one without providing a way to recover the individual dependency vectors, and as you pointed out in the description it could make some interchangeable cases look illegal. Another example is if we have [S, >, = ] and [S, <, =], we can safely interchange the two inner loops (by factoring out the S from the beginning of both vectors), but after merging them we get [S, <>, = ] and now the inner two loops can no longer be interchanged.

Is the size of dependency matrix proving to be problematic for certain workloads? If so, maybe looking at some concrete examples can help solve the problem in a more effective way (eg. considering sparse data structures or  some sort of compression/decompression techniques).

On a different note, I think we can make the code more elegant and robust by encapsulating all the dependency matrix analysis inside a class. For example:

  class DepMatrix {
  public:
    /// Enumerates the possible kind of dependency in a dependence vector.
    enum class DepKind : char {
      NEG, 
      EQ,  
      POS,
      S,
      STAR
    };
  
    /// Represents a row of the dependency matrix.
    using DepVector = std::vector<DepKind>;
    ...
  
    /// Update the dependence matrix by exchanging two columns.
    void interchangeColumns(const Loop &A, const Loop &B);
    
    bool isPositive(int row);
    ...
  private:
    /// The dependency matrix is represented by a vector of dependency vectors.
    std::vector<DepVector> DM;
  };



================
Comment at: llvm/lib/Transforms/Scalar/LoopInterchange.cpp:84
 // Maximum number of dependencies that can be handled in the dependency matrix.
-static const unsigned MaxMemInstrCount = 100;
+static const unsigned MaxMemInstrCount = 10;
 
----------------
why does this need to be reduced so much?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136277/new/

https://reviews.llvm.org/D136277