[flang-commits] [flang] [flang] AliasAnalysis: More formally define and distinguish between data and non-data (PR #91020)
via flang-commits
flang-commits at lists.llvm.org
Mon May 6 17:07:48 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-flang-fir-hlfir
Author: Renaud Kauffmann (Renaud-K)
<details>
<summary>Changes</summary>
This PR is an implementation for changes proposed in https://discourse.llvm.org/t/rfc-distinguish-between-data-and-non-data-in-fir-alias-analysis/78759
Test updates were made when the query was on the wrong reference. So, it is my hope that this will clear ambiguity on the nature of the queries from here on.
There are also some TODOs that were addressed.
It also partly implements what https://github.com/llvm/llvm-project/pull/87723 is attempting to accomplish. At least, on a point-to-point query between references, the distinction is made. To apply it to TBAA, would be another PR.
Note that, the changes were minimal in the TBAA code to retain the current results.
---
Patch is 29.71 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/91020.diff
6 Files Affected:
- (modified) flang/include/flang/Optimizer/Analysis/AliasAnalysis.h (+75-5)
- (modified) flang/lib/Optimizer/Analysis/AliasAnalysis.cpp (+63-90)
- (modified) flang/lib/Optimizer/Transforms/AddAliasTags.cpp (+10-7)
- (modified) flang/test/Analysis/AliasAnalysis/alias-analysis-2.fir (+24-13)
- (modified) flang/test/Analysis/AliasAnalysis/alias-analysis-3.fir (+2-2)
- (added) flang/test/Analysis/AliasAnalysis/alias-analysis-9.fir (+51)
``````````diff
diff --git a/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h b/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
index dfcafe88fee1b5..8a0eb525f19157 100644
--- a/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
+++ b/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
@@ -36,9 +36,6 @@ struct AliasAnalysis {
/// Represents memory allocated outside of a function
/// and passed to the function via host association tuple.
HostAssoc,
- /// Represents direct memory access whose source cannot be further
- /// determined
- Direct,
/// Represents memory allocated by unknown means and
/// with the memory address defined by a memory reading
/// operation (e.g. fir::LoadOp).
@@ -50,12 +47,70 @@ struct AliasAnalysis {
/// Attributes of the memory source object.
ENUM_CLASS(Attribute, Target, Pointer, IntentIn);
+ // See
+ // https://discourse.llvm.org/t/rfc-distinguish-between-data-and-non-data-in-fir-alias-analysis/78759/1
+ //
+ // It is possible, while following the source of a memory reference through
+ // the use-def chain, to arrive at the same origin, even though the starting
+ // points were known to not alias.
+ //
+ // Example:
+ // clang-format off
+ // fir.global @_QMtopEa : !fir.box<!fir.ptr<!fir.array<?xf32>>>
+ //
+ // func.func @_QPtest() {
+ // %c1 = arith.constant 1 : index
+ // %cst = arith.constant 1.000000e+00 : f32
+ // %0 = fir.address_of(@_QMtopEa) : !fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>
+ // %1 = fir.declare %0 {fortran_attrs = #fir.var_attrs<pointer>, uniq_name = "_QMtopEa"} : (!fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>) -> !fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>
+ // %2 = fir.load %1 : !fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>
+ // ...
+ // %5 = fir.array_coor %2 %c1 : (!fir.box<!fir.ptr<!fir.array<?xf32>>>, !fir.shift<1>, index) -> !fir.ref<f32>
+ // fir.store %cst to %5 : !fir.ref<f32>
+ // return
+ // }
+ //
+ // With high level operations, such as fir.array_coor, it is possible to
+ // reach into the data wrapped by the box (the descriptor) therefore when
+ // asking about the memory source of the %5, we are really asking about the
+ // source of the data of box %2.
+ //
+ // When asking about the source of %0 which is the address of the box, we
+ // reach the same source as in the first case: the global @_QMtopEa. Yet one
+ // source refers to the data while the other refers to the address of the box
+ // itself.
+ //
+ // To distinguish between the two, the isData flag has been added, whereby
+ // data is defined as any memory reference that is not a box reference.
+ // Additionally, because it is relied on in HLFIR lowering, we allow querying
+ // on a box SSA value, which is interpreted as querying on its data.
+ //
+ // So in the above example, !fir.ref<f32> and !fir.box<!fir.ptr<!fir.array<?xf32>>> is data,
+ // while !fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>> is not data.
+
+ // This generally applies to function arguments. In the example below, %arg0
+ // is data, %arg1 is not data but a load of %arg1 is.
+ //
+ // func.func @_QFPtest2(%arg0: !fir.ref<f32>, %arg1: !fir.ref<!fir.box<!fir.ptr<f32>>> ) {
+ // %0 = fir.load %arg1 : !fir.ref<!fir.box<!fir.ptr<f32>>>
+ // ... }
+ //
+ // clang-format on
+
struct Source {
using SourceUnion = llvm::PointerUnion<mlir::SymbolRefAttr, mlir::Value>;
using Attributes = Fortran::common::EnumSet<Attribute, Attribute_enumSize>;
- /// Source definition of a value.
- SourceUnion u;
+ struct SourceOrigin {
+ /// Source definition of a value.
+ SourceUnion u;
+
+ /// Whether the source was reached following data or box reference
+ bool isData{false};
+ };
+
+ SourceOrigin origin;
+
/// Kind of the memory source.
SourceKind kind;
/// Value type of the source definition.
@@ -77,6 +132,12 @@ struct AliasAnalysis {
/// attribute.
bool isRecordWithPointerComponent() const;
+ bool isDummyArgument() const;
+ bool isData() const;
+ bool isBoxData() const;
+
+ mlir::Type getType() const;
+
/// Return true, if `ty` is a reference type to a boxed
/// POINTER object or a raw fir::PointerType.
static bool isPointerReference(mlir::Type ty);
@@ -95,6 +156,15 @@ struct AliasAnalysis {
Source getSource(mlir::Value);
};
+inline bool operator==(const AliasAnalysis::Source::SourceOrigin &lhs,
+ const AliasAnalysis::Source::SourceOrigin &rhs) {
+ return lhs.u == rhs.u && lhs.isData == rhs.isData;
+}
+inline bool operator!=(const AliasAnalysis::Source::SourceOrigin &lhs,
+ const AliasAnalysis::Source::SourceOrigin &rhs) {
+ return !(lhs == rhs);
+}
+
inline llvm::raw_ostream &operator<<(llvm::raw_ostream &os,
const AliasAnalysis::Source &op) {
op.print(os);
diff --git a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
index f723e8f66e3e4b..ade240bc2364e4 100644
--- a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
+++ b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
@@ -33,7 +33,9 @@ static bool isDummyArgument(mlir::Value v) {
if (!blockArg)
return false;
- return blockArg.getOwner()->isEntryBlock();
+ auto *owner{blockArg.getOwner()};
+ return owner->isEntryBlock() &&
+ mlir::isa<mlir::FunctionOpInterface>(owner->getParentOp());
}
/// Temporary function to skip through all the no op operations
@@ -54,12 +56,17 @@ static mlir::Value getOriginalDef(mlir::Value v) {
namespace fir {
void AliasAnalysis::Source::print(llvm::raw_ostream &os) const {
- if (auto v = llvm::dyn_cast<mlir::Value>(u))
+ if (auto v = llvm::dyn_cast<mlir::Value>(origin.u))
os << v;
- else if (auto gbl = llvm::dyn_cast<mlir::SymbolRefAttr>(u))
+ else if (auto gbl = llvm::dyn_cast<mlir::SymbolRefAttr>(origin.u))
os << gbl;
os << " SourceKind: " << EnumToString(kind);
os << " Type: " << valueType << " ";
+ if (origin.isData) {
+ os << " following data ";
+ } else {
+ os << " following box reference ";
+ }
attributes.Dump(os, EnumToString);
}
@@ -76,6 +83,19 @@ bool AliasAnalysis::Source::isTargetOrPointer() const {
attributes.test(Attribute::Target);
}
+bool AliasAnalysis::Source::isDummyArgument() const {
+ if (auto v = origin.u.dyn_cast<mlir::Value>()) {
+ return ::isDummyArgument(v);
+ }
+ return false;
+}
+
+bool AliasAnalysis::Source::isData() const { return origin.isData; }
+bool AliasAnalysis::Source::isBoxData() const {
+ return mlir::isa<fir::BaseBoxType>(fir::unwrapRefType(valueType)) &&
+ origin.isData;
+}
+
bool AliasAnalysis::Source::isRecordWithPointerComponent() const {
auto eleTy = fir::dyn_cast_ptrEleTy(valueType);
if (!eleTy)
@@ -88,70 +108,35 @@ AliasResult AliasAnalysis::alias(Value lhs, Value rhs) {
auto lhsSrc = getSource(lhs);
auto rhsSrc = getSource(rhs);
bool approximateSource = lhsSrc.approximateSource || rhsSrc.approximateSource;
- LLVM_DEBUG(llvm::dbgs() << "AliasAnalysis::alias\n";
+ LLVM_DEBUG(llvm::dbgs() << "\n"; llvm::dbgs() << "AliasAnalysis::alias\n";
llvm::dbgs() << " lhs: " << lhs << "\n";
llvm::dbgs() << " lhsSrc: " << lhsSrc << "\n";
llvm::dbgs() << " rhs: " << rhs << "\n";
- llvm::dbgs() << " rhsSrc: " << rhsSrc << "\n";
- llvm::dbgs() << "\n";);
+ llvm::dbgs() << " rhsSrc: " << rhsSrc << "\n";);
// Indirect case currently not handled. Conservatively assume
// it aliases with everything
- if (lhsSrc.kind > SourceKind::Direct || rhsSrc.kind > SourceKind::Direct) {
+ if (lhsSrc.kind >= SourceKind::Indirect ||
+ rhsSrc.kind >= SourceKind::Indirect) {
return AliasResult::MayAlias;
}
- // SourceKind::Direct is set for the addresses wrapped in a global boxes.
- // ie: fir.global @_QMpointersEp : !fir.box<!fir.ptr<f32>>
- // Though nothing is known about them, they would only alias with targets or
- // pointers
- bool directSourceToNonTargetOrPointer = false;
- if (lhsSrc.u != rhsSrc.u || lhsSrc.kind != rhsSrc.kind) {
- if ((lhsSrc.kind == SourceKind::Direct && !rhsSrc.isTargetOrPointer()) ||
- (rhsSrc.kind == SourceKind::Direct && !lhsSrc.isTargetOrPointer()))
- directSourceToNonTargetOrPointer = true;
- }
-
- if (lhsSrc.kind == SourceKind::Direct ||
- rhsSrc.kind == SourceKind::Direct) {
- if (!directSourceToNonTargetOrPointer)
- return AliasResult::MayAlias;
- }
-
if (lhsSrc.kind == rhsSrc.kind) {
- if (lhsSrc.u == rhsSrc.u) {
+ if (lhsSrc.origin == rhsSrc.origin) {
+ LLVM_DEBUG(llvm::dbgs()
+ << " aliasing because same source kind and origin\n");
if (approximateSource)
return AliasResult::MayAlias;
return AliasResult::MustAlias;
}
// Two host associated accesses may overlap due to an equivalence.
- if (lhsSrc.kind == SourceKind::HostAssoc)
- return AliasResult::MayAlias;
-
- // Allocate and global memory address cannot physically alias
- if (lhsSrc.kind == SourceKind::Allocate ||
- lhsSrc.kind == SourceKind::Global)
- return AliasResult::NoAlias;
-
- // Dummy TARGET/POINTER arguments may alias.
- if (lhsSrc.isTargetOrPointer() && rhsSrc.isTargetOrPointer())
- return AliasResult::MayAlias;
-
- // Box for POINTER component inside an object of a derived type
- // may alias box of a POINTER object, as well as boxes for POINTER
- // components inside two objects of derived types may alias.
- if ((lhsSrc.isRecordWithPointerComponent() && rhsSrc.isTargetOrPointer()) ||
- (rhsSrc.isRecordWithPointerComponent() && lhsSrc.isTargetOrPointer()) ||
- (lhsSrc.isRecordWithPointerComponent() &&
- rhsSrc.isRecordWithPointerComponent()))
+ if (lhsSrc.kind == SourceKind::HostAssoc) {
+ LLVM_DEBUG(llvm::dbgs() << " aliasing because of host association\n");
return AliasResult::MayAlias;
-
- return AliasResult::NoAlias;
+ }
}
- assert(lhsSrc.kind != rhsSrc.kind && "memory source kinds must be different");
-
Source *src1, *src2;
if (lhsSrc.kind < rhsSrc.kind) {
src1 = &lhsSrc;
@@ -182,8 +167,11 @@ AliasResult AliasAnalysis::alias(Value lhs, Value rhs) {
}
// Dummy TARGET/POINTER argument may alias with a global TARGET/POINTER.
- if (src1->isTargetOrPointer() && src2->isTargetOrPointer())
+ if (src1->isTargetOrPointer() && src2->isTargetOrPointer() &&
+ src1->isData() == src2->isData()) {
+ LLVM_DEBUG(llvm::dbgs() << " aliasing because of target or pointer\n");
return AliasResult::MayAlias;
+ }
// Box for POINTER component inside an object of a derived type
// may alias box of a POINTER object, as well as boxes for POINTER
@@ -191,8 +179,10 @@ AliasResult AliasAnalysis::alias(Value lhs, Value rhs) {
if ((src1->isRecordWithPointerComponent() && src2->isTargetOrPointer()) ||
(src2->isRecordWithPointerComponent() && src1->isTargetOrPointer()) ||
(src1->isRecordWithPointerComponent() &&
- src2->isRecordWithPointerComponent()))
+ src2->isRecordWithPointerComponent())) {
+ LLVM_DEBUG(llvm::dbgs() << " aliasing because of pointer components\n");
return AliasResult::MayAlias;
+ }
return AliasResult::NoAlias;
}
@@ -258,7 +248,10 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v) {
mlir::Type ty;
bool breakFromLoop{false};
bool approximateSource{false};
- bool followBoxAddr{mlir::isa<fir::BaseBoxType>(v.getType())};
+ bool followBoxData{mlir::isa<fir::BaseBoxType>(v.getType())};
+ bool isBoxRef{fir::isa_ref_type(v.getType()) &&
+ mlir::isa<fir::BaseBoxType>(fir::unwrapRefType(v.getType()))};
+ bool followingData = !isBoxRef || followBoxData;
mlir::SymbolRefAttr global;
Source::Attributes attributes;
while (defOp && !breakFromLoop) {
@@ -278,34 +271,32 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v) {
v = op->getOperand(0);
defOp = v.getDefiningOp();
if (mlir::isa<fir::BaseBoxType>(v.getType()))
- followBoxAddr = true;
+ followBoxData = true;
})
.Case<fir::ArrayCoorOp, fir::CoordinateOp>([&](auto op) {
v = op->getOperand(0);
defOp = v.getDefiningOp();
if (mlir::isa<fir::BaseBoxType>(v.getType()))
- followBoxAddr = true;
+ followBoxData = true;
approximateSource = true;
})
.Case<fir::EmboxOp, fir::ReboxOp>([&](auto op) {
- if (followBoxAddr) {
+ if (followBoxData) {
v = op->getOperand(0);
defOp = v.getDefiningOp();
} else
breakFromLoop = true;
})
.Case<fir::LoadOp>([&](auto op) {
- if (followBoxAddr && mlir::isa<fir::BaseBoxType>(op.getType())) {
- // For now, support the load of an argument or fir.address_of
- // TODO: generalize to all operations (in particular fir.alloca and
- // fir.allocmem)
- auto def = getOriginalDef(op.getMemref());
- if (isDummyArgument(def) ||
- def.template getDefiningOp<fir::AddrOfOp>()) {
- v = def;
- defOp = v.getDefiningOp();
- return;
- }
+ // If the load is from a leaf source, return the leaf. Do not track
+ // through indirections otherwise.
+ // TODO: At support to fir.alloca and fir.allocmem
+ auto def = getOriginalDef(op.getMemref());
+ if (isDummyArgument(def) ||
+ def.template getDefiningOp<fir::AddrOfOp>()) {
+ v = def;
+ defOp = v.getDefiningOp();
+ return;
}
// No further tracking for addresses loaded from memory for now.
type = SourceKind::Indirect;
@@ -314,24 +305,7 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v) {
.Case<fir::AddrOfOp>([&](auto op) {
// Address of a global scope object.
ty = v.getType();
-
- // When the global is a
- // fir.global @_QMpointersEp : !fir.box<!fir.ptr<f32>>
- // or
- // fir.global @_QMpointersEp : !fir.box<!fir.heap<f32>>
- //
- // and when following through the wrapped address, capture
- // the fact that there is nothing known about it. Therefore setting
- // the source to Direct.
- //
- // When not following the wrapped address, then consider the address
- // of the box, which has nothing to do with the wrapped address and
- // lies in the global memory space.
- if (followBoxAddr &&
- mlir::isa<fir::BaseBoxType>(fir::unwrapRefType(ty)))
- type = SourceKind::Direct;
- else
- type = SourceKind::Global;
+ type = SourceKind::Global;
auto globalOpName = mlir::OperationName(
fir::GlobalOp::getOperationName(), defOp->getContext());
@@ -339,11 +313,10 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v) {
v, fir::GlobalOp::getTargetAttrName(globalOpName)))
attributes.set(Attribute::Target);
- // TODO: Take followBoxAddr into account when setting the pointer
+ // TODO: Take followBoxData into account when setting the pointer
// attribute
if (Source::isPointerReference(ty))
attributes.set(Attribute::Pointer);
-
global = llvm::cast<fir::AddrOfOp>(op).getSymbol();
breakFromLoop = true;
})
@@ -389,7 +362,7 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v) {
// MustAlias after going through a designate operation
approximateSource = true;
if (mlir::isa<fir::BaseBoxType>(v.getType()))
- followBoxAddr = true;
+ followBoxData = true;
})
.Default([&](auto op) {
defOp = nullptr;
@@ -408,10 +381,10 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v) {
attributes.set(Attribute::Pointer);
}
- if (type == SourceKind::Global || type == SourceKind::Direct)
- return {global, type, ty, attributes, approximateSource};
-
- return {v, type, ty, attributes, approximateSource};
+ if (type == SourceKind::Global) {
+ return {{global, followingData}, type, ty, attributes, approximateSource};
+ }
+ return {{v, followingData}, type, ty, attributes, approximateSource};
}
} // namespace fir
diff --git a/flang/lib/Optimizer/Transforms/AddAliasTags.cpp b/flang/lib/Optimizer/Transforms/AddAliasTags.cpp
index 684aa4462915e5..3884c370b877b9 100644
--- a/flang/lib/Optimizer/Transforms/AddAliasTags.cpp
+++ b/flang/lib/Optimizer/Transforms/AddAliasTags.cpp
@@ -144,7 +144,7 @@ void AddAliasTagsPass::runOnAliasInterface(fir::FirAliasTagOpInterface op,
source.kind == fir::AliasAnalysis::SourceKind::Argument) {
LLVM_DEBUG(llvm::dbgs().indent(2)
<< "Found reference to dummy argument at " << *op << "\n");
- std::string name = getFuncArgName(source.u.get<mlir::Value>());
+ std::string name = getFuncArgName(source.origin.u.get<mlir::Value>());
if (!name.empty())
tag = state.getFuncTree(func).dummyArgDataTree.getTag(name);
else
@@ -154,8 +154,9 @@ void AddAliasTagsPass::runOnAliasInterface(fir::FirAliasTagOpInterface op,
// TBAA for global variables
} else if (enableGlobals &&
- source.kind == fir::AliasAnalysis::SourceKind::Global) {
- mlir::SymbolRefAttr glbl = source.u.get<mlir::SymbolRefAttr>();
+ source.kind == fir::AliasAnalysis::SourceKind::Global &&
+ !source.isBoxData()) {
+ mlir::SymbolRefAttr glbl = source.origin.u.get<mlir::SymbolRefAttr>();
const char *name = glbl.getRootReference().data();
LLVM_DEBUG(llvm::dbgs().indent(2) << "Found reference to global " << name
<< " at " << *op << "\n");
@@ -163,9 +164,10 @@ void AddAliasTagsPass::runOnAliasInterface(fir::FirAliasTagOpInterface op,
// TBAA for SourceKind::Direct
} else if (enableDirect &&
- source.kind == fir::AliasAnalysis::SourceKind::Direct) {
- if (source.u.is<mlir::SymbolRefAttr>()) {
- mlir::SymbolRefAttr glbl = source.u.get<mlir::SymbolRefAttr>();
+ source.kind == fir::AliasAnalysis::SourceKind::Global &&
+ source.isBoxData()) {
+ if (source.origin.u.is<mlir::SymbolRefAttr>()) {
+ mlir::SymbolRefAttr glbl = source.origin.u.get<mlir::SymbolRefAttr>();
const char *name = glbl.getRootReference().data();
LLVM_DEBUG(llvm::dbgs().indent(2) << "Found reference to direct " << name
<< " at " << *op << "\n");
@@ -181,7 +183,8 @@ void AddAliasTagsPass::runOnAliasInterface(fir::FirAliasTagOpInterface op,
} else if (enableLocalAllocs &&
source.kind == fir::AliasAnalysis::SourceKind::Allocate) {
std::optional<llvm::StringRef> name;
- mlir::Operation *sourceOp = source.u.get<mlir::Value>().getDefiningOp();
+ mlir::Operation *sourceOp =
+ source.origin.u.get<mlir::Value>().getDefiningOp();
if (auto alloc = mlir::dyn_cast_or_null<fir::AllocaOp>(sourceOp))
name = alloc.getUniqName();
else if (auto alloc = mlir::dyn_cast_or_null<fir::AllocMemOp>(sourceOp))
diff --git a/flang/test/Analysis/AliasAnalysis/alias-analysis-2.fir b/flang/test/Analysis/AliasAnalysis/alias-analysis-2.fir
index 31459ef21d947c..af3e491e062c3e 100644
--- a/flang/test/Analysis/AliasAnalysis/alias-analysis-2.fir
+++ b/flang/test/Analysis/AliasAnalysis/alias-analysis-2.fir
@@ -37,14 +37,14 @@
// arg2 is a reference to a pointer. Modifying arg2 could
// modify a target with a pointer component
-// CHECK-DAG: func.region0#0 <-> func.region0#2: MayAlias
-// CHECK-DAG: func.region0#1 <-> func.region0#2: MayAlias
+// CHECK-DAG: arg2.load#0 <-> func.region0#0: MayAlias
+// CHECK-DAG: arg2.load#0 <-> func.region0#1: MayAlias
// However, the address wrapped by arg2, can alias with any target or
// pointer arguments
// CHECK-DAG: arg2.addr#0 <-> func....
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/91020
More information about the flang-commits
mailing list