Skip to content

[RFC][mlir] ViewLikeOpInterface method for detecting partial views. #164020

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[RFC][mlir] ViewLikeOpInterface method for detecting partial views. #164020

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1826c30
[RFC][mlir] ViewLikeOpInterface method for detecting partial views.
vzakhari Oct 29, 2025
afed03e
Renamed the methods to clearly indicate what their result means.
vzakhari Oct 30, 2025
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
1 change: 1 addition & 0 deletions flang/include/flang/Optimizer/Dialect/FIROps.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "mlir/Dialect/LLVMIR/LLVMAttrs.h"
#include "mlir/Interfaces/LoopLikeInterface.h"
#include "mlir/Interfaces/SideEffectInterfaces.h"
#include "mlir/Interfaces/ViewLikeInterface.h"

namespace fir {

Expand Down
19 changes: 15 additions & 4 deletions flang/include/flang/Optimizer/Dialect/FIROps.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
include "mlir/Dialect/Arith/IR/ArithBase.td"
include "mlir/Dialect/Arith/IR/ArithOpsInterfaces.td"
include "mlir/Dialect/LLVMIR/LLVMAttrDefs.td"
include "mlir/Interfaces/ViewLikeInterface.td"
include "flang/Optimizer/Dialect/CUF/Attributes/CUFAttr.td"
include "flang/Optimizer/Dialect/FIRDialect.td"
include "flang/Optimizer/Dialect/FIRTypes.td"
Expand Down Expand Up @@ -1765,8 +1766,9 @@ def fir_ArrayMergeStoreOp : fir_Op<"array_merge_store",
// Record and array type operations
//===----------------------------------------------------------------------===//

def fir_ArrayCoorOp : fir_Op<"array_coor",
[NoMemoryEffect, AttrSizedOperandSegments]> {
def fir_ArrayCoorOp
: fir_Op<"array_coor", [NoMemoryEffect, AttrSizedOperandSegments,
ViewLikeOpInterface]> {

let summary = "Find the coordinate of an element of an array";

Expand Down Expand Up @@ -1809,9 +1811,13 @@ def fir_ArrayCoorOp : fir_Op<"array_coor",

let hasVerifier = 1;
let hasCanonicalizer = 1;
let extraClassDeclaration = [{
mlir::Value getViewSource() { return getMemref(); }
}];
}

def fir_CoordinateOp : fir_Op<"coordinate_of", [NoMemoryEffect]> {
def fir_CoordinateOp
: fir_Op<"coordinate_of", [NoMemoryEffect, ViewLikeOpInterface]> {

let summary = "Finds the coordinate (location) of a value in memory";

Expand Down Expand Up @@ -1863,6 +1869,7 @@ def fir_CoordinateOp : fir_Op<"coordinate_of", [NoMemoryEffect]> {
let extraClassDeclaration = [{
constexpr static int32_t kDynamicIndex = std::numeric_limits<int32_t>::min();
CoordinateIndicesAdaptor getIndices();
mlir::Value getViewSource() { return getRef(); }
}];
}

Expand Down Expand Up @@ -2828,7 +2835,8 @@ def fir_VolatileCastOp : fir_SimpleOneResultOp<"volatile_cast", [Pure]> {
let hasFolder = 1;
}

def fir_ConvertOp : fir_SimpleOneResultOp<"convert", [NoMemoryEffect]> {
def fir_ConvertOp
: fir_SimpleOneResultOp<"convert", [NoMemoryEffect, ViewLikeOpInterface]> {
let summary = "encapsulates all Fortran entity type conversions";

let description = [{
Expand Down Expand Up @@ -2866,6 +2874,9 @@ def fir_ConvertOp : fir_SimpleOneResultOp<"convert", [NoMemoryEffect]> {
static bool isPointerCompatible(mlir::Type ty);
static bool canBeConverted(mlir::Type inType, mlir::Type outType);
static bool areVectorsCompatible(mlir::Type inTy, mlir::Type outTy);
mlir::Value getViewSource() { return getValue(); }
bool isKnownToHaveSameStart() { return true; }
bool isKnownToBeCompleteView() { return true; }
}];
let hasCanonicalizer = 1;
}
Expand Down
28 changes: 16 additions & 12 deletions flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,10 @@ AliasResult AliasAnalysis::alias(Source lhsSrc, Source rhsSrc, mlir::Value lhs,
if (lhsSrc.origin == rhsSrc.origin) {
LLVM_DEBUG(llvm::dbgs()
<< " aliasing because same source kind and origin\n");
// TODO: we should return PartialAlias here. Need to decide
// if returning PartialAlias for fir.pack_array is okay;
// if not, then we need to handle it specially to still return
// MayAlias.
Comment on lines +215 to +218
Copy link
Contributor

@tblah tblah Oct 23, 2025

Choose a reason for hiding this comment

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

From the LLVM documentation:

The PartialAlias response is used when the two memory objects are known to be overlapping in some way, regardless of whether they start at the same address or not.

I guess it depends what exactly is meant by "object". Previously I had interpreted it as referring to the specific memory addresses which could be accessed. So array(1:4) and array(3) do overlap with each other but not with array(100:200). Therefore I think array indexing (until analysed more carefully) should use MayAlias rather than PartialAlias.

Alternatively, if "object" means the entire allocation then PartialAlias makes sense here.

Copy link
Contributor Author

@vzakhari vzakhari Oct 23, 2025

Choose a reason for hiding this comment

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

Thanks for looking, Tom!

I believe in LLVM alias analysis an "object" is defined at a high level as a starting address and the size that are passed to the alias API. So, indeed, the accesses array(1:4) and array(3) do partially overlap, and they do not alias with array(100:200). Now, from the implementation point of view, an alias query may be made with an "infinite" size (up to the end of the underlying memory object). In this case, array(1:4), array(3) and array(100:200) all partially overlap.

When the starting addresses of the two accesses are not compilation constants and cannot be proven to be the same/different, then they may be in both MustAlias and PartialAlias states, so returning MayAlias seems reasonable.

The current implementation of MLIR alias analysis does not accept the access size and I assume that it is "infinite", so returning PartialAlias for such accesses seems conservatively correct to me.

At the same time, maybe I should drop my changes to return PartialAlias in LocalAliasAnalysis and always return MayAlias to allow further alias analyses to make up a more precise answer.

Copy link
Contributor

@tblah tblah Oct 24, 2025

Choose a reason for hiding this comment

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

Thinking about it some more, I think local alias analysis should at least return MayAlias. As I understand it, the distinction between MayAlias and PartialAlias is that a MayAlias result invites further analysis by other methods whereas PartialAlias is considered a definate "these accesses are proven to overlap" and so does not need further analysis if other alias analysis implementations are available.

Copy link
Contributor Author

@vzakhari vzakhari Oct 28, 2025

Choose a reason for hiding this comment

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

I agree with you, Tom. I think the current algorithm in LocalAliasAnalysis (which I am not trying to affect too much, because this is not my goal) does not allow reasoning about partial aliases quite precisely. So I feel okay returning MayAlias in cases where there might be an offset involved. At least, this makes the analysis conservatively correct.

if (approximateSource)
return AliasResult::MayAlias;
return AliasResult::MustAlias;
Expand Down Expand Up @@ -559,10 +563,19 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v,
type = SourceKind::Allocate;
breakFromLoop = true;
})
.Case<fir::ConvertOp>([&](auto op) {
// Skip ConvertOp's and track further through the operand.
v = op->getOperand(0);
.Case<mlir::ViewLikeOpInterface>([&](auto op) {
Copy link
Member

@matthias-springer matthias-springer Nov 1, 2025

Choose a reason for hiding this comment

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

Why does Flang have its own alias analysis? How is it different from the MLIR alias analysis? Given that you are improving this interface, could the Flang alias analysis be replaced with the one from MLIR?

Copy link
Contributor Author

@vzakhari vzakhari Nov 3, 2025

Choose a reason for hiding this comment

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

Flang alias analysis has special handling for HLFIR/FIR operations that carry extra information about the rules of aliasing for objects created from Fortran variables. For example, there are special rules for DUMMY arguments of functions that cannot alias each other unless they have TARGET or POINTER attributes. Flang carries this extra information on [hl]fir.declare operations. For the two given "addresses", Flang alias analysis tries to discover the "sources" of those addresses, e.g. by tracking through view-like operations, and then reasons about the aliasing having reached [hl]fir.declare operations.

I think the HLFIR/FIR specifics used by Flang alias analysis can be abstracted by using some generic interfaces that [hl]fir.declare (and some other operations, e.g. fir.global), but I want to leave this out of scope of this PR.

if (isPointerReference(ty))
attributes.set(Attribute::Pointer);
v = op.getViewSource();
defOp = v.getDefiningOp();
// If the source is a box, and the result is not a box,
// then this is one of the box "unpacking" operations,
// so we should set followingData.
if (mlir::isa<fir::BaseBoxType>(v.getType()) &&
!mlir::isa<fir::BaseBoxType>(ty))
followBoxData = true;
if (!op.isKnownToHaveSameStart())
approximateSource = true;
})
.Case<fir::PackArrayOp>([&](auto op) {
// The packed array is not distinguishable from the original
Expand All @@ -578,15 +591,6 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v,
if (mlir::isa<fir::BaseBoxType>(v.getType()))
followBoxData = true;
})
.Case<fir::ArrayCoorOp, fir::CoordinateOp>([&](auto op) {
if (isPointerReference(ty))
attributes.set(Attribute::Pointer);
v = op->getOperand(0);
defOp = v.getDefiningOp();
if (mlir::isa<fir::BaseBoxType>(v.getType()))
followBoxData = true;
approximateSource = true;
})
.Case<fir::EmboxOp, fir::ReboxOp>([&](auto op) {
if (followBoxData) {
v = op->getOperand(0);
Expand Down
7 changes: 5 additions & 2 deletions mlir/include/mlir/Analysis/AliasAnalysis.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,12 @@ class AliasResult {
/// The two locations may or may not alias. This is the least precise
/// result.
MayAlias,
/// The two locations alias, but only due to a partial overlap.
/// The two locations overlap in some way, regardless of whether
/// they start at the same address or not.
Copy link
Member

@matthias-springer matthias-springer Nov 1, 2025

Choose a reason for hiding this comment

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

This seems to change the semantics of PartialAlias. Is it safe?

Copy link
Contributor Author

@vzakhari vzakhari Nov 3, 2025

Choose a reason for hiding this comment

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

I think the new comment is a clarification of the old one (The two locations alias, but only due to a partial overlap.), which may be interpreted differently by the readers.

Yes, I decided to reuse the LLVM definition here, which seems more verbose to me.

I believe there are no current check for PartialAlias or isPartial in MLIR sources, so the change should be safe in a sense that it should not break any existing code. The change seems good to me also because it is more precise.

PartialAlias,
/// The two locations precisely alias each other.
/// The two locations precisely alias each other, meaning that
/// they always start at exactly the same location.
/// This result does not imply that the pointers compare equal.
MustAlias,
Copy link
Member

@matthias-springer matthias-springer Nov 1, 2025

Choose a reason for hiding this comment

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

I think it's worth mentioning as a comment of the enum that these values are taken from LLVM.

Copy link
Contributor Author

@vzakhari vzakhari Nov 3, 2025

Choose a reason for hiding this comment

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

Thanks! I will add a comment.

};

Expand Down
10 changes: 9 additions & 1 deletion mlir/include/mlir/Dialect/MemRef/IR/MemRefOps.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2288,7 +2288,7 @@ def SubViewOp : MemRef_OpWithOffsetSizesAndStrides<"subview", [
CArg<"ArrayRef<NamedAttribute>", "{}">:$attrs)>
];

let extraClassDeclaration = extraBaseClassDeclaration # [{
let extraClassDeclaration = extraBaseClassDeclaration#[{
/// Returns the type of the base memref operand.
MemRefType getSourceType() {
return ::llvm::cast<MemRefType>(getSource().getType());
Expand Down Expand Up @@ -2350,6 +2350,10 @@ def SubViewOp : MemRef_OpWithOffsetSizesAndStrides<"subview", [
/// If the shape of `value` cannot be rank-reduced to `desiredShape`, fail.
static FailureOr<Value> rankReduceIfNeeded(
OpBuilder &b, Location loc, Value value, ArrayRef<int64_t> desiredShape);

/// Return true iff the input and the resulting views
/// are known to start at the same address.
bool isKnownToHaveSameStart();
}];

let hasCanonicalizer = 1;
Expand Down Expand Up @@ -2459,6 +2463,10 @@ def MemRef_ViewOp : MemRef_Op<"view", [
operand_range getDynamicSizes() {
return {getSizes().begin(), getSizes().end()};
}

/// Return true iff the input and the resulting views
/// are known to start at the same address.
bool isKnownToHaveSameStart();
}];

let assemblyFormat = [{
Expand Down
60 changes: 46 additions & 14 deletions mlir/include/mlir/Interfaces/ViewLikeInterface.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,21 +23,53 @@ def ViewLikeOpInterface : OpInterface<"ViewLikeOpInterface"> {
}];
let cppNamespace = "::mlir";

let methods = [
InterfaceMethod<
"Returns the source buffer from which the view is created.",
"::mlir::Value", "getViewSource">,
InterfaceMethod<
/*desc=*/[{ Returns the buffer which the view created. }],
/*retTy=*/"::mlir::Value",
/*methodName=*/"getViewDest",
/*args=*/(ins),
/*methodBody=*/"",
/*defaultImplementation=*/[{
let methods =
[InterfaceMethod<
"Returns the source buffer from which the view is created.",
"::mlir::Value", "getViewSource">,
InterfaceMethod<
/*desc=*/[{ Returns the buffer which the view created. }],
/*retTy=*/"::mlir::Value",
/*methodName=*/"getViewDest",
/*args=*/(ins),
/*methodBody=*/"",
/*defaultImplementation=*/[{
return $_op->getResult(0);
}]
>
];
}]>,
InterfaceMethod<
/*desc=*/
[{
Returns true iff the source buffer and the resulting view
are known to start at the same "address".
}],
/*retTy=*/"bool",
/*methodName=*/"isKnownToHaveSameStart",
Copy link
Member

@matthias-springer matthias-springer Nov 1, 2025

Choose a reason for hiding this comment

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

Can isKnownToHaveSameStart and isKnownToBeCompleteView be replaced by a single interface method that returns AliasResult::Kind?

Copy link
Contributor Author

@vzakhari vzakhari Nov 3, 2025

Choose a reason for hiding this comment

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

Yes, I think adding a method like alias will allow me to make the same changes in the alias analyses.

At the same time, I think the new alias method may become redundant if we decide to add the two more "basic" methods like isKnownToHaveSameStart/isKnownToBeCompleteView in future.

I think the following pieces of "basic" information provide everything we need to make a decision about the aliasing of the input and resulting views:

  • Can we prove that the resulting view is a complete view of the input view? This method may return true even in cases when the views have dynamic sizes, e.g. based on the semantics of the concrete operation.
  • Constant size of the input and the resulting views (or nullopt).
  • Constant offset applied to the resulting input view to produce the resulting view (or nullopt). isKnownToHaveSameStart is a simplified version of this query, i.e. it only says if the offset is zero and does not care to compute the actual non-zero constant offset, if it is possible.

I believe having these "basic" methods available, we can implement alias for all operations with ViewLikeOpInterface, and the concrete operations won't have any extra knowledge that will make sense overriding the alias method. That is why I decided to go with more "basic" methods (that may also be useful outside of the alias analysis, probably).

With that said, I agree replacing the two methods with an alias method. We can always adjust it in future if we end up adding all the method providing the "basic" information.

I can think of the following specification for the new alias method:

  • This method returns AliasResult::Kind representing the aliasing between the input and the resulting views of a ViewLikeOpInterface operation.
  • It returns NoAlias only when the resulting view is known to have zero size. Note that when the input view has zero size the resulting view must have zero size as well. Otherwise,
  • it returns MustAlias when the resulting view is known to start at the same address as the input view. Otherwise,
  • it returns PartialView when the resulting view is known to start at a non-zero offset of the input view. Otherwise,
  • it returns MayAlias.

Does this definition make sense to you?

Copy link
Member

@matthias-springer matthias-springer Nov 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I just noticed that isKnownToBeCompleteView is not expressible with the current AliasingResult::Kind. What I would recommend: Add a new FullAlias to AliasResult::Kind. Add documentation that this is not available in LLVM and that it's a superset of MustAlias. Existing code that looks for MustAlias should now use for MustAlias or FullAlias.

I believe having these "basic" methods available, we can implement alias for all operations with ViewLikeOpInterface, and the concrete operations won't have any extra knowledge that will make sense overriding the alias method.

I don't follow yet. What's the benefit of having "basic" methods if everything is expressible by a single method that returns AliasResult::Kind? The reason why I'm advocating for a single method is because we have an existing API that we can reuse with minor modifications. We don't need to invent a new one.

It returns NoAlias only when the resulting view is known to have zero size.

Is that actually correct according to the LLVM definition of these enum values? Even if both memrefs have size 0, their pointers could be the same. This sounds like MustAlias to me. (Or if we cannot prove that the two pointers are the same, it sounds like MayAlias to me.)

At the same time, LLVM's definition talks about: Another is when the two pointers are only ever used for reading memory. I'm not sure what to make of this. If the two memrefs have size 0, you can neither read nor write from it. Can we start with a more conservative implementation of MayAlias in this case, until we figure out the details?

Note that when the input view has zero size the resulting view must have zero size as well.

This is not necessarily the case. memref.reinterpret_cast implements this interface and it can return a larger memref. (Maybe it shouldn't implement the interface...)

/*args=*/(ins),
/*methodBody=*/"",
/*defaultImplementation=*/[{
return false;
Copy link
Member

@matthias-springer matthias-springer Oct 28, 2025

Choose a reason for hiding this comment

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

Is it safe for this to have a default implementation? Same question for isCompleteView.

Copy link
Contributor Author

@vzakhari vzakhari Oct 28, 2025

Choose a reason for hiding this comment

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

I added the default implementations so that they are conservatively correct, assuming that the methods will be used to perform some transformations based on the fact that a view starts at the same address as the source or/and a view is a complete view of the source.

Maybe, the names of the methods are confusing. Would it be more clear if I named them knownToHaveSameStart and knownToBeCompleteView? The idea is that when the methods return false, one cannot assume whether the view have or not have the same start, etc.

}]>,
InterfaceMethod<
/*desc=*/
[{
Returns true iff the resulting view is known to be
a complete view of the source buffer, i.e.
isKnownToHaveSameStart() is true and the result
has the same total size and is not a subview of the input.
}],
/*retTy=*/"bool",
/*methodName=*/"isKnownToBeCompleteView",
/*args=*/(ins),
/*methodBody=*/"",
/*defaultImplementation=*/[{
return false;
}]>];

let verify = [{
auto concreteOp = ::mlir::cast<ConcreteOp>($_op);
// isKnownToBeCompleteView() == true implies isKnownToHaveSameStart() == true.
return !concreteOp.isKnownToBeCompleteView() || concreteOp.isKnownToHaveSameStart() ? ::mlir::success() : ::mlir::failure();
}];
}

def OffsetSizeAndStrideOpInterface : OpInterface<"OffsetSizeAndStrideOpInterface"> {
Expand Down
Loading