[Mlir-commits] [flang] [mlir] [RFC][mlir] ViewLikeOpInterface method for detecting partial views. (PR #164020)
Slava Zakharin
llvmlistbot at llvm.org
Tue Oct 21 11:35:23 PDT 2025
https://github.com/vzakhari updated https://github.com/llvm/llvm-project/pull/164020
>From 19c498fdef9d18ced713af4fe0bc4cf834b603b3 Mon Sep 17 00:00:00 2001
From: Slava Zakharin <szakharin at nvidia.com>
Date: Fri, 17 Oct 2025 14:27:33 -0700
Subject: [PATCH 1/2] [RFC][mlir] ViewLikeOpInterface method for detecting
partial views.
I am trying to simplify the implementation of Flang's AliasAnalysis
by using ViewLikeOpInterface for some FIR dialect operations.
Currently, Flang's AliasAnalysis implementation explicitly recognizes
specific set of operations, which is inconvenient.
ViewLikeOpInterface may also be used in other parts of Flang.
In this patch, I only add ViewLikeOpInterface support to few
FIR operations to demonstrate the intent. Flang's alias analysis
LIT tests are failing with this change, and I would like to get
some advices on how to proceed.
As soon as I add, ViewLikeOpInterface interface support for FIR
operations, the LocalAliasAnalysis kicks in, and starts reporting
`MustAlias` for some cases where Flang currently reports MayAlias.
I believe both analyses are incorrect here: they should actually
report `PartialAlias` for those cases.
This is what the new method `isSameStart` should be used for.
I would like to fix both LocalAliasAnalysis and Flang's alias analysis
to return `PartialAlias` in those cases, but I am not sure if
querying `isSameStart` is enough - see the TBD comment in
ViewLikeInterface.td.
If my understanding of `PartialAlias` is correct, then we can have
a partial alias even when the starting addresses of the source
and the resulting view are the same - though, I might be wrong.
---
.../include/flang/Optimizer/Dialect/FIROps.h | 1 +
.../include/flang/Optimizer/Dialect/FIROps.td | 18 ++++++--
.../lib/Optimizer/Analysis/AliasAnalysis.cpp | 24 +++++------
.../mlir/Interfaces/ViewLikeInterface.td | 41 ++++++++++++-------
.../AliasAnalysis/LocalAliasAnalysis.cpp | 4 ++
5 files changed, 58 insertions(+), 30 deletions(-)
diff --git a/flang/include/flang/Optimizer/Dialect/FIROps.h b/flang/include/flang/Optimizer/Dialect/FIROps.h
index 62ef8b4b502f2..4651f2bb8038e 100644
--- a/flang/include/flang/Optimizer/Dialect/FIROps.h
+++ b/flang/include/flang/Optimizer/Dialect/FIROps.h
@@ -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 {
diff --git a/flang/include/flang/Optimizer/Dialect/FIROps.td b/flang/include/flang/Optimizer/Dialect/FIROps.td
index fc6eedc6ed4c6..5ce7390fca0d8 100644
--- a/flang/include/flang/Optimizer/Dialect/FIROps.td
+++ b/flang/include/flang/Optimizer/Dialect/FIROps.td
@@ -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"
@@ -1730,8 +1731,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";
@@ -1774,9 +1776,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";
@@ -1828,6 +1834,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(); }
}];
}
@@ -2792,7 +2799,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 = [{
@@ -2830,6 +2838,8 @@ 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 isSameStart() { return true; }
}];
let hasCanonicalizer = 1;
}
diff --git a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
index 73ddd1ff80126..f3470ecaa0ffc 100644
--- a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
+++ b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
@@ -559,10 +559,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) {
+ 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))
+ followingData = true;
+ if (!op.isSameStart())
+ approximateSource = true;
})
.Case<fir::PackArrayOp>([&](auto op) {
// The packed array is not distinguishable from the original
@@ -578,15 +587,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);
diff --git a/mlir/include/mlir/Interfaces/ViewLikeInterface.td b/mlir/include/mlir/Interfaces/ViewLikeInterface.td
index 131c1a0d92b24..5903623572b6e 100644
--- a/mlir/include/mlir/Interfaces/ViewLikeInterface.td
+++ b/mlir/include/mlir/Interfaces/ViewLikeInterface.td
@@ -23,21 +23,34 @@ 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<
+ // TBD: should it be generalized into isPartialView that will
+ // return true in any of these cases?
+ // 1. Resulting view is a slice view of the source.
+ // 2. Resulting view's start does not match the source's start.
+ // 3. Resulting view's end does not match the source's end.
+ /*desc=*/
+ [{ Returns true iff the source buffer and the resulting view start at the same "address". }],
+ /*retTy=*/"bool",
+ /*methodName=*/"isSameStart",
+ /*args=*/(ins),
+ /*methodBody=*/"",
+ /*defaultImplementation=*/[{
+ return false;
+ }]>];
}
def OffsetSizeAndStrideOpInterface : OpInterface<"OffsetSizeAndStrideOpInterface"> {
diff --git a/mlir/lib/Analysis/AliasAnalysis/LocalAliasAnalysis.cpp b/mlir/lib/Analysis/AliasAnalysis/LocalAliasAnalysis.cpp
index a84d10d5d609d..b0ec97bd9f264 100644
--- a/mlir/lib/Analysis/AliasAnalysis/LocalAliasAnalysis.cpp
+++ b/mlir/lib/Analysis/AliasAnalysis/LocalAliasAnalysis.cpp
@@ -129,6 +129,10 @@ static void collectUnderlyingAddressValues(OpResult result, unsigned maxDepth,
// If this is a view, unwrap to the source.
if (ViewLikeOpInterface view = dyn_cast<ViewLikeOpInterface>(op)) {
if (result == view.getViewDest()) {
+ // TODO: if view.isSameStart() returns false here,
+ // we have to make sure that further analysis may return
+ // PartialAlias for all underlying addresses we are going
+ // to collect from this point up the def-use.
return collectUnderlyingAddressValues(view.getViewSource(), maxDepth,
visited, output);
}
>From 36577e385ba1728654098f19e420c628ef68c22d Mon Sep 17 00:00:00 2001
From: Slava Zakharin <szakharin at nvidia.com>
Date: Tue, 21 Oct 2025 10:50:54 -0700
Subject: [PATCH 2/2] Fixed LocalAliasAnalysis to return PartialAlias instead
of MustAlias in more cases. There are still cases where LocalAliasAnalysis
incorrectly returns MustAlias (see
mlir/test/Analysis/test-alias-analysis.mlir).
Also see a NOTE in flang/test/Analysis/AliasAnalysis/ptr-component.fir,
where combining LocalAliasAnalysis result (PartialAlias) and FIR AliasAnalysis
result (NoAlias) results in PartialAlias, which is more conservative than
the previous NoAlias result. It is unclear how to address this case,
given the merging rules for multiple alias analyses' results.
---
.../include/flang/Optimizer/Dialect/FIROps.td | 1 +
.../lib/Optimizer/Analysis/AliasAnalysis.cpp | 6 +-
.../AliasAnalysis/load-ptr-designate.fir | 12 +-
.../Analysis/AliasAnalysis/ptr-component.fir | 21 ++--
mlir/include/mlir/Analysis/AliasAnalysis.h | 7 +-
.../mlir/Dialect/MemRef/IR/MemRefOps.td | 10 +-
.../mlir/Interfaces/ViewLikeInterface.td | 21 +++-
.../AliasAnalysis/LocalAliasAnalysis.cpp | 98 ++++++++-------
mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp | 16 +++
.../test-alias-analysis-extending.mlir | 9 ++
mlir/test/Analysis/test-alias-analysis.mlir | 112 ++++++++++++++++++
11 files changed, 243 insertions(+), 70 deletions(-)
diff --git a/flang/include/flang/Optimizer/Dialect/FIROps.td b/flang/include/flang/Optimizer/Dialect/FIROps.td
index 5ce7390fca0d8..58aaa1cdf18fa 100644
--- a/flang/include/flang/Optimizer/Dialect/FIROps.td
+++ b/flang/include/flang/Optimizer/Dialect/FIROps.td
@@ -2840,6 +2840,7 @@ def fir_ConvertOp
static bool areVectorsCompatible(mlir::Type inTy, mlir::Type outTy);
mlir::Value getViewSource() { return getValue(); }
bool isSameStart() { return true; }
+ bool isCompleteView() { return true; }
}];
let hasCanonicalizer = 1;
}
diff --git a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
index f3470ecaa0ffc..62b18e5a7258c 100644
--- a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
+++ b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
@@ -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.
if (approximateSource)
return AliasResult::MayAlias;
return AliasResult::MustAlias;
@@ -569,7 +573,7 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v,
// so we should set followingData.
if (mlir::isa<fir::BaseBoxType>(v.getType()) &&
!mlir::isa<fir::BaseBoxType>(ty))
- followingData = true;
+ followBoxData = true;
if (!op.isSameStart())
approximateSource = true;
})
diff --git a/flang/test/Analysis/AliasAnalysis/load-ptr-designate.fir b/flang/test/Analysis/AliasAnalysis/load-ptr-designate.fir
index 6b9ff25f590f3..501c824574da7 100644
--- a/flang/test/Analysis/AliasAnalysis/load-ptr-designate.fir
+++ b/flang/test/Analysis/AliasAnalysis/load-ptr-designate.fir
@@ -88,8 +88,8 @@
// CHECK-DAG: obj#0 <-> obj%alloc#0: MayAlias
// CHECK-DAG: obj#0 <-> obj%p0.tgt#0: MayAlias
// CHECK-DAG: obj#0 <-> obj%alloc.tgt#0: MayAlias
-// CHECK-DAG: obj.fir#0 <-> obj%p0.fir#0: MayAlias
-// CHECK-DAG: obj.fir#0 <-> obj%alloc.fir#0: MayAlias
+// CHECK-DAG: obj.fir#0 <-> obj%p0.fir#0: PartialAlias
+// CHECK-DAG: obj.fir#0 <-> obj%alloc.fir#0: PartialAlias
// CHECK-DAG: obj.fir#0 <-> obj%p0.tgt.fir#0: MayAlias
// CHECK-DAG: obj.fir#0 <-> obj%alloc.tgt.fir#0: MayAlias
@@ -252,8 +252,8 @@ func.func @_QPtest.fir() {
// CHECK-DAG: obj#0 <-> obj%alloc#0: MayAlias
// CHECK-DAG: obj#0 <-> obj%p0.tgt#0: MayAlias
// CHECK-DAG: obj#0 <-> obj%alloc.tgt#0: MayAlias
-// CHECK-DAG: obj.fir#0 <-> obj%p0.fir#0: MayAlias
-// CHECK-DAG: obj.fir#0 <-> obj%alloc.fir#0: MayAlias
+// CHECK-DAG: obj.fir#0 <-> obj%p0.fir#0: PartialAlias
+// CHECK-DAG: obj.fir#0 <-> obj%alloc.fir#0: PartialAlias
// CHECK-DAG: obj.fir#0 <-> obj%p0.tgt.fir#0: MayAlias
// CHECK-DAG: obj.fir#0 <-> obj%alloc.tgt.fir#0: MayAlias
@@ -412,8 +412,8 @@ func.func @_QPtest.fir(%arg0: !fir.ref<!fir.type<_QMmTty{p0:!fir.box<!fir.ptr<f3
// CHECK-DAG: obj#0 <-> obj%alloc#0: MayAlias
// CHECK-DAG: obj#0 <-> obj%p0.tgt#0: MayAlias
// CHECK-DAG: obj#0 <-> obj%alloc.tgt#0: MayAlias
-// CHECK-DAG: obj.fir#0 <-> obj%p0.fir#0: MayAlias
-// CHECK-DAG: obj.fir#0 <-> obj%alloc.fir#0: MayAlias
+// CHECK-DAG: obj.fir#0 <-> obj%p0.fir#0: PartialAlias
+// CHECK-DAG: obj.fir#0 <-> obj%alloc.fir#0: PartialAlias
// CHECK-DAG: obj.fir#0 <-> obj%p0.tgt.fir#0: MayAlias
// CHECK-DAG: obj.fir#0 <-> obj%alloc.tgt.fir#0: MayAlias
diff --git a/flang/test/Analysis/AliasAnalysis/ptr-component.fir b/flang/test/Analysis/AliasAnalysis/ptr-component.fir
index cdd18832b113e..0b9209bd008a6 100644
--- a/flang/test/Analysis/AliasAnalysis/ptr-component.fir
+++ b/flang/test/Analysis/AliasAnalysis/ptr-component.fir
@@ -41,13 +41,13 @@
// therefore x needs to alias with x.next to prevent the loads from being merged.
// CHECK-DAG: x#0 <-> xnext1#0: MayAlias
// CHECK-DAG: x#0 <-> xnext2#0: MayAlias
-// CHECK-DAG: x.fir#0 <-> xnext1.fir#0: MayAlias
-// CHECK-DAG: x.fir#0 <-> xnext2.fir#0: MayAlias
+// CHECK-DAG: x.fir#0 <-> xnext1.fir#0: PartialAlias
+// CHECK-DAG: x.fir#0 <-> xnext2.fir#0: PartialAlias
// TODO: xnext1#0 <-> xnext2#0 are the same and therefore MustAlias but
// we are currently not comparing operands involved in offset computations
// CHECK-DAG: xnext1#0 <-> xnext2#0: MayAlias
-// CHECK-DAG: xnext1.fir#0 <-> xnext2.fir#0: MayAlias
+// CHECK-DAG: xnext1.fir#0 <-> xnext2.fir#0: PartialAlias
// CHECK-DAG: xnext1#0 <-> ynext#0: NoAlias
// CHECK-DAG: xnext1.fir#0 <-> ynext.fir#0: NoAlias
@@ -217,7 +217,7 @@ func.func @_QMmPfoo3.fir(%arg0: !fir.ref<!fir.type<_QMmTta{array:!fir.box<!fir.p
// CHECK-LABEL: Testing : "_QMmPtest"
// CHECK-DAG: x#0 <-> x%p#0: MayAlias
-// CHECK-DAG: x.fir#0 <-> x%p.fir#0: MayAlias
+// CHECK-DAG: x.fir#0 <-> x%p.fir#0: PartialAlias
func.func @_QMmPtest() {
%0 = fir.address_of(@_QMmEx) : !fir.ref<!fir.type<_QMmTt{p:!fir.box<!fir.heap<i32>>}>>
@@ -258,8 +258,8 @@ func.func @_QMmPtest.fir() {
// composite is nested within another composite.
// CHECK-DAG: x#0 <-> x%p#0: MayAlias
// CHECK-DAG: x%x#0 <-> x%x%p#0: MayAlias
-// CHECK-DAG: x.fir#0 <-> x%p.fir#0: MayAlias
-// CHECK-DAG: x%x.fir#0 <-> x%x%p.fir#0: MayAlias
+// CHECK-DAG: x.fir#0 <-> x%p.fir#0: PartialAlias
+// CHECK-DAG: x%x.fir#0 <-> x%x%p.fir#0: PartialAlias
// The addresses of different components of the same composite do not alias.
//
@@ -274,9 +274,12 @@ func.func @_QMmPtest.fir() {
// CHECK-DAG: x%x#0 <-> x%i#0: MayAlias
// CHECK-DAG: x%p#0 <-> x%i#0: NoAlias
// CHECK-DAG: x%x#0 <-> x%p#0: MayAlias
-// CHECK-DAG: x%x.fir#0 <-> x%i.fir#0: MayAlias
-// CHECK-DAG: x%p.fir#0 <-> x%i.fir#0: NoAlias
-// CHECK-DAG: x%x.fir#0 <-> x%p.fir#0: MayAlias
+// CHECK-DAG: x%x.fir#0 <-> x%i.fir#0: PartialAlias
+
+// NOTE: FIR AliasAnalysis can prove NoAlias, but LocalAliasAnalysis
+// conservatively assumes PartialAlias.
+// CHECK-DAG: x%p.fir#0 <-> x%i.fir#0: PartialAlias
+// CHECK-DAG: x%x.fir#0 <-> x%p.fir#0: PartialAlias
func.func @_QMmPtest() {
%0 = fir.alloca !fir.type<_QMmTt2{x:!fir.type<_QMmTt1{p:!fir.box<!fir.ptr<i32>>}>,p:!fir.box<!fir.ptr<i32>>,i:i32}> {bindc_name = "x", uniq_name = "_QMmFtestEx"}
diff --git a/mlir/include/mlir/Analysis/AliasAnalysis.h b/mlir/include/mlir/Analysis/AliasAnalysis.h
index 5beae79ad1a03..d5528a0ceb181 100644
--- a/mlir/include/mlir/Analysis/AliasAnalysis.h
+++ b/mlir/include/mlir/Analysis/AliasAnalysis.h
@@ -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.
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,
};
diff --git a/mlir/include/mlir/Dialect/MemRef/IR/MemRefOps.td b/mlir/include/mlir/Dialect/MemRef/IR/MemRefOps.td
index b39207fc30dd7..e7a4dd545f6ba 100644
--- a/mlir/include/mlir/Dialect/MemRef/IR/MemRefOps.td
+++ b/mlir/include/mlir/Dialect/MemRef/IR/MemRefOps.td
@@ -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());
@@ -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 start
+ /// at the same address.
+ bool isSameStart();
}];
let hasCanonicalizer = 1;
@@ -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 start
+ /// at the same address.
+ bool isSameStart();
}];
let assemblyFormat = [{
diff --git a/mlir/include/mlir/Interfaces/ViewLikeInterface.td b/mlir/include/mlir/Interfaces/ViewLikeInterface.td
index 5903623572b6e..8d71d50a5c155 100644
--- a/mlir/include/mlir/Interfaces/ViewLikeInterface.td
+++ b/mlir/include/mlir/Interfaces/ViewLikeInterface.td
@@ -37,11 +37,6 @@ def ViewLikeOpInterface : OpInterface<"ViewLikeOpInterface"> {
return $_op->getResult(0);
}]>,
InterfaceMethod<
- // TBD: should it be generalized into isPartialView that will
- // return true in any of these cases?
- // 1. Resulting view is a slice view of the source.
- // 2. Resulting view's start does not match the source's start.
- // 3. Resulting view's end does not match the source's end.
/*desc=*/
[{ Returns true iff the source buffer and the resulting view start at the same "address". }],
/*retTy=*/"bool",
@@ -50,7 +45,23 @@ def ViewLikeOpInterface : OpInterface<"ViewLikeOpInterface"> {
/*methodBody=*/"",
/*defaultImplementation=*/[{
return false;
+ }]>,
+ InterfaceMethod<
+ /*desc=*/
+ [{ Returns true iff the resulting view is a complete view of the source buffer, i.e. isSameStart() is true and the result has the same total size and is not a subview of the input. }],
+ /*retTy=*/"bool",
+ /*methodName=*/"isCompleteView",
+ /*args=*/(ins),
+ /*methodBody=*/"",
+ /*defaultImplementation=*/[{
+ return false;
}]>];
+
+ let verify = [{
+ auto concreteOp = ::mlir::cast<ConcreteOp>($_op);
+ // isCompleteView() == true implies isSameStart() == true.
+ return !concreteOp.isCompleteView() || concreteOp.isSameStart() ? ::mlir::success() : ::mlir::failure();
+ }];
}
def OffsetSizeAndStrideOpInterface : OpInterface<"OffsetSizeAndStrideOpInterface"> {
diff --git a/mlir/lib/Analysis/AliasAnalysis/LocalAliasAnalysis.cpp b/mlir/lib/Analysis/AliasAnalysis/LocalAliasAnalysis.cpp
index b0ec97bd9f264..610d7d0150ed0 100644
--- a/mlir/lib/Analysis/AliasAnalysis/LocalAliasAnalysis.cpp
+++ b/mlir/lib/Analysis/AliasAnalysis/LocalAliasAnalysis.cpp
@@ -38,20 +38,23 @@ using namespace mlir;
static constexpr unsigned maxUnderlyingValueSearchDepth = 10;
/// Given a value, collect all of the underlying values being addressed.
-static void collectUnderlyingAddressValues(Value value, unsigned maxDepth,
- DenseSet<Value> &visited,
- SmallVectorImpl<Value> &output);
+/// When `maybeOffset` is true, this means that the value is used
+/// by an operation that potentially offsets the `value` making
+/// the access through the operation's result potentially partial
+/// with regards to the memory object pointed to by the `value`.
+static void
+collectUnderlyingAddressValues(Value value, unsigned maxDepth,
+ DenseSet<Value> &visited, bool maybeOffset,
+ SmallVectorImpl<std::pair<Value, bool>> &output);
/// Given a successor (`region`) of a RegionBranchOpInterface, collect all of
/// the underlying values being addressed by one of the successor inputs. If the
/// provided `region` is null, as per `RegionBranchOpInterface` this represents
/// the parent operation.
-static void collectUnderlyingAddressValues(RegionBranchOpInterface branch,
- Region *region, Value inputValue,
- unsigned inputIndex,
- unsigned maxDepth,
- DenseSet<Value> &visited,
- SmallVectorImpl<Value> &output) {
+static void collectUnderlyingAddressValues(
+ RegionBranchOpInterface branch, Region *region, Value inputValue,
+ unsigned inputIndex, unsigned maxDepth, DenseSet<Value> &visited,
+ bool maybeOffset, SmallVectorImpl<std::pair<Value, bool>> &output) {
// Given the index of a region of the branch (`predIndex`), or std::nullopt to
// represent the parent operation, try to return the index into the outputs of
// this region predecessor that correspond to the input values of `region`. If
@@ -66,7 +69,7 @@ static void collectUnderlyingAddressValues(RegionBranchOpInterface branch,
// Check that the successor inputs map to the given input value.
ValueRange inputs = successor.getSuccessorInputs();
if (inputs.empty()) {
- output.push_back(inputValue);
+ output.push_back({inputValue, maybeOffset});
break;
}
unsigned firstInputIndex, lastInputIndex;
@@ -78,7 +81,7 @@ static void collectUnderlyingAddressValues(RegionBranchOpInterface branch,
lastInputIndex = cast<OpResult>(inputs.back()).getResultNumber();
}
if (firstInputIndex > inputIndex || lastInputIndex < inputIndex) {
- output.push_back(inputValue);
+ output.push_back({inputValue, maybeOffset});
break;
}
return inputIndex - firstInputIndex;
@@ -95,7 +98,7 @@ static void collectUnderlyingAddressValues(RegionBranchOpInterface branch,
getOperandIndexIfPred(/*predIndex=*/RegionBranchPoint::parent())) {
collectUnderlyingAddressValues(
branch.getEntrySuccessorOperands(branchPoint)[*operandIndex], maxDepth,
- visited, output);
+ visited, maybeOffset, output);
}
// Check branches from each child region.
Operation *op = branch.getOperation();
@@ -108,11 +111,11 @@ static void collectUnderlyingAddressValues(RegionBranchOpInterface branch,
block.getTerminator())) {
collectUnderlyingAddressValues(
term.getSuccessorOperands(branchPoint)[*operandIndex], maxDepth,
- visited, output);
+ visited, maybeOffset, output);
} else if (block.getNumSuccessors()) {
// Otherwise, if this terminator may exit the region we can't make
// any assumptions about which values get passed.
- output.push_back(inputValue);
+ output.push_back({inputValue, maybeOffset});
return;
}
}
@@ -121,37 +124,33 @@ static void collectUnderlyingAddressValues(RegionBranchOpInterface branch,
}
/// Given a result, collect all of the underlying values being addressed.
-static void collectUnderlyingAddressValues(OpResult result, unsigned maxDepth,
- DenseSet<Value> &visited,
- SmallVectorImpl<Value> &output) {
+static void collectUnderlyingAddressValues(
+ OpResult result, unsigned maxDepth, DenseSet<Value> &visited,
+ bool maybeOffset, SmallVectorImpl<std::pair<Value, bool>> &output) {
Operation *op = result.getOwner();
// If this is a view, unwrap to the source.
if (ViewLikeOpInterface view = dyn_cast<ViewLikeOpInterface>(op)) {
if (result == view.getViewDest()) {
- // TODO: if view.isSameStart() returns false here,
- // we have to make sure that further analysis may return
- // PartialAlias for all underlying addresses we are going
- // to collect from this point up the def-use.
- return collectUnderlyingAddressValues(view.getViewSource(), maxDepth,
- visited, output);
+ return collectUnderlyingAddressValues(
+ view.getViewSource(), maxDepth, visited, !view.isSameStart(), output);
}
}
// Check to see if we can reason about the control flow of this op.
if (auto branch = dyn_cast<RegionBranchOpInterface>(op)) {
return collectUnderlyingAddressValues(branch, /*region=*/nullptr, result,
result.getResultNumber(), maxDepth,
- visited, output);
+ visited, maybeOffset, output);
}
- output.push_back(result);
+ output.push_back({result, maybeOffset});
}
/// Given a block argument, collect all of the underlying values being
/// addressed.
-static void collectUnderlyingAddressValues(BlockArgument arg, unsigned maxDepth,
- DenseSet<Value> &visited,
- SmallVectorImpl<Value> &output) {
+static void collectUnderlyingAddressValues(
+ BlockArgument arg, unsigned maxDepth, DenseSet<Value> &visited,
+ bool maybeOffset, SmallVectorImpl<std::pair<Value, bool>> &output) {
Block *block = arg.getOwner();
unsigned argNumber = arg.getArgNumber();
@@ -161,7 +160,7 @@ static void collectUnderlyingAddressValues(BlockArgument arg, unsigned maxDepth,
auto branch = dyn_cast<BranchOpInterface>((*it)->getTerminator());
if (!branch) {
// We can't analyze the control flow, so bail out early.
- output.push_back(arg);
+ output.push_back({arg, maybeOffset});
return;
}
@@ -170,10 +169,11 @@ static void collectUnderlyingAddressValues(BlockArgument arg, unsigned maxDepth,
Value operand = branch.getSuccessorOperands(index)[argNumber];
if (!operand) {
// We can't analyze the control flow, so bail out early.
- output.push_back(arg);
+ output.push_back({arg, maybeOffset});
return;
}
- collectUnderlyingAddressValues(operand, maxDepth, visited, output);
+ collectUnderlyingAddressValues(operand, maxDepth, visited, maybeOffset,
+ output);
}
return;
}
@@ -182,39 +182,40 @@ static void collectUnderlyingAddressValues(BlockArgument arg, unsigned maxDepth,
Region *region = block->getParent();
Operation *op = region->getParentOp();
if (auto branch = dyn_cast<RegionBranchOpInterface>(op)) {
- return collectUnderlyingAddressValues(branch, region, arg, argNumber,
- maxDepth, visited, output);
+ return collectUnderlyingAddressValues(
+ branch, region, arg, argNumber, maxDepth, visited, maybeOffset, output);
}
// We can't reason about the underlying address of this argument.
- output.push_back(arg);
+ output.push_back({arg, maybeOffset});
}
/// Given a value, collect all of the underlying values being addressed.
-static void collectUnderlyingAddressValues(Value value, unsigned maxDepth,
- DenseSet<Value> &visited,
- SmallVectorImpl<Value> &output) {
+static void collectUnderlyingAddressValues(
+ Value value, unsigned maxDepth, DenseSet<Value> &visited, bool maybeOffset,
+ SmallVectorImpl<std::pair<Value, bool>> &output) {
// Check that we don't infinitely recurse.
if (!visited.insert(value).second)
return;
if (maxDepth == 0) {
- output.push_back(value);
+ output.push_back({value, maybeOffset});
return;
}
--maxDepth;
if (BlockArgument arg = dyn_cast<BlockArgument>(value))
- return collectUnderlyingAddressValues(arg, maxDepth, visited, output);
+ return collectUnderlyingAddressValues(arg, maxDepth, visited, maybeOffset,
+ output);
collectUnderlyingAddressValues(cast<OpResult>(value), maxDepth, visited,
- output);
+ maybeOffset, output);
}
/// Given a value, collect all of the underlying values being addressed.
-static void collectUnderlyingAddressValues(Value value,
- SmallVectorImpl<Value> &output) {
+static void collectUnderlyingAddressValues(
+ Value value, SmallVectorImpl<std::pair<Value, bool>> &output) {
DenseSet<Value> visited;
collectUnderlyingAddressValues(value, maxUnderlyingValueSearchDepth, visited,
- output);
+ /*maybeOffset=*/false, output);
}
//===----------------------------------------------------------------------===//
@@ -375,7 +376,7 @@ AliasResult LocalAliasAnalysis::alias(Value lhs, Value rhs) {
return AliasResult::MustAlias;
// Get the underlying values being addressed.
- SmallVector<Value, 8> lhsValues, rhsValues;
+ SmallVector<std::pair<Value, bool>, 8> lhsValues, rhsValues;
collectUnderlyingAddressValues(lhs, lhsValues);
collectUnderlyingAddressValues(rhs, rhsValues);
@@ -386,9 +387,14 @@ AliasResult LocalAliasAnalysis::alias(Value lhs, Value rhs) {
// Check the alias results against each of the underlying values.
std::optional<AliasResult> result;
- for (Value lhsVal : lhsValues) {
- for (Value rhsVal : rhsValues) {
+ for (auto [lhsVal, lhsPartial] : lhsValues) {
+ for (auto [rhsVal, rhsPartial] : rhsValues) {
AliasResult nextResult = aliasImpl(lhsVal, rhsVal);
+ // If the original lhs/rhs may be an offsetted access
+ // of the memory objects pointed to by lhsVal/rhsVal,
+ // we'd better turn MustAlias results into PartialAlias.
+ if (nextResult == AliasResult::MustAlias && (lhsPartial || rhsPartial))
+ nextResult = AliasResult::PartialAlias;
result = result ? result->merge(nextResult) : nextResult;
}
}
diff --git a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
index 94947b760251e..148abeb99f54f 100644
--- a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
+++ b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
@@ -3032,6 +3032,15 @@ void SubViewOp::build(OpBuilder &b, OperationState &result, Value source,
/// For ViewLikeOpInterface.
Value SubViewOp::getViewSource() { return getSource(); }
+bool SubViewOp::isSameStart() {
+ // TODO: we can recognize simple constant operands.
+ if (!getOffsets().empty())
+ return false;
+ ArrayRef<int64_t> staticOffsets = getStaticOffsets();
+ return llvm::all_of(staticOffsets,
+ [](int64_t offset) { return offset == 0; });
+}
+
/// Return true if `t1` and `t2` have equal offsets (both dynamic or of same
/// static value).
static bool haveCompatibleOffsets(MemRefType t1, MemRefType t2) {
@@ -3671,6 +3680,13 @@ LogicalResult ViewOp::verify() {
Value ViewOp::getViewSource() { return getSource(); }
+bool ViewOp::isSameStart() {
+ IntegerAttr offsetAttr;
+ if (matchPattern(getByteShift(), m_Constant(&offsetAttr)))
+ return offsetAttr.getValue().isZero();
+ return false;
+}
+
OpFoldResult ViewOp::fold(FoldAdaptor adaptor) {
MemRefType sourceMemrefType = getSource().getType();
MemRefType resultMemrefType = getResult().getType();
diff --git a/mlir/test/Analysis/test-alias-analysis-extending.mlir b/mlir/test/Analysis/test-alias-analysis-extending.mlir
index 20f2a7a3de171..a5078abc6a9e5 100644
--- a/mlir/test/Analysis/test-alias-analysis-extending.mlir
+++ b/mlir/test/Analysis/test-alias-analysis-extending.mlir
@@ -6,10 +6,19 @@
// CHECK-DAG: view1#0 <-> view2#0: NoAlias
// CHECK-DAG: view1#0 <-> func.region0#0: MustAlias
// CHECK-DAG: view1#0 <-> func.region0#1: NoAlias
+// CHECK-DAG: view1#0 <-> view3#0: PartialAlias
+// CHECK-DAG: view1#0 <-> view4#0: NoAlias
// CHECK-DAG: view2#0 <-> func.region0#0: NoAlias
// CHECK-DAG: view2#0 <-> func.region0#1: MustAlias
+// CHECK-DAG: view2#0 <-> view3#0: NoAlias
+// CHECK-DAG: view2#0 <-> view4#0: PartialAlias
+// CHECK-DAG: view3#0 <-> func.region0#0: PartialAlias
+// CHECK-DAG: view3#0 <-> func.region0#1: NoAlias
+// CHECK-DAG: view3#0 <-> view4#0: NoAlias
func.func @restrict(%arg: memref<?xf32>, %arg1: memref<?xf32> {local_alias_analysis.restrict}) attributes {test.ptr = "func"} {
%0 = memref.subview %arg[0][2][1] {test.ptr = "view1"} : memref<?xf32> to memref<2xf32>
%1 = memref.subview %arg1[0][2][1] {test.ptr = "view2"} : memref<?xf32> to memref<2xf32>
+ %2 = memref.subview %arg[1][2][1] {test.ptr = "view3"} : memref<?xf32> to memref<2xf32, strided<[1], offset: 1>>
+ %3 = memref.subview %arg1[1][2][1] {test.ptr = "view4"} : memref<?xf32> to memref<2xf32, strided<[1], offset: 1>>
return
}
diff --git a/mlir/test/Analysis/test-alias-analysis.mlir b/mlir/test/Analysis/test-alias-analysis.mlir
index d71adee05c7a3..2263f2f57a21e 100644
--- a/mlir/test/Analysis/test-alias-analysis.mlir
+++ b/mlir/test/Analysis/test-alias-analysis.mlir
@@ -272,3 +272,115 @@ func.func @distinct_objects(%arg: memref<?xf32>, %arg1: memref<?xf32>) attribute
%0, %1 = memref.distinct_objects %arg, %arg1 {test.ptr = "distinct"} : memref<?xf32>, memref<?xf32>
return
}
+
+// -----
+
+// CHECK-LABEL: Testing : "view_like_offset"
+
+// The current LocalAliasAnalysis algortithm is quite conservative
+// about differentiating PartialAlias and MustAlias, and it often
+// returns PartialAlias for MustAlias situations.
+
+// These are the PartialAlias cases, where LocalAliasAnalysis
+// used to incorrectly return MustAlias:
+// CHECK-DAG: view_with_offset#0 <-> func.region0#0: PartialAlias
+// CHECK-DAG: complete_view1#0 <-> func.region0#0: PartialAlias
+// CHECK-DAG: complete_view2#0 <-> func.region0#0: PartialAlias
+
+// TODO: these are the MustAlias cases, where PartialAlias is returned currently:
+// CHECK-DAG: view_with_offset#0 <-> complete_view1#0: PartialAlias
+// CHECK-DAG: view_with_offset#0 <-> complete_view2#0: PartialAlias
+// CHECK-DAG: complete_view1#0 <-> complete_view2#0: PartialAlias
+func.func @view_like_offset(%arg: memref<?xi8>) attributes {test.ptr = "func"} {
+ %c0 = arith.constant 0 : index
+ %c1 = arith.constant 1 : index
+ %view_with_offset = memref.view %arg[%c1][] {test.ptr = "view_with_offset"} : memref<?xi8> to memref<8xi8>
+ %complete_view1 = memref.view %view_with_offset[%c0][] {test.ptr = "complete_view1"} : memref<8xi8> to memref<8xi8>
+ %complete_view2 = memref.view %view_with_offset[%c0][] {test.ptr = "complete_view2"} : memref<8xi8> to memref<8xi8>
+ return
+}
+
+// -----
+
+// CHECK-LABEL: Testing : "view_like_offset_with_cfg1"
+
+// These are the PartialAlias cases, where LocalAliasAnalysis
+// used to incorrectly return MustAlias:
+// CHECK-DAG: complete_view_if#0 <-> view_with_offset#0: PartialAlias
+// CHECK-DAG: view_with_offset#0 <-> if_view#0: PartialAlias
+// CHECK-DAG: view_with_offset#0 <-> complete_view1#0: PartialAlias
+// CHECK-DAG: view_with_offset#0 <-> complete_view2#0: PartialAlias
+// CHECK-DAG: view_with_offset#0 <-> func.region0#0: PartialAlias
+
+// These are correctly classified as MustAlias:
+// CHECK-DAG: if_view#0 <-> complete_view1#0: MustAlias
+// CHECK-DAG: if_view#0 <-> complete_view2#0: MustAlias
+// CHECK-DAG: complete_view1#0 <-> complete_view2#0: MustAlias
+// CHECK-DAG: complete_view_if#0 <-> func.region0#0: MustAlias
+
+// TODO: these should be either PartialAlias or MayAlias:
+// CHECK-DAG: complete_view_if#0 <-> if_view#0: MustAlias
+// CHECK-DAG: complete_view_if#0 <-> complete_view1#0: MustAlias
+// CHECK-DAG: complete_view_if#0 <-> complete_view2#0: MustAlias
+// CHECK-DAG: if_view#0 <-> func.region0#0: MustAlias
+// CHECK-DAG: complete_view1#0 <-> func.region0#0: MustAlias
+// CHECK-DAG: complete_view2#0 <-> func.region0#0: MustAlias
+func.func @view_like_offset_with_cfg1(%arg: memref<?xi8>, %cond: i1) attributes {test.ptr = "func"} {
+ %c0 = arith.constant 0 : index
+ %c1 = arith.constant 1 : index
+ %0 = scf.if %cond -> (memref<8xi8>) {
+ %complete_view_if = memref.view %arg[%c0][] {test.ptr = "complete_view_if"} : memref<?xi8> to memref<8xi8>
+ scf.yield %complete_view_if : memref<8xi8>
+ } else {
+ %view_with_offset = memref.view %arg[%c1][] {test.ptr = "view_with_offset"} : memref<?xi8> to memref<8xi8>
+ scf.yield %view_with_offset : memref<8xi8>
+ } {test.ptr = "if_view"}
+ %complete_view1 = memref.view %0[%c0][] {test.ptr = "complete_view1"} : memref<8xi8> to memref<8xi8>
+ %complete_view2 = memref.view %0[%c0][] {test.ptr = "complete_view2"} : memref<8xi8> to memref<8xi8>
+ return
+}
+
+// -----
+
+// CHECK-LABEL: Testing : "view_like_offset_with_cfg2"
+
+// This test is a different version of view_like_offset_with_cfg1:
+// the then and else clauses are swapped, and this affects
+// the visiting order causing different PartialAlias vs MustAlias
+// reports.
+
+// These are the PartialAlias cases, where LocalAliasAnalysis
+// used to incorrectly return MustAlias:
+// CHECK-DAG: view_with_offset#0 <-> complete_view_if#0: PartialAlias
+// CHECK-DAG: view_with_offset#0 <-> if_view#0: PartialAlias
+// CHECK-DAG: complete_view_if#0 <-> if_view#0: PartialAlias
+// CHECK-DAG: view_with_offset#0 <-> complete_view1#0: PartialAlias
+// CHECK-DAG: complete_view_if#0 <-> complete_view1#0: PartialAlias
+// CHECK-DAG: view_with_offset#0 <-> complete_view2#0: PartialAlias
+// CHECK-DAG: complete_view_if#0 <-> complete_view2#0: PartialAlias
+// CHECK-DAG: view_with_offset#0 <-> func.region0#0: PartialAlias
+// CHECK-DAG: if_view#0 <-> func.region0#0: PartialAlias
+// CHECK-DAG: complete_view1#0 <-> func.region0#0: PartialAlias
+// CHECK-DAG: complete_view2#0 <-> func.region0#0: PartialAlias
+
+// These are correctly classified as MustAlias:
+// CHECK-DAG: complete_view_if#0 <-> func.region0#0: MustAlias
+
+// TODO: these are the MustAlias cases, where PartialAlias is returned currently:
+// CHECK-DAG: if_view#0 <-> complete_view1#0: PartialAlias
+// CHECK-DAG: if_view#0 <-> complete_view2#0: PartialAlias
+// CHECK-DAG: complete_view1#0 <-> complete_view2#0: PartialAlias
+func.func @view_like_offset_with_cfg2(%arg: memref<?xi8>, %cond: i1) attributes {test.ptr = "func"} {
+ %c0 = arith.constant 0 : index
+ %c1 = arith.constant 1 : index
+ %0 = scf.if %cond -> (memref<8xi8>) {
+ %view_with_offset = memref.view %arg[%c1][] {test.ptr = "view_with_offset"} : memref<?xi8> to memref<8xi8>
+ scf.yield %view_with_offset : memref<8xi8>
+ } else {
+ %complete_view_if = memref.view %arg[%c0][] {test.ptr = "complete_view_if"} : memref<?xi8> to memref<8xi8>
+ scf.yield %complete_view_if : memref<8xi8>
+ } {test.ptr = "if_view"}
+ %complete_view1 = memref.view %0[%c0][] {test.ptr = "complete_view1"} : memref<8xi8> to memref<8xi8>
+ %complete_view2 = memref.view %0[%c0][] {test.ptr = "complete_view2"} : memref<8xi8> to memref<8xi8>
+ return
+}
More information about the Mlir-commits
mailing list