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..58aaa1cdf18fa 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::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,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 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 73ddd1ff80126..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; @@ -559,10 +563,19 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v, type = SourceKind::Allocate; breakFromLoop = true; }) - .Case([&](auto op) { - // Skip ConvertOp's and track further through the operand. - v = op->getOperand(0); + .Case([&](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(v.getType()) && + !mlir::isa(ty)) + followBoxData = true; + if (!op.isSameStart()) + approximateSource = true; }) .Case([&](auto op) { // The packed array is not distinguishable from the original @@ -578,15 +591,6 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v, if (mlir::isa(v.getType())) followBoxData = true; }) - .Case([&](auto op) { - if (isPointerReference(ty)) - attributes.set(Attribute::Pointer); - v = op->getOperand(0); - defOp = v.getDefiningOp(); - if (mlir::isa(v.getType())) - followBoxData = true; - approximateSource = true; - }) .Case([&](auto op) { if (followBoxData) { v = op->getOperand(0); 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 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 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>}>> @@ -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>}>,p:!fir.box>,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", "{}">:$attrs)> ]; - let extraClassDeclaration = extraBaseClassDeclaration # [{ + let extraClassDeclaration = extraBaseClassDeclaration#[{ /// Returns the type of the base memref operand. MemRefType getSourceType() { return ::llvm::cast(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 rankReduceIfNeeded( OpBuilder &b, Location loc, Value value, ArrayRef 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 131c1a0d92b24..8d71d50a5c155 100644 --- a/mlir/include/mlir/Interfaces/ViewLikeInterface.td +++ b/mlir/include/mlir/Interfaces/ViewLikeInterface.td @@ -23,21 +23,45 @@ 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 start at the same "address". }], + /*retTy=*/"bool", + /*methodName=*/"isSameStart", + /*args=*/(ins), + /*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($_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 a84d10d5d609d..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 &visited, - SmallVectorImpl &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 &visited, bool maybeOffset, + SmallVectorImpl> &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 &visited, - SmallVectorImpl &output) { +static void collectUnderlyingAddressValues( + RegionBranchOpInterface branch, Region *region, Value inputValue, + unsigned inputIndex, unsigned maxDepth, DenseSet &visited, + bool maybeOffset, SmallVectorImpl> &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(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,33 +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 &visited, - SmallVectorImpl &output) { +static void collectUnderlyingAddressValues( + OpResult result, unsigned maxDepth, DenseSet &visited, + bool maybeOffset, SmallVectorImpl> &output) { Operation *op = result.getOwner(); // If this is a view, unwrap to the source. if (ViewLikeOpInterface view = dyn_cast(op)) { if (result == view.getViewDest()) { - 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(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 &visited, - SmallVectorImpl &output) { +static void collectUnderlyingAddressValues( + BlockArgument arg, unsigned maxDepth, DenseSet &visited, + bool maybeOffset, SmallVectorImpl> &output) { Block *block = arg.getOwner(); unsigned argNumber = arg.getArgNumber(); @@ -157,7 +160,7 @@ static void collectUnderlyingAddressValues(BlockArgument arg, unsigned maxDepth, auto branch = dyn_cast((*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; } @@ -166,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; } @@ -178,39 +182,40 @@ static void collectUnderlyingAddressValues(BlockArgument arg, unsigned maxDepth, Region *region = block->getParent(); Operation *op = region->getParentOp(); if (auto branch = dyn_cast(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 &visited, - SmallVectorImpl &output) { +static void collectUnderlyingAddressValues( + Value value, unsigned maxDepth, DenseSet &visited, bool maybeOffset, + SmallVectorImpl> &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(value)) - return collectUnderlyingAddressValues(arg, maxDepth, visited, output); + return collectUnderlyingAddressValues(arg, maxDepth, visited, maybeOffset, + output); collectUnderlyingAddressValues(cast(value), maxDepth, visited, - output); + maybeOffset, output); } /// Given a value, collect all of the underlying values being addressed. -static void collectUnderlyingAddressValues(Value value, - SmallVectorImpl &output) { +static void collectUnderlyingAddressValues( + Value value, SmallVectorImpl> &output) { DenseSet visited; collectUnderlyingAddressValues(value, maxUnderlyingValueSearchDepth, visited, - output); + /*maybeOffset=*/false, output); } //===----------------------------------------------------------------------===// @@ -371,7 +376,7 @@ AliasResult LocalAliasAnalysis::alias(Value lhs, Value rhs) { return AliasResult::MustAlias; // Get the underlying values being addressed. - SmallVector lhsValues, rhsValues; + SmallVector, 8> lhsValues, rhsValues; collectUnderlyingAddressValues(lhs, lhsValues); collectUnderlyingAddressValues(rhs, rhsValues); @@ -382,9 +387,14 @@ AliasResult LocalAliasAnalysis::alias(Value lhs, Value rhs) { // Check the alias results against each of the underlying values. std::optional 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 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, %arg1: memref {local_alias_analysis.restrict}) attributes {test.ptr = "func"} { %0 = memref.subview %arg[0][2][1] {test.ptr = "view1"} : memref to memref<2xf32> %1 = memref.subview %arg1[0][2][1] {test.ptr = "view2"} : memref to memref<2xf32> + %2 = memref.subview %arg[1][2][1] {test.ptr = "view3"} : memref to memref<2xf32, strided<[1], offset: 1>> + %3 = memref.subview %arg1[1][2][1] {test.ptr = "view4"} : memref 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, %arg1: memref) attribute %0, %1 = memref.distinct_objects %arg, %arg1 {test.ptr = "distinct"} : memref, memref 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) 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 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, %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 to memref<8xi8> + scf.yield %complete_view_if : memref<8xi8> + } else { + %view_with_offset = memref.view %arg[%c1][] {test.ptr = "view_with_offset"} : memref 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, %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 to memref<8xi8> + scf.yield %view_with_offset : memref<8xi8> + } else { + %complete_view_if = memref.view %arg[%c0][] {test.ptr = "complete_view_if"} : memref 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 +}