[flang-commits] [flang] [flang] AliasAnalysis: More formally define and distinguish between data and non-data (PR #91020)
Renaud Kauffmann via flang-commits
flang-commits at lists.llvm.org
Wed May 15 14:13:14 PDT 2024
https://github.com/Renaud-K updated https://github.com/llvm/llvm-project/pull/91020
>From 2343b8cd03ee743fc937a86d9501462652f262d9 Mon Sep 17 00:00:00 2001
From: Renaud-K <rkauffmann at nvidia.com>
Date: Fri, 3 May 2024 12:46:49 -0700
Subject: [PATCH 1/7] Remove SourceKind::Direct. Include data/non-data flang in
source origin
---
.../flang/Optimizer/Analysis/AliasAnalysis.h | 31 ++++-
.../lib/Optimizer/Analysis/AliasAnalysis.cpp | 109 ++++++++----------
.../lib/Optimizer/Transforms/AddAliasTags.cpp | 15 ++-
.../AliasAnalysis/alias-analysis-2.fir | 37 +++---
.../AliasAnalysis/alias-analysis-3.fir | 4 +-
5 files changed, 109 insertions(+), 87 deletions(-)
diff --git a/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h b/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
index dfcafe88fee1b..b11ba229cf07e 100644
--- a/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
+++ b/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
@@ -38,10 +38,6 @@ struct AliasAnalysis {
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).
Indirect,
/// Starting point to the analysis whereby nothing is known about
/// the source
@@ -53,9 +49,17 @@ struct AliasAnalysis {
struct Source {
using SourceUnion = llvm::PointerUnion<mlir::SymbolRefAttr, mlir::Value>;
using Attributes = Fortran::common::EnumSet<Attribute, Attribute_enumSize>;
+
+ struct SourceOrigin {
+ /// Source definition of a value.
+ SourceUnion u;
+
+ /// Whether the source was reached following data or box reference
+ bool isData{false};
+ };
+
+ SourceOrigin origin;
- /// Source definition of a value.
- SourceUnion u;
/// Kind of the memory source.
SourceKind kind;
/// Value type of the source definition.
@@ -77,6 +81,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 +105,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 f723e8f66e3e4..4060ae649730b 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)
@@ -97,29 +117,20 @@ AliasResult AliasAnalysis::alias(Value lhs, Value rhs) {
// 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 we have reached the same source but comparing box reference against
+ // data we are not comparing apples-to-apples. The 2 cannot alias.
+ if ((lhsSrc.origin.u == rhsSrc.origin.u) &&
+ lhsSrc.isData() != rhsSrc.isData()) {
+ return AliasResult::NoAlias;
}
if (lhsSrc.kind == rhsSrc.kind) {
- if (lhsSrc.u == rhsSrc.u) {
+ if (lhsSrc.origin == rhsSrc.origin) {
if (approximateSource)
return AliasResult::MayAlias;
return AliasResult::MustAlias;
@@ -129,13 +140,9 @@ AliasResult AliasAnalysis::alias(Value lhs, Value rhs) {
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())
+ // TARGET/POINTER arguments may alias.
+ if (lhsSrc.isTargetOrPointer() && rhsSrc.isTargetOrPointer() &&
+ lhsSrc.isData() == rhsSrc.isData())
return AliasResult::MayAlias;
// Box for POINTER component inside an object of a derived type
@@ -182,7 +189,8 @@ 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())
return AliasResult::MayAlias;
// Box for POINTER component inside an object of a derived type
@@ -258,7 +266,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,24 +289,24 @@ 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())) {
+ if (followBoxData && 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)
@@ -314,24 +325,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 +333,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 +382,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 +401,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 684aa4462915e..f8e88b0495579 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,18 +154,17 @@ 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");
tag = state.getFuncTree(func).globalDataTree.getTag(name);
// 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>();
+ } else if (enableDirect && 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 +180,7 @@ 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 31459ef21d947..af3e491e062c3 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.region0#0: MayAlias
// CHECK-DAG: arg2.addr#0 <-> func.region0#1: MayAlias
-// CHECK-DAG: arg2.addr#0 <-> func.region0#2: MustAlias
+// CHECK-DAG: arg2.load#0 <-> arg2.addr#0: MustAlias
// CHECK-DAG: boxp1.addr#0 <-> arg2.addr#0: MayAlias
func.func @_QFPtest(%arg0: !fir.ref<f32> {fir.bindc_name = "v1", fir.target}, %arg1: !fir.ref<f32> {fir.bindc_name = "v2", fir.target}, %arg2: !fir.ref<!fir.box<!fir.ptr<f32>>> ) attributes {test.ptr = "func"} {
@@ -80,7 +80,7 @@ func.func @_QFPtest(%arg0: !fir.ref<f32> {fir.bindc_name = "v1", fir.target}, %a
%14 = fir.box_addr %13 : (!fir.box<!fir.ptr<f32>>) -> !fir.ptr<f32>
fir.store %14 to %4 : !fir.ref<!fir.ptr<f32>>
- %15 = fir.load %arg2 : !fir.ref<!fir.box<!fir.ptr<f32>>>
+ %15 = fir.load %arg2 {test.ptr = "arg2.load"} : !fir.ref<!fir.box<!fir.ptr<f32>>>
%16 = fir.box_addr %15 {test.ptr = "arg2.addr"} : (!fir.box<!fir.ptr<f32>>) -> !fir.ptr<f32>
return
}
@@ -99,10 +99,12 @@ func.func @_QFPtest(%arg0: !fir.ref<f32> {fir.bindc_name = "v1", fir.target}, %a
// CHECK-DAG: func.region0#1 <-> func.region0#2: MayAlias
// They can also modify targets that have pointer components
-// CHECK-DAG: func.region0#0 <-> func.region0#1: MayAlias
-// CHECK-DAG: func.region0#0 <-> func.region0#2: MayAlias
+// CHECK-DAG: arg1.load#0 <-> func.region0#0: MayAlias
+// CHECK-DAG: arg2.load#0 <-> func.region0#0: MayAlias
func.func @_QFPtest2(%arg0: !fir.ref<f32> {fir.bindc_name = "v1", fir.target}, %arg1: !fir.ref<!fir.box<!fir.ptr<f32>>>, %arg2: !fir.ref<!fir.box<!fir.ptr<f32>>> ) attributes {test.ptr = "func"} {
+ %0 = fir.load %arg1 {test.ptr = "arg1.load"} : !fir.ref<!fir.box<!fir.ptr<f32>>>
+ %1 = fir.load %arg2 {test.ptr = "arg2.load"} : !fir.ref<!fir.box<!fir.ptr<f32>>>
return
}
@@ -141,14 +143,14 @@ func.func @_QFPtest2(%arg0: !fir.ref<f32> {fir.bindc_name = "v1", fir.target}, %
// alias with the wrapped scalar _QFEvar2. We meant box_addr of _QMpointersEp
// CHECK-DAG: p#0 <-> box.addr#0: NoAlias
-// TODO: Still need to handle more gracefully the difference between !fir.ref<!fir.box<>> and !fir.box<>
-// CHECK-DAG: box.addr#0 <-> func.region0#0: MayAlias
+// Now handling more gracefully the difference between !fir.ref<!fir.box<>> and !fir.box<>
+// CHECK-DAG: box.addr#0 <-> func.region0#0: NoAlias
// var2, although it is a target, cannot alias with p
// A modification of p would only make them point to a new target but not modify it
// CHECK-DAG: var2#0 <-> p#0: NoAlias
// It can alias with p1, if p1 is a pointer component
-// CHECK-DAG: var2#0 <-> func.region0#0: MayAlias
+// CHECK-DAG: arg0.load#0 <-> var2#0: MayAlias
// It is the same as box.addr
// CHECK-DAG: var2#0 <-> box.addr#0: MustAlias
@@ -173,6 +175,7 @@ fir.global internal @_QFEvar2 target : f32 {
}
func.func @_QFPtest3(%arg0: !fir.ref<!fir.box<!fir.ptr<f32>>> {fir.bindc_name = "p1"}, %arg1: !fir.ref<f32>) attributes {test.ptr = "func"} {
+ %3 = fir.load %arg0 {test.ptr = "arg0.load"}: !fir.ref<!fir.box<!fir.ptr<f32>>>
%4 = fir.address_of(@_QFEvar2) {test.ptr = "var2"} : !fir.ref<f32>
%5 = fir.address_of(@_QMpointersEp) {test.ptr = "p"} : !fir.ref<!fir.box<!fir.ptr<f32>>>
%6 = fir.embox %4 : (!fir.ref<f32>) -> !fir.box<!fir.ptr<f32>>
@@ -214,10 +217,17 @@ func.func @_QFPtest3(%arg0: !fir.ref<!fir.box<!fir.ptr<f32>>> {fir.bindc_name =
// CHECK-DAG: var2_hlfir#1 <-> p_hlfir#0: NoAlias
// CHECK-DAG: var2_hlfir#1 <-> p_hlfir#1: NoAlias
-// CHECK-DAG: var2#0 <-> func.region0#0: MayAlias
-// CHECK-DAG: var2_fir#0 <-> func.region0#0: MayAlias
-// CHECK-DAG: var2_hlfir#0 <-> func.region0#0: MayAlias
-// CHECK-DAG: var2_hlfir#1 <-> func.region0#0: MayAlias
+// The data cannot alias with the box references
+// CHECK-DAG: var2#0 <-> func.region0#0: NoAlias
+// CHECK-DAG: var2_fir#0 <-> func.region0#0: NoAlias
+// CHECK-DAG: var2_hlfir#0 <-> func.region0#0: NoAlias
+// CHECK-DAG: var2_hlfir#1 <-> func.region0#0: NoAlias
+
+// But it can alias with the box's own data
+// CHECK-DAG: arg0.load#0 <-> var2#0: MayAlias
+// CHECK-DAG: arg0.load#0 <-> var2_fir#0: MayAlias
+// CHECK-DAG: arg0.load#0 <-> var2_hlfir#0: MayAlias
+// CHECK-DAG: arg0.load#0 <-> var2_hlfir#1: MayAlias
// CHECK-DAG: var2#0 <-> box.addr#0: MustAlias
// CHECK-DAG: var2#0 <-> box.addr_fir#0: MustAlias
@@ -255,6 +265,7 @@ fir.global internal @_QFEvar2 target : f32 {
}
func.func @_QFPtest4(%arg0: !fir.ref<!fir.box<!fir.ptr<f32>>> {fir.bindc_name = "p1"}, %arg1: !fir.ref<f32>) attributes {test.ptr = "func"} {
+ %3 = fir.load %arg0 {test.ptr = "arg0.load"} : !fir.ref<!fir.box<!fir.ptr<f32>>>
%4 = fir.address_of(@_QFEvar2) {test.ptr = "var2"} : !fir.ref<f32>
%fir_decl_var2 = fir.declare %4 {uniq_name = "var2_fir", test.ptr = "var2_fir"}: (!fir.ref<f32>) -> !fir.ref<f32>
%hlfir_decl_var2:2 = hlfir.declare %4 {uniq_name = "var2_hlfir", test.ptr = "var2_hlfir"}: (!fir.ref<f32>) -> (!fir.ref<f32>, !fir.ref<f32>)
diff --git a/flang/test/Analysis/AliasAnalysis/alias-analysis-3.fir b/flang/test/Analysis/AliasAnalysis/alias-analysis-3.fir
index d7bd970b51907..3d576cb5c0af9 100644
--- a/flang/test/Analysis/AliasAnalysis/alias-analysis-3.fir
+++ b/flang/test/Analysis/AliasAnalysis/alias-analysis-3.fir
@@ -23,8 +23,8 @@
// FIXME: a's box cannot alias with raw reference to f32 (x), so MayAlias below must be NoAlias:
// CHECK: a#0 <-> func.region0#1: MayAlias
-// FIXME: pointer_dummy's box cannot alias with raw reference to f32 (x), so MayAlias below must be NoAlias:
-// CHECK: func.region0#0 <-> func.region0#1: MayAlias
+// pointer_dummy's box cannot alias with raw reference to f32 (x), so MayAlias below must be NoAlias:
+// CHECK: func.region0#0 <-> func.region0#1: NoAlias
fir.global @_QMmEa : !fir.type<_QMmTt{pointer_component:!fir.box<!fir.ptr<f32>>}> {
%0 = fir.undefined !fir.type<_QMmTt{pointer_component:!fir.box<!fir.ptr<f32>>}>
>From 458ced53fd959b1afeac5a20bd054845387d2488 Mon Sep 17 00:00:00 2001
From: Renaud-K <rkauffmann at nvidia.com>
Date: Fri, 3 May 2024 15:28:44 -0700
Subject: [PATCH 2/7] Applied formatting
---
flang/include/flang/Optimizer/Analysis/AliasAnalysis.h | 2 +-
flang/lib/Optimizer/Transforms/AddAliasTags.cpp | 6 ++++--
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h b/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
index b11ba229cf07e..64835ace68955 100644
--- a/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
+++ b/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
@@ -49,7 +49,7 @@ struct AliasAnalysis {
struct Source {
using SourceUnion = llvm::PointerUnion<mlir::SymbolRefAttr, mlir::Value>;
using Attributes = Fortran::common::EnumSet<Attribute, Attribute_enumSize>;
-
+
struct SourceOrigin {
/// Source definition of a value.
SourceUnion u;
diff --git a/flang/lib/Optimizer/Transforms/AddAliasTags.cpp b/flang/lib/Optimizer/Transforms/AddAliasTags.cpp
index f8e88b0495579..59c84920a1e18 100644
--- a/flang/lib/Optimizer/Transforms/AddAliasTags.cpp
+++ b/flang/lib/Optimizer/Transforms/AddAliasTags.cpp
@@ -154,7 +154,8 @@ void AddAliasTagsPass::runOnAliasInterface(fir::FirAliasTagOpInterface op,
// TBAA for global variables
} else if (enableGlobals &&
- source.kind == fir::AliasAnalysis::SourceKind::Global && !source.isBoxData()) {
+ 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
@@ -180,7 +181,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.origin.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))
>From fe4b1294834b8249220b3be9324fe570c076210b Mon Sep 17 00:00:00 2001
From: Renaud-K <rkauffmann at nvidia.com>
Date: Fri, 3 May 2024 17:05:02 -0700
Subject: [PATCH 3/7] Restoring previous behavior on enableDirect
---
flang/lib/Optimizer/Transforms/AddAliasTags.cpp | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/flang/lib/Optimizer/Transforms/AddAliasTags.cpp b/flang/lib/Optimizer/Transforms/AddAliasTags.cpp
index 59c84920a1e18..3884c370b877b 100644
--- a/flang/lib/Optimizer/Transforms/AddAliasTags.cpp
+++ b/flang/lib/Optimizer/Transforms/AddAliasTags.cpp
@@ -163,7 +163,9 @@ void AddAliasTagsPass::runOnAliasInterface(fir::FirAliasTagOpInterface op,
tag = state.getFuncTree(func).globalDataTree.getTag(name);
// TBAA for SourceKind::Direct
- } else if (enableDirect && source.isBoxData()) {
+ } else if (enableDirect &&
+ 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();
>From 5216e462d09223eb4acc5edf579a06da0e182e6a Mon Sep 17 00:00:00 2001
From: Renaud-K <rkauffmann at nvidia.com>
Date: Mon, 6 May 2024 17:06:52 -0700
Subject: [PATCH 4/7] Applying review feedback and additional clean-ups
---
.../flang/Optimizer/Analysis/AliasAnalysis.h | 55 +++++++++++++++-
.../lib/Optimizer/Analysis/AliasAnalysis.cpp | 64 +++++++------------
.../AliasAnalysis/alias-analysis-9.fir | 51 +++++++++++++++
3 files changed, 126 insertions(+), 44 deletions(-)
create mode 100644 flang/test/Analysis/AliasAnalysis/alias-analysis-9.fir
diff --git a/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h b/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
index 64835ace68955..8a0eb525f1915 100644
--- a/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
+++ b/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
@@ -36,8 +36,9 @@ 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
+ /// Represents memory allocated by unknown means and
+ /// with the memory address defined by a memory reading
+ /// operation (e.g. fir::LoadOp).
Indirect,
/// Starting point to the analysis whereby nothing is known about
/// the source
@@ -46,6 +47,56 @@ 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>;
diff --git a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
index 4060ae649730b..ade240bc2364e 100644
--- a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
+++ b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
@@ -108,12 +108,11 @@ 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
@@ -122,43 +121,22 @@ AliasResult AliasAnalysis::alias(Value lhs, Value rhs) {
return AliasResult::MayAlias;
}
- // If we have reached the same source but comparing box reference against
- // data we are not comparing apples-to-apples. The 2 cannot alias.
- if ((lhsSrc.origin.u == rhsSrc.origin.u) &&
- lhsSrc.isData() != rhsSrc.isData()) {
- return AliasResult::NoAlias;
- }
-
if (lhsSrc.kind == rhsSrc.kind) {
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;
-
- // TARGET/POINTER arguments may alias.
- if (lhsSrc.isTargetOrPointer() && rhsSrc.isTargetOrPointer() &&
- lhsSrc.isData() == rhsSrc.isData())
+ if (lhsSrc.kind == SourceKind::HostAssoc) {
+ LLVM_DEBUG(llvm::dbgs() << " aliasing because of host association\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
- // components inside two objects of derived types may alias.
- if ((lhsSrc.isRecordWithPointerComponent() && rhsSrc.isTargetOrPointer()) ||
- (rhsSrc.isRecordWithPointerComponent() && lhsSrc.isTargetOrPointer()) ||
- (lhsSrc.isRecordWithPointerComponent() &&
- rhsSrc.isRecordWithPointerComponent()))
- 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;
@@ -190,8 +168,10 @@ AliasResult AliasAnalysis::alias(Value lhs, Value rhs) {
// Dummy TARGET/POINTER argument may alias with a global TARGET/POINTER.
if (src1->isTargetOrPointer() && src2->isTargetOrPointer() &&
- src1->isData() == src2->isData())
+ 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
@@ -199,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;
}
@@ -306,17 +288,15 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v) {
breakFromLoop = true;
})
.Case<fir::LoadOp>([&](auto op) {
- if (followBoxData && 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;
diff --git a/flang/test/Analysis/AliasAnalysis/alias-analysis-9.fir b/flang/test/Analysis/AliasAnalysis/alias-analysis-9.fir
new file mode 100644
index 0000000000000..2b92872a2c4b5
--- /dev/null
+++ b/flang/test/Analysis/AliasAnalysis/alias-analysis-9.fir
@@ -0,0 +1,51 @@
+// RUN: fir-opt %s -pass-pipeline='builtin.module(func.func(test-fir-alias-analysis))' 2>&1 | FileCheck %s
+
+
+// module m
+// type t
+// type(t), pointer :: next
+// integer :: i
+// end type
+// contains
+// subroutine foo(x, y)
+// type(t) :: x, y
+// integer :: i1, i2
+// i1 = x%next%i
+// x = y
+// i2 = x%next%i
+// end subroutine
+// end module
+
+// CHECK-LABEL: Testing : "_QMmPfoo"
+// TODO: x and y do not alias
+// CHECK-DAG: x#0 <-> y#0: MayAlias
+// CHECK-DAG: y#0 <-> xnext1#0: MayAlias
+// CHECK-DAG: y#0 <-> xnext2#0: MayAlias
+
+// These however do alias
+// CHECK-DAG: x#0 <-> xnext1#0: MayAlias
+// CHECK-DAG: x#0 <-> xnext2#0: MayAlias
+// CHECK-DAG: xnext1#0 <-> xnext2#0: MayAlias
+
+func.func @_QMmPfoo(%arg0: !fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>> {fir.bindc_name = "x"}, %arg1: !fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>> {fir.bindc_name = "y"}) {
+ %0 = fir.alloca i32 {bindc_name = "i1", uniq_name = "_QMmFfooEi1"}
+ %1:2 = hlfir.declare %0 {uniq_name = "_QMmFfooEi1"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
+ %2 = fir.alloca i32 {bindc_name = "i2", uniq_name = "_QMmFfooEi2"}
+ %3:2 = hlfir.declare %2 {uniq_name = "_QMmFfooEi2"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
+ %4:2 = hlfir.declare %arg0 {uniq_name = "_QMmFfooEx", test.ptr = "x"} : (!fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>) -> (!fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>, !fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>)
+ %5:2 = hlfir.declare %arg1 {uniq_name = "_QMmFfooEy", test.ptr = "y"} : (!fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>) -> (!fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>, !fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>)
+ %6 = hlfir.designate %4#0{"next"} {fortran_attrs = #fir.var_attrs<pointer>, test.ptr = "xnext1"} : (!fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>) -> !fir.ref<!fir.box<!fir.ptr<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>>>
+ %7 = fir.load %6 : !fir.ref<!fir.box<!fir.ptr<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>>>
+ %8 = fir.box_addr %7 : (!fir.box<!fir.ptr<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>>) -> !fir.ptr<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>
+ %9 = hlfir.designate %8{"i"} : (!fir.ptr<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>) -> !fir.ref<i32>
+ %10 = fir.load %9 : !fir.ref<i32>
+ hlfir.assign %10 to %1#0 : i32, !fir.ref<i32>
+ hlfir.assign %5#0 to %4#0 : !fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>, !fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>
+ %11 = hlfir.designate %4#0{"next"} {fortran_attrs = #fir.var_attrs<pointer>, test.ptr = "xnext2"} : (!fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>) -> !fir.ref<!fir.box<!fir.ptr<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>>>
+ %12 = fir.load %11 : !fir.ref<!fir.box<!fir.ptr<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>>>
+ %13 = fir.box_addr %12 : (!fir.box<!fir.ptr<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>>) -> !fir.ptr<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>
+ %14 = hlfir.designate %13{"i"} : (!fir.ptr<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>>) -> !fir.ref<i32>
+ %15 = fir.load %14 : !fir.ref<i32>
+ hlfir.assign %15 to %3#0 : i32, !fir.ref<i32>
+ return
+}
>From 840311fd6043ef6849ca0d7123cf95bbce44230e Mon Sep 17 00:00:00 2001
From: Renaud-K <rkauffmann at nvidia.com>
Date: Mon, 6 May 2024 17:12:52 -0700
Subject: [PATCH 5/7] Simplifying initialization of followingData
---
flang/lib/Optimizer/Analysis/AliasAnalysis.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
index ade240bc2364e..fb9f6045586dc 100644
--- a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
+++ b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
@@ -251,7 +251,7 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v) {
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;
+ bool followingData = !isBoxRef;
mlir::SymbolRefAttr global;
Source::Attributes attributes;
while (defOp && !breakFromLoop) {
>From 2281ad05fe40c428adc6206d62fcec34f12a7886 Mon Sep 17 00:00:00 2001
From: Renaud-K <rkauffmann at nvidia.com>
Date: Mon, 13 May 2024 13:00:34 -0700
Subject: [PATCH 6/7] Review feedback
---
.../flang/Optimizer/Analysis/AliasAnalysis.h | 24 +++++++++++++++----
.../lib/Optimizer/Analysis/AliasAnalysis.cpp | 7 +++---
.../AliasAnalysis/alias-analysis-2.fir | 2 +-
.../AliasAnalysis/alias-analysis-3.fir | 2 +-
.../AliasAnalysis/alias-analysis-9.fir | 10 ++++++--
5 files changed, 32 insertions(+), 13 deletions(-)
diff --git a/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h b/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
index 8a0eb525f1915..b6f18879dd04d 100644
--- a/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
+++ b/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
@@ -54,9 +54,22 @@ struct AliasAnalysis {
// 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>>>
+ // Example:
+ // ------------------- test.f90 --------------------
+ // module top
+ // real, pointer :: a(:)
+ // end module
+ //
+ // subroutine test()
+ // use top
+ // a(1) = 1
+ // -------------------------------------------------
+ //
+ // flang-new -fc1 -emit-fir test.f90 -o test.fir
+ //
+ // ------------------- test.fir --------------------
+ // fir.global @_QMtopEa : !fir.box<!fir.ptr<!fir.array<?xf32>>>
//
// func.func @_QPtest() {
// %c1 = arith.constant 1 : index
@@ -69,10 +82,11 @@ struct AliasAnalysis {
// 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
+ // reach into the data wrapped by the box (the descriptor). Therefore when
+ // asking about the memory source of %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
@@ -88,7 +102,7 @@ struct AliasAnalysis {
// 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
+ // This also 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>>> ) {
diff --git a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
index fb9f6045586dc..165ceef60d768 100644
--- a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
+++ b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
@@ -173,9 +173,8 @@ AliasResult AliasAnalysis::alias(Value lhs, Value rhs) {
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.
+ // POINTER components may alias with POINTER objects,
+ // as well as other POINTER components
if ((src1->isRecordWithPointerComponent() && src2->isTargetOrPointer()) ||
(src2->isRecordWithPointerComponent() && src1->isTargetOrPointer()) ||
(src1->isRecordWithPointerComponent() &&
@@ -290,7 +289,7 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v) {
.Case<fir::LoadOp>([&](auto op) {
// 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
+ // TODO: Add support to fir.alloca and fir.allocmem
auto def = getOriginalDef(op.getMemref());
if (isDummyArgument(def) ||
def.template getDefiningOp<fir::AddrOfOp>()) {
diff --git a/flang/test/Analysis/AliasAnalysis/alias-analysis-2.fir b/flang/test/Analysis/AliasAnalysis/alias-analysis-2.fir
index af3e491e062c3..d03348efd2a68 100644
--- a/flang/test/Analysis/AliasAnalysis/alias-analysis-2.fir
+++ b/flang/test/Analysis/AliasAnalysis/alias-analysis-2.fir
@@ -143,7 +143,7 @@ func.func @_QFPtest2(%arg0: !fir.ref<f32> {fir.bindc_name = "v1", fir.target}, %
// alias with the wrapped scalar _QFEvar2. We meant box_addr of _QMpointersEp
// CHECK-DAG: p#0 <-> box.addr#0: NoAlias
-// Now handling more gracefully the difference between !fir.ref<!fir.box<>> and !fir.box<>
+// Handling gracefully the difference between !fir.ref<!fir.box<>> and !fir.box<>
// CHECK-DAG: box.addr#0 <-> func.region0#0: NoAlias
// var2, although it is a target, cannot alias with p
diff --git a/flang/test/Analysis/AliasAnalysis/alias-analysis-3.fir b/flang/test/Analysis/AliasAnalysis/alias-analysis-3.fir
index 3d576cb5c0af9..91829a461dc72 100644
--- a/flang/test/Analysis/AliasAnalysis/alias-analysis-3.fir
+++ b/flang/test/Analysis/AliasAnalysis/alias-analysis-3.fir
@@ -23,7 +23,7 @@
// FIXME: a's box cannot alias with raw reference to f32 (x), so MayAlias below must be NoAlias:
// CHECK: a#0 <-> func.region0#1: MayAlias
-// pointer_dummy's box cannot alias with raw reference to f32 (x), so MayAlias below must be NoAlias:
+// pointer_dummy's box cannot alias with raw reference to f32 (x)
// CHECK: func.region0#0 <-> func.region0#1: NoAlias
fir.global @_QMmEa : !fir.type<_QMmTt{pointer_component:!fir.box<!fir.ptr<f32>>}> {
diff --git a/flang/test/Analysis/AliasAnalysis/alias-analysis-9.fir b/flang/test/Analysis/AliasAnalysis/alias-analysis-9.fir
index 2b92872a2c4b5..8194358d48797 100644
--- a/flang/test/Analysis/AliasAnalysis/alias-analysis-9.fir
+++ b/flang/test/Analysis/AliasAnalysis/alias-analysis-9.fir
@@ -17,14 +17,20 @@
// end module
// CHECK-LABEL: Testing : "_QMmPfoo"
-// TODO: x and y do not alias
+// TODO: x and y are non pointer, non target argument and therefore do not alias.
// CHECK-DAG: x#0 <-> y#0: MayAlias
+
+// TODO: y is not a pointer object and therefore does not alias with the x%next component.
// CHECK-DAG: y#0 <-> xnext1#0: MayAlias
// CHECK-DAG: y#0 <-> xnext2#0: MayAlias
-// These however do alias
+// Pointer components may alias with pointer objects,
+// as well as other pointer components
// CHECK-DAG: x#0 <-> xnext1#0: MayAlias
// CHECK-DAG: x#0 <-> xnext2#0: MayAlias
+
+// 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
func.func @_QMmPfoo(%arg0: !fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>> {fir.bindc_name = "x"}, %arg1: !fir.ref<!fir.type<_QMmTt{next:!fir.box<!fir.ptr<!fir.type<_QMmTt>>>,i:i32}>> {fir.bindc_name = "y"}) {
>From 3ef1e89345c4fc4644f991e87df2c042d1621213 Mon Sep 17 00:00:00 2001
From: Renaud-K <rkauffmann at nvidia.com>
Date: Wed, 15 May 2024 14:12:59 -0700
Subject: [PATCH 7/7] Review feedback
---
flang/include/flang/Optimizer/Analysis/AliasAnalysis.h | 1 +
flang/lib/Optimizer/Analysis/AliasAnalysis.cpp | 7 ++++---
flang/test/Analysis/AliasAnalysis/alias-analysis-9.fir | 6 ++++--
3 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h b/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
index b6f18879dd04d..40fd1705115a0 100644
--- a/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
+++ b/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
@@ -64,6 +64,7 @@ struct AliasAnalysis {
// subroutine test()
// use top
// a(1) = 1
+ // end subroutine
// -------------------------------------------------
//
// flang-new -fc1 -emit-fir test.f90 -o test.fir
diff --git a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
index 165ceef60d768..9d0d706a85c5e 100644
--- a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
+++ b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
@@ -108,7 +108,7 @@ 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() << "\n"; llvm::dbgs() << "AliasAnalysis::alias\n";
+ LLVM_DEBUG(llvm::dbgs() << "\nAliasAnalysis::alias\n";
llvm::dbgs() << " lhs: " << lhs << "\n";
llvm::dbgs() << " lhsSrc: " << lhsSrc << "\n";
llvm::dbgs() << " rhs: " << rhs << "\n";
@@ -173,8 +173,9 @@ AliasResult AliasAnalysis::alias(Value lhs, Value rhs) {
return AliasResult::MayAlias;
}
- // POINTER components may alias with POINTER objects,
- // as well as other POINTER components
+ // 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 ((src1->isRecordWithPointerComponent() && src2->isTargetOrPointer()) ||
(src2->isRecordWithPointerComponent() && src1->isTargetOrPointer()) ||
(src1->isRecordWithPointerComponent() &&
diff --git a/flang/test/Analysis/AliasAnalysis/alias-analysis-9.fir b/flang/test/Analysis/AliasAnalysis/alias-analysis-9.fir
index 8194358d48797..df24a6d32aa59 100644
--- a/flang/test/Analysis/AliasAnalysis/alias-analysis-9.fir
+++ b/flang/test/Analysis/AliasAnalysis/alias-analysis-9.fir
@@ -21,11 +21,13 @@
// CHECK-DAG: x#0 <-> y#0: MayAlias
// TODO: y is not a pointer object and therefore does not alias with the x%next component.
+// Also assigning x to y would not modify x.next
// CHECK-DAG: y#0 <-> xnext1#0: MayAlias
// CHECK-DAG: y#0 <-> xnext2#0: MayAlias
-// Pointer components may alias with pointer objects,
-// as well as other pointer components
+// We need to catch the fact that assigning y to x will modify xnext.
+// The only side-effect between the 2 loads of x.next is the assignment to x,
+// 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
More information about the flang-commits
mailing list