From 4554a1b426401a7d2dbefd48767ead408bb41426 Mon Sep 17 00:00:00 2001 From: Uday Bondhugula Date: Wed, 4 Dec 2019 09:29:51 -0800 Subject: [PATCH] NFC - fix name / comments - isAccessInvariant - the name was misleading; this is really checking if a Value being used to index was loop IV invariant. Update comment. - the method is only used locally; what can be exposed in the future is isAccessInvariant(LoadOrStoreOp op, Value *iv) Signed-off-by: Uday Bondhugula Closes #285 COPYBARA_INTEGRATE_REVIEW=https://github.com/tensorflow/mlir/pull/285 from bondhugula:quickfix fe5837abe987980c4ab469a9aa7de8e4f0007d9f PiperOrigin-RevId: 283771923 Change-Id: Ic0ac78b08ead73bc56cad843d9c535524a57a921 --- .../mlir/include/mlir/Analysis/LoopAnalysis.h | 17 --------------- .../mlir/lib/Analysis/LoopAnalysis.cpp | 21 ++++++++++++++++--- 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/third_party/mlir/include/mlir/Analysis/LoopAnalysis.h b/third_party/mlir/include/mlir/Analysis/LoopAnalysis.h index 8832c1469bc..140d9e91719 100644 --- a/third_party/mlir/include/mlir/Analysis/LoopAnalysis.h +++ b/third_party/mlir/include/mlir/Analysis/LoopAnalysis.h @@ -57,23 +57,6 @@ llvm::Optional getConstantTripCount(AffineForOp forOp); /// this method is thus able to determine non-trivial divisors. uint64_t getLargestDivisorOfTripCount(AffineForOp forOp); -/// Given an induction variable `iv` of type AffineForOp and an `index` of type -/// IndexType, returns `true` if `index` is independent of `iv` and false -/// otherwise. -/// The determination supports composition with at most one AffineApplyOp. -/// The at most one AffineApplyOp comes from the fact that composition of -/// AffineApplyOp need to be canonicalized by construction to avoid writing code -/// that composes arbitrary numbers of AffineApplyOps everywhere. To achieve -/// this, at the very least, the compose-affine-apply pass must have been run. -/// -/// Prerequisites: -/// 1. `iv` and `index` of the proper type; -/// 2. at most one reachable AffineApplyOp from index; -/// -/// Returns false in cases with more than one AffineApplyOp, this is -/// conservative. -bool isAccessInvariant(Value *iv, Value *index); - /// Given an induction variable `iv` of type AffineForOp and `indices` of type /// IndexType, returns the set of `indices` that are independent of `iv`. /// diff --git a/third_party/mlir/lib/Analysis/LoopAnalysis.cpp b/third_party/mlir/lib/Analysis/LoopAnalysis.cpp index b297a63cb62..1d88d09d269 100644 --- a/third_party/mlir/lib/Analysis/LoopAnalysis.cpp +++ b/third_party/mlir/lib/Analysis/LoopAnalysis.cpp @@ -158,7 +158,22 @@ uint64_t mlir::getLargestDivisorOfTripCount(AffineForOp forOp) { return gcd.getValue(); } -bool mlir::isAccessInvariant(Value *iv, Value *index) { +/// Given an induction variable `iv` of type AffineForOp and an access `index` +/// of type index, returns `true` if `index` is independent of `iv` and +/// false otherwise. The determination supports composition with at most one +/// AffineApplyOp. The 'at most one AffineApplyOp' comes from the fact that +/// the composition of AffineApplyOp needs to be canonicalized by construction +/// to avoid writing code that composes arbitrary numbers of AffineApplyOps +/// everywhere. To achieve this, at the very least, the compose-affine-apply +/// pass must have been run. +/// +/// Prerequisites: +/// 1. `iv` and `index` of the proper type; +/// 2. at most one reachable AffineApplyOp from index; +/// +/// Returns false in cases with more than one AffineApplyOp, this is +/// conservative. +static bool isAccessIndexInvariant(Value *iv, Value *index) { assert(isForInductionVar(iv) && "iv must be a AffineForOp"); assert(index->getType().isa() && "index must be of IndexType"); SmallVector affineApplyOps; @@ -187,7 +202,7 @@ mlir::getInvariantAccesses(Value *iv, llvm::ArrayRef indices) { llvm::DenseSet res; for (unsigned idx = 0, n = indices.size(); idx < n; ++idx) { auto *val = indices[idx]; - if (isAccessInvariant(iv, val)) { + if (isAccessIndexInvariant(iv, val)) { res.insert(val); } } @@ -249,7 +264,7 @@ static bool isContiguousAccess(Value *iv, LoadOrStoreOp memoryOp, }); // Check access invariance of each operand in 'exprOperands'. for (auto *exprOperand : exprOperands) { - if (!isAccessInvariant(iv, exprOperand)) { + if (!isAccessIndexInvariant(iv, exprOperand)) { if (uniqueVaryingIndexAlongIv != -1) { // 2+ varying indices -> do not vectorize along iv. return false;