Skip to content

[mlir][Interfaces][NFC] ValueBoundsConstraintSet: Add columns for constant values/dims #86097

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions mlir/include/mlir/Interfaces/ValueBoundsOpInterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -295,14 +295,19 @@ class ValueBoundsConstraintSet
/// value/dimension exists in the constraint set.
int64_t getPos(Value value, std::optional<int64_t> dim = std::nullopt) const;

/// Return an affine expression that represents column `pos` in the constraint
/// set.
AffineExpr getPosExpr(int64_t pos);

/// Insert a value/dimension into the constraint set. If `isSymbol` is set to
/// "false", a dimension is added. The value/dimension is added to the
/// worklist.
/// worklist if `addToWorklist` is set.
///
/// Note: There are certain affine restrictions wrt. dimensions. E.g., they
/// cannot be multiplied. Furthermore, bounds can only be queried for
/// dimensions but not for symbols.
int64_t insert(Value value, std::optional<int64_t> dim, bool isSymbol = true);
int64_t insert(Value value, std::optional<int64_t> dim, bool isSymbol = true,
bool addToWorklist = true);

/// Insert an anonymous column into the constraint set. The column is not
/// bound to any value/dimension. If `isSymbol` is set to "false", a dimension
Expand Down
64 changes: 49 additions & 15 deletions mlir/lib/Interfaces/ValueBoundsOpInterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,25 +110,47 @@ AffineExpr ValueBoundsConstraintSet::getExpr(Value value,
assertValidValueDim(value, dim);
#endif // NDEBUG

// Check if the value/dim is statically known. In that case, an affine
// constant expression should be returned. This allows us to support
// multiplications with constants. (Multiplications of two columns in the
// constraint set is not supported.)
std::optional<int64_t> constSize = std::nullopt;
auto shapedType = dyn_cast<ShapedType>(value.getType());
if (shapedType) {
// Static dimension: return constant directly.
if (shapedType.hasRank() && !shapedType.isDynamicDim(*dim))
return builder.getAffineConstantExpr(shapedType.getDimSize(*dim));
} else {
// Constant index value: return directly.
if (auto constInt = ::getConstantIntValue(value))
return builder.getAffineConstantExpr(*constInt);
constSize = shapedType.getDimSize(*dim);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I feel this is tricky: this really depends about the context in which we query the analysis.
It is easy to create a varying shape (i.e. size depends on loop iv) and this will not be a symbol anymore.

Basically, if the analysis was properly rooted at an op / block, we could say that everything defined above is a symbol.

IOW, any such analysis is relative to its context: if your transform/analysis cares about information on say k innermost loops, you can "make" information about the outermost loops constant.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't fully understand the different between symbols and dimensions in IntegerRelation. The limitations that I am aware of:

  • getSliceBounds can only be called on a dimensions, so the first "column" that I add to the constraint set (for which we want to compute LB/UB) is a dimension.
  • projectOut works on both symbols and dimensions, so all other values are added as symbols.

Basically we have only one dimension and everything else is a symbol. (But the current implementation would make it easy to change this.) This seems to work well so far.

Copy link
Member Author

@matthias-springer matthias-springer Mar 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any such analysis is relative to its context

On second thought, that's actually how it is implemented at the moment. We provide a ValueBoundsConstraintSet::computeBound(Value) API. That function will build a constraint set and add the given value as a dimension, everything else as a symbol. When computing a bound for a different value, we build a brand new constraint set, this time with the other value being the only dimension. (There is no public API that exposes the constraint set itself.) So this analysis is actually relative to a context and rooted at a certain op.

Computing multiple bounds back-to-back is slow at the moment because we have to re-analyze the entire IR. If we improve this at some point in the future, we will have to take a closer look at what's a symbol and what's a dimension, as you suggested.

} else if (auto constInt = ::getConstantIntValue(value)) {
constSize = *constInt;
}

// Dynamic value: add to constraint set.
// If the value/dim is already mapped, return the corresponding expression
// directly.
ValueDim valueDim = std::make_pair(value, dim.value_or(kIndexValue));
if (!valueDimToPosition.contains(valueDim))
(void)insert(value, dim);
int64_t pos = getPos(value, dim);
return pos < cstr.getNumDimVars()
? builder.getAffineDimExpr(pos)
: builder.getAffineSymbolExpr(pos - cstr.getNumDimVars());
if (valueDimToPosition.contains(valueDim)) {
// If it is a constant, return an affine constant expression. Otherwise,
// return an affine expression that represents the respective column in the
// constraint set.
if (constSize)
return builder.getAffineConstantExpr(*constSize);
return getPosExpr(getPos(value, dim));
}

if (constSize) {
// Constant index value/dim: add column to the constraint set, add EQ bound
// and return an affine constant expression without pushing the newly added
// column to the worklist.
(void)insert(value, dim, /*isSymbol=*/true, /*addToWorklist=*/false);
if (shapedType)
bound(value)[*dim] == *constSize;
else
bound(value) == *constSize;
return builder.getAffineConstantExpr(*constSize);
}

// Dynamic value/dim: insert column to the constraint set and put it on the
// worklist. Return an affine expression that represents the newly inserted
// column in the constraint set.
return getPosExpr(insert(value, dim, /*isSymbol=*/true));
}

AffineExpr ValueBoundsConstraintSet::getExpr(OpFoldResult ofr) {
Expand All @@ -145,7 +167,7 @@ AffineExpr ValueBoundsConstraintSet::getExpr(int64_t constant) {

int64_t ValueBoundsConstraintSet::insert(Value value,
std::optional<int64_t> dim,
bool isSymbol) {
bool isSymbol, bool addToWorklist) {
#ifndef NDEBUG
assertValidValueDim(value, dim);
#endif // NDEBUG
Expand All @@ -160,7 +182,12 @@ int64_t ValueBoundsConstraintSet::insert(Value value,
if (positionToValueDim[i].has_value())
valueDimToPosition[*positionToValueDim[i]] = i;

worklist.push(pos);
if (addToWorklist) {
LLVM_DEBUG(llvm::dbgs() << "Push to worklist: " << value
<< " (dim: " << dim.value_or(kIndexValue) << ")\n");
worklist.push(pos);
}

return pos;
}

Expand Down Expand Up @@ -190,6 +217,13 @@ int64_t ValueBoundsConstraintSet::getPos(Value value,
return it->second;
}

AffineExpr ValueBoundsConstraintSet::getPosExpr(int64_t pos) {
assert(pos >= 0 && pos < cstr.getNumDimAndSymbolVars() && "invalid position");
return pos < cstr.getNumDimVars()
? builder.getAffineDimExpr(pos)
: builder.getAffineSymbolExpr(pos - cstr.getNumDimVars());
}

static Operation *getOwnerOfValue(Value value) {
if (auto bbArg = dyn_cast<BlockArgument>(value))
return bbArg.getOwner()->getParentOp();
Expand Down