Skip to content

[flang] AliasAnalysis: Handle fir.load on hlfir.designate #127107

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 19, 2025

Conversation

jdenny-ornl
Copy link
Collaborator

For example, determine that the address in obj%p below cannot alias the address of v:

module m
  type :: ty
    real, pointer :: p
  end type ty
end module m
subroutine test()
  use m
  real, target :: t
  real :: v
  type(ty) :: obj
  obj%p => t
  v = obj%p
end subroutine test

For example, determine that the address in `obj%p` below cannot alias
the address of `v`:

```
module m
  type :: ty
    real, pointer :: p
  end type ty
end module m
subroutine test()
  use m
  real, target :: t
  real :: v
  type(ty) :: obj
  obj%p => t
  v = obj%p
end subroutine test
```
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir labels Feb 13, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 13, 2025

@llvm/pr-subscribers-flang-fir-hlfir

Author: Joel E. Denny (jdenny-ornl)

Changes

For example, determine that the address in obj%p below cannot alias the address of v:

module m
  type :: ty
    real, pointer :: p
  end type ty
end module m
subroutine test()
  use m
  real, target :: t
  real :: v
  type(ty) :: obj
  obj%p => t
  v = obj%p
end subroutine test

Patch is 44.10 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/127107.diff

3 Files Affected:

  • (modified) flang/include/flang/Optimizer/Analysis/AliasAnalysis.h (+4-4)
  • (modified) flang/lib/Optimizer/Analysis/AliasAnalysis.cpp (+16-2)
  • (added) flang/test/Analysis/AliasAnalysis/load-ptr-designate.fir (+484)
diff --git a/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h b/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
index 8d17e4e476d10..c71988d081dd0 100644
--- a/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
+++ b/flang/include/flang/Optimizer/Analysis/AliasAnalysis.h
@@ -211,14 +211,14 @@ struct AliasAnalysis {
   fir::AliasAnalysis::Source getSource(mlir::Value,
                                        bool getLastInstantiationPoint = false);
 
+  /// Return true, if `ty` is a reference type to a boxed
+  /// POINTER object or a raw fir::PointerType.
+  static bool isPointerReference(mlir::Type ty);
+
 private:
   /// Return true, if `ty` is a reference type to an object of derived type
   /// that contains a component with POINTER attribute.
   static bool isRecordWithPointerComponent(mlir::Type ty);
-
-  /// Return true, if `ty` is a reference type to a boxed
-  /// POINTER object or a raw fir::PointerType.
-  static bool isPointerReference(mlir::Type ty);
 };
 
 inline bool operator==(const AliasAnalysis::Source::SourceOrigin &lhs,
diff --git a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
index 873758487ddd0..70fa18ad65b9b 100644
--- a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
+++ b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
@@ -54,10 +54,11 @@ static bool hasGlobalOpTargetAttr(mlir::Value v, fir::AddrOfOp op) {
 static mlir::Value
 getOriginalDef(mlir::Value v,
                fir::AliasAnalysis::Source::Attributes &attributes,
-               bool &isCapturedInInternalProcedure) {
+               bool &isCapturedInInternalProcedure, bool &approximateSource) {
   mlir::Operation *defOp;
   bool breakFromLoop = false;
   while (!breakFromLoop && (defOp = v.getDefiningOp())) {
+    mlir::Type ty = defOp->getResultTypes()[0];
     llvm::TypeSwitch<Operation *>(defOp)
         .Case<fir::ConvertOp>([&](fir::ConvertOp op) { v = op.getValue(); })
         .Case<fir::DeclareOp, hlfir::DeclareOp>([&](auto op) {
@@ -67,6 +68,18 @@ getOriginalDef(mlir::Value v,
           isCapturedInInternalProcedure |=
               varIf.isCapturedInInternalProcedure();
         })
+        .Case<fir::CoordinateOp>([&](auto op) {
+          if (fir::AliasAnalysis::isPointerReference(ty))
+            attributes.set(fir::AliasAnalysis::Attribute::Pointer);
+          v = op->getOperand(0);
+          approximateSource = true;
+        })
+        .Case<hlfir::DesignateOp>([&](hlfir::DesignateOp op) {
+          auto varIf = llvm::cast<fir::FortranVariableOpInterface>(defOp);
+          attributes |= getAttrsFromVariable(varIf);
+          v = op.getMemref();
+          approximateSource = true;
+        })
         .Default([&](auto op) { breakFromLoop = true; });
   }
   return v;
@@ -609,7 +622,8 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v,
               attributes.set(Attribute::Pointer);
 
             auto def = getOriginalDef(op.getMemref(), attributes,
-                                      isCapturedInInternalProcedure);
+                                      isCapturedInInternalProcedure,
+                                      approximateSource);
             if (auto addrOfOp = def.template getDefiningOp<fir::AddrOfOp>()) {
               global = addrOfOp.getSymbol();
 
diff --git a/flang/test/Analysis/AliasAnalysis/load-ptr-designate.fir b/flang/test/Analysis/AliasAnalysis/load-ptr-designate.fir
new file mode 100644
index 0000000000000..192de7f96f928
--- /dev/null
+++ b/flang/test/Analysis/AliasAnalysis/load-ptr-designate.fir
@@ -0,0 +1,484 @@
+// Check aliasing with the address *in* (not *of*) a pointer component
+// (hlfir.designate).
+//
+// Throughout this test, the ".fir" suffix on symbols indicates a version of the
+// MLIR after convert-hlfir-to-fir.  A key difference is that component access
+// is via fir.coordinate_of instead of hlfir.designate.  We would like alias
+// analysis results to be the same in both versions.
+
+// RUN: fir-opt %s -split-input-file -o /dev/null --mlir-disable-threading  \
+// RUN:   -pass-pipeline='builtin.module(func.func(test-fir-alias-analysis))' \
+// RUN:   2>&1 | FileCheck -match-full-lines %s
+
+// module m
+//   type :: ty
+//     real, pointer :: p0, p1
+//     real :: arr(2)
+//     real, allocatable :: alloc
+//     ! target attribute on components is not supported
+//   end type ty
+// end module m
+// subroutine test()
+//   use m
+//   real, target :: t
+//   real :: v
+//   type(ty) :: obj
+//   type(ty), target :: t_obj
+// end subroutine test
+
+// CHECK-LABEL: Testing : "_QPtest"
+
+// The address in a pointer can alias the address in another pointer or the
+// address of a target but not the address of other variables.
+// CHECK-DAG: obj%p0.tgt#0 <-> obj%p1.tgt#0: MayAlias
+// CHECK-DAG: t#0 <-> obj%p0.tgt#0: MayAlias
+// CHECK-DAG: t#0 <-> obj%p1.tgt#0: MayAlias
+// CHECK-DAG: v#0 <-> obj%p0.tgt#0: NoAlias
+// CHECK-DAG: v#0 <-> obj%p1.tgt#0: NoAlias
+// CHECK-DAG: obj%p0.tgt.fir#0 <-> obj%p1.tgt.fir#0: MayAlias
+// CHECK-DAG: t.fir#0 <-> obj%p0.tgt.fir#0: MayAlias
+// CHECK-DAG: t.fir#0 <-> obj%p1.tgt.fir#0: MayAlias
+// CHECK-DAG: v.fir#0 <-> obj%p0.tgt.fir#0: NoAlias
+// CHECK-DAG: v.fir#0 <-> obj%p1.tgt.fir#0: NoAlias
+
+// The address in a pointer cannot alias the address of a pointer.
+// CHECK-DAG: obj%p0#0 <-> obj%p0.tgt#0: NoAlias
+// CHECK-DAG: obj%p0#0 <-> obj%p1.tgt#0: NoAlias
+// CHECK-DAG: obj%p0.tgt#0 <-> obj%p1#0: NoAlias
+// CHECK-DAG: obj%p1#0 <-> obj%p1.tgt#0: NoAlias
+// CHECK-DAG: obj%p0.fir#0 <-> obj%p0.tgt.fir#0: NoAlias
+// CHECK-DAG: obj%p0.fir#0 <-> obj%p1.tgt.fir#0: NoAlias
+// CHECK-DAG: obj%p0.tgt.fir#0 <-> obj%p1.fir#0: NoAlias
+// CHECK-DAG: obj%p1.fir#0 <-> obj%p1.tgt.fir#0: NoAlias
+
+// For some cases, AliasAnalysis analyzes hlfir.designate like fir.box_addr, so
+// make sure it doesn't mistakenly see the address of obj%arr(1) as an address
+// that was loaded from a pointer and that could alias something.  However,
+// t_obj%arr is a target.
+// TODO: Thus, we expect the first case (and corresponding .fir case) below to
+// be NoAlias.  However, the addresses obj%p0.tgt and obj%arr(1) are analyzed as
+// MayAlias because they have the same source and both are data.
+// CHECK-DAG: obj%p0.tgt#0 <-> obj%arr(1)#0: MayAlias
+// CHECK-DAG: obj%p0.tgt#0 <-> t_obj%arr(1)#0: MayAlias
+// CHECK-DAG: obj%p0.tgt.fir#0 <-> obj%arr(1).fir#0: MayAlias
+// CHECK-DAG: obj%p0.tgt.fir#0 <-> t_obj%arr(1).fir#0: MayAlias
+
+// Like a pointer, an allocatable contains an address, but an allocatable is not
+// a pointer and so cannot alias pointers.  However, t_obj%alloc is a target.
+// TODO: Thus, we expect the first case (and corresponding .fir case) below to
+// be NoAlias.  However, the addresses obj%p0.tgt and obj%alloc.tgt are analyzed
+// as MayAlias because they have the same source and both are data.
+// CHECK-DAG: obj%p0.tgt#0 <-> obj%alloc.tgt#0: MayAlias
+// CHECK-DAG: obj%p0.tgt#0 <-> t_obj%alloc.tgt#0: MayAlias
+// CHECK-DAG: obj%p0.tgt.fir#0 <-> obj%alloc.tgt.fir#0: MayAlias
+// CHECK-DAG: obj%p0.tgt.fir#0 <-> t_obj%alloc.tgt.fir#0: MayAlias
+
+// The address in an allocatable cannot alias the address of that allocatable.
+// CHECK-DAG: obj%alloc#0 <-> obj%alloc.tgt#0: NoAlias
+// CHECK-DAG: t_obj%alloc#0 <-> t_obj%alloc.tgt#0: NoAlias
+// CHECK-DAG: obj%alloc.fir#0 <-> obj%alloc.tgt.fir#0: NoAlias
+// CHECK-DAG: t_obj%alloc.fir#0 <-> t_obj%alloc.tgt.fir#0: NoAlias
+
+// The address of a composite aliases the address of any component but not the
+// address in a pointer or allocatable component.
+// TODO: Thus, we expect the obj%*.tgt cases below to be NoAlias.  However, the
+// addresses obj and obj%*.tgt are analyzed as MayAlias because they have the
+// same source and both are data.
+// CHECK-DAG: obj#0 <-> obj%p0#0: MayAlias
+// 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.tgt.fir#0: MayAlias
+// CHECK-DAG: obj.fir#0 <-> obj%alloc.tgt.fir#0: MayAlias
+
+func.func @_QPtest() {
+  %0 = fir.alloca !fir.type<_QMmTty{p0:!fir.box<!fir.ptr<f32>>,p1:!fir.box<!fir.ptr<f32>>,arr:!fir.array<2xf32>,alloc:!fir.box<!fir.heap<f32>>}> {bindc_name = "obj", uniq_name = "_QFtestEobj"}
+  %1:2 = hlfir.declare %0 {test.ptr="obj", uniq_name = "_QFtestEobj"} : (!fir.ref<!fir.type<_QMmTty{p0:!fir.box<!fir.ptr<f32>>,p1:!fir.box<!fir.ptr<f32>>,arr:!fir.array<2xf32>,alloc:!fir.box<!fir.heap<f32>>}>>) -> (!fir.ref<!fir.type<_QMmTty{p0:!fir.box<!fir.ptr<f32>>,p1:!fir.box<!fir.ptr<f32>>,arr:!fir.array<2xf32>,alloc:!fir.box<!fir.heap<f32>>}>>, !fir.ref<!fir.type<_QMmTty{p0:!fir.box<!fir.ptr<f32>>,p1:!fir.box<!fir.ptr<f32>>,arr:!fir.array<2xf32>,alloc:!fir.box<!fir.heap<f32>>}>>)
+  %2 = fir.alloca f32 {bindc_name = "t", fir.target, uniq_name = "_QFtestEt"}
+  %3:2 = hlfir.declare %2 {test.ptr="t", fortran_attrs = #fir.var_attrs<target>, uniq_name = "_QFtestEt"} : (!fir.ref<f32>) -> (!fir.ref<f32>, !fir.ref<f32>)
+  %4 = fir.alloca !fir.type<_QMmTty{p0:!fir.box<!fir.ptr<f32>>,p1:!fir.box<!fir.ptr<f32>>,arr:!fir.array<2xf32>,alloc:!fir.box<!fir.heap<f32>>}> {bindc_name = "t_obj", fir.target, uniq_name = "_QFtestEt_obj"}
+  %5:2 = hlfir.declare %4 {test.ptr="t_obj", fortran_attrs = #fir.var_attrs<target>, uniq_name = "_QFtestEt_obj"} : (!fir.ref<!fir.type<_QMmTty{p0:!fir.box<!fir.ptr<f32>>,p1:!fir.box<!fir.ptr<f32>>,arr:!fir.array<2xf32>,alloc:!fir.box<!fir.heap<f32>>}>>) -> (!fir.ref<!fir.type<_QMmTty{p0:!fir.box<!fir.ptr<f32>>,p1:!fir.box<!fir.ptr<f32>>,arr:!fir.array<2xf32>,alloc:!fir.box<!fir.heap<f32>>}>>, !fir.ref<!fir.type<_QMmTty{p0:!fir.box<!fir.ptr<f32>>,p1:!fir.box<!fir.ptr<f32>>,arr:!fir.array<2xf32>,alloc:!fir.box<!fir.heap<f32>>}>>)
+  %6 = fir.alloca f32 {bindc_name = "v", uniq_name = "_QFtestEv"}
+  %7:2 = hlfir.declare %6 {test.ptr="v", uniq_name = "_QFtestEv"} : (!fir.ref<f32>) -> (!fir.ref<f32>, !fir.ref<f32>)
+  %8 = hlfir.designate %1#0{"p0"}   {test.ptr="obj%p0", fortran_attrs = #fir.var_attrs<pointer>} : (!fir.ref<!fir.type<_QMmTty{p0:!fir.box<!fir.ptr<f32>>,p1:!fir.box<!fir.ptr<f32>>,arr:!fir.array<2xf32>,alloc:!fir.box<!fir.heap<f32>>}>>) -> !fir.ref<!fir.box<!fir.ptr<f32>>>
+  %9 = fir.load %8 : !fir.ref<!fir.box<!fir.ptr<f32>>>
+  %10 = fir.box_addr %9 {test.ptr="obj%p0.tgt"} : (!fir.box<!fir.ptr<f32>>) -> !fir.ptr<f32>
+  %11 = hlfir.designate %1#0{"p1"}   {test.ptr="obj%p1", fortran_attrs = #fir.var_attrs<pointer>} : (!fir.ref<!fir.type<_QMmTty{p0:!fir.box<!fir.ptr<f32>>,p1:!fir.box<!fir.ptr<f32>>,arr:!fir.array<2xf32>,alloc:!fir.box<!fir.heap<f32>>}>>) -> !fir.ref<!fir.box<!fir.ptr<f32>>>
+  %12 = fir.load %11 : !fir.ref<!fir.box<!fir.ptr<f32>>>
+  %13 = fir.box_addr %12 {test.ptr="obj%p1.tgt"}: (!fir.box<!fir.ptr<f32>>) -> !fir.ptr<f32>
+  %c2 = arith.constant 2 : index
+  %14 = fir.shape %c2 : (index) -> !fir.shape<1>
+  %c1 = arith.constant 1 : index
+  %15 = hlfir.designate %1#0{"arr"} <%14> (%c1) {test.ptr="obj%arr(1)"} : (!fir.ref<!fir.type<_QMmTty{p0:!fir.box<!fir.ptr<f32>>,p1:!fir.box<!fir.ptr<f32>>,arr:!fir.array<2xf32>,alloc:!fir.box<!fir.heap<f32>>}>>, !fir.shape<1>, index) -> !fir.ref<f32>
+  %16 = hlfir.designate %1#0{"alloc"}   {test.ptr="obj%alloc", fortran_attrs = #fir.var_attrs<allocatable>} : (!fir.ref<!fir.type<_QMmTty{p0:!fir.box<!fir.ptr<f32>>,p1:!fir.box<!fir.ptr<f32>>,arr:!fir.array<2xf32>,alloc:!fir.box<!fir.heap<f32>>}>>) -> !fir.ref<!fir.box<!fir.heap<f32>>>
+  %17 = fir.load %16 : !fir.ref<!fir.box<!fir.heap<f32>>>
+  %18 = fir.box_addr %17 {test.ptr="obj%alloc.tgt"}: (!fir.box<!fir.heap<f32>>) -> !fir.heap<f32>
+  %c2_1 = arith.constant 2 : index
+  %19 = fir.shape %c2_1 : (index) -> !fir.shape<1>
+  %c1_2 = arith.constant 1 : index
+  %20 = hlfir.designate %5#0{"arr"} <%19> (%c1_2) {test.ptr="t_obj%arr(1)"} : (!fir.ref<!fir.type<_QMmTty{p0:!fir.box<!fir.ptr<f32>>,p1:!fir.box<!fir.ptr<f32>>,arr:!fir.array<2xf32>,alloc:!fir.box<!fir.heap<f32>>}>>, !fir.shape<1>, index) -> !fir.ref<f32>
+  %21 = hlfir.designate %5#0{"alloc"}   {test.ptr="t_obj%alloc", fortran_attrs = #fir.var_attrs<allocatable>} : (!fir.ref<!fir.type<_QMmTty{p0:!fir.box<!fir.ptr<f32>>,p1:!fir.box<!fir.ptr<f32>>,arr:!fir.array<2xf32>,alloc:!fir.box<!fir.heap<f32>>}>>) -> !fir.ref<!fir.box<!fir.heap<f32>>>
+  %22 = fir.load %21 : !fir.ref<!fir.box<!fir.heap<f32>>>
+  %23 = fir.box_addr %22 {test.ptr="t_obj%alloc.tgt"} : (!fir.box<!fir.heap<f32>>) -> !fir.heap<f32>
+  return
+}
+
+func.func @_QPtest.fir() {
+  %0 = fir.alloca !fir.type<_QMmTty{p0:!fir.box<!fir.ptr<f32>>,p1:!fir.box<!fir.ptr<f32>>,arr:!fir.array<2xf32>,alloc:!fir.box<!fir.heap<f32>>}> {bindc_name = "obj", uniq_name = "_QFtestEobj"}
+  %1 = fir.declare %0 {test.ptr="obj.fir", uniq_name = "_QFtestEobj"} : (!fir.ref<!fir.type<_QMmTty{p0:!fir.box<!fir.ptr<f32>>,p1:!fir.box<!fir.ptr<f32>>,arr:!fir.array<2xf32>,alloc:!fir.box<!fir.heap<f32>>}>>) -> !fir.ref<!fir.type<_QMmTty{p0:!fir.box<!fir.ptr<f32>>,p1:!fir.box<!fir.ptr<f32>>,arr:!fir.array<2xf32>,alloc:!fir.box<!fir.heap<f32>>}>>
+  %2 = fir.alloca f32 {bindc_name = "t", fir.target, uniq_name = "_QFtestEt"}
+  %3 = fir.declare %2 {test.ptr = "t.fir", fortran_attrs = #fir.var_attrs<target>, uniq_name = "_QFtestEt"} : (!fir.ref<f32>) -> !fir.ref<f32>
+  %4 = fir.alloca !fir.type<_QMmTty{p0:!fir.box<!fir.ptr<f32>>,p1:!fir.box<!fir.ptr<f32>>,arr:!fir.array<2xf32>,alloc:!fir.box<!fir.heap<f32>>}> {bindc_name = "t_obj", fir.target, uniq_name = "_QFtestEt_obj"}
+  %5 = fir.declare %4 {test.ptr="t_obj.fir", fortran_attrs = #fir.var_attrs<target>, uniq_name = "_QFtestEt_obj"} : (!fir.ref<!fir.type<_QMmTty{p0:!fir.box<!fir.ptr<f32>>,p1:!fir.box<!fir.ptr<f32>>,arr:!fir.array<2xf32>,alloc:!fir.box<!fir.heap<f32>>}>>) -> !fir.ref<!fir.type<_QMmTty{p0:!fir.box<!fir.ptr<f32>>,p1:!fir.box<!fir.ptr<f32>>,arr:!fir.array<2xf32>,alloc:!fir.box<!fir.heap<f32>>}>>
+  %6 = fir.alloca f32 {bindc_name = "v", uniq_name = "_QFtestEv"}
+  %7 = fir.declare %6 {test.ptr = "v.fir", uniq_name = "_QFtestEv"} : (!fir.ref<f32>) -> !fir.ref<f32>
+  %8 = fir.field_index p0, !fir.type<_QMmTty{p0:!fir.box<!fir.ptr<f32>>,p1:!fir.box<!fir.ptr<f32>>,arr:!fir.array<2xf32>,alloc:!fir.box<!fir.heap<f32>>}>
+  %9 = fir.coordinate_of %1, %8 {test.ptr="obj%p0.fir"} : (!fir.ref<!fir.type<_QMmTty{p0:!fir.box<!fir.ptr<f32>>,p1:!fir.box<!fir.ptr<f32>>,arr:!fir.array<2xf32>,alloc:!fir.box<!fir.heap<f32>>}>>, !fir.field) -> !fir.ref<!fir.box<!fir.ptr<f32>>>
+  %10 = fir.load %9 : !fir.ref<!fir.box<!fir.ptr<f32>>>
+  %11 = fir.box_addr %10 {test.ptr = "obj%p0.tgt.fir"} : (!fir.box<!fir.ptr<f32>>) -> !fir.ptr<f32>
+  %12 = fir.field_index p1, !fir.type<_QMmTty{p0:!fir.box<!fir.ptr<f32>>,p1:!fir.box<!fir.ptr<f32>>,arr:!fir.array<2xf32>,alloc:!fir.box<!fir.heap<f32>>}>
+  %13 = fir.coordinate_of %1, %12 {test.ptr="obj%p1.fir"} : (!fir.ref<!fir.type<_QMmTty{p0:!fir.box<!fir.ptr<f32>>,p1:!fir.box<!fir.ptr<f32>>,arr:!fir.array<2xf32>,alloc:!fir.box<!fir.heap<f32>>}>>, !fir.field) -> !fir.ref<!fir.box<!fir.ptr<f32>>>
+  %14 = fir.load %13 : !fir.ref<!fir.box<!fir.ptr<f32>>>
+  %15 = fir.box_addr %14 {test.ptr = "obj%p1.tgt.fir"} : (!fir.box<!fir.ptr<f32>>) -> !fir.ptr<f32>
+  %c2 = arith.constant 2 : index
+  %16 = fir.shape %c2 : (index) -> !fir.shape<1>
+  %c1 = arith.constant 1 : index
+  %17 = fir.field_index arr, !fir.type<_QMmTty{p0:!fir.box<!fir.ptr<f32>>,p1:!fir.box<!fir.ptr<f32>>,arr:!fir.array<2xf32>,alloc:!fir.box<!fir.heap<f32>>}>
+  %18 = fir.coordinate_of %1, %17 : (!fir.ref<!fir.type<_QMmTty{p0:!fir.box<!fir.ptr<f32>>,p1:!fir.box<!fir.ptr<f32>>,arr:!fir.array<2xf32>,alloc:!fir.box<!fir.heap<f32>>}>>, !fir.field) -> !fir.ref<!fir.array<2xf32>>
+  %19 = fir.array_coor %18(%16) %c1 {test.ptr="obj%arr(1).fir"} : (!fir.ref<!fir.array<2xf32>>, !fir.shape<1>, index) -> !fir.ref<f32>
+  %20 = fir.field_index alloc, !fir.type<_QMmTty{p0:!fir.box<!fir.ptr<f32>>,p1:!fir.box<!fir.ptr<f32>>,arr:!fir.array<2xf32>,alloc:!fir.box<!fir.heap<f32>>}>
+  %21 = fir.coordinate_of %1, %20 {test.ptr="obj%alloc.fir"} : (!fir.ref<!fir.type<_QMmTty{p0:!fir.box<!fir.ptr<f32>>,p1:!fir.box<!fir.ptr<f32>>,arr:!fir.array<2xf32>,alloc:!fir.box<!fir.heap<f32>>}>>, !fir.field) -> !fir.ref<!fir.box<!fir.heap<f32>>>
+  %22 = fir.load %21 : !fir.ref<!fir.box<!fir.heap<f32>>>
+  %23 = fir.box_addr %22 {test.ptr = "obj%alloc.tgt.fir"} : (!fir.box<!fir.heap<f32>>) -> !fir.heap<f32>
+  %c2_0 = arith.constant 2 : index
+  %24 = fir.shape %c2_0 : (index) -> !fir.shape<1>
+  %c1_1 = arith.constant 1 : index
+  %25 = fir.field_index arr, !fir.type<_QMmTty{p0:!fir.box<!fir.ptr<f32>>,p1:!fir.box<!fir.ptr<f32>>,arr:!fir.array<2xf32>,alloc:!fir.box<!fir.heap<f32>>}>
+  %26 = fir.coordinate_of %5, %25 : (!fir.ref<!fir.type<_QMmTty{p0:!fir.box<!fir.ptr<f32>>,p1:!fir.box<!fir.ptr<f32>>,arr:!fir.array<2xf32>,alloc:!fir.box<!fir.heap<f32>>}>>, !fir.field) -> !fir.ref<!fir.array<2xf32>>
+  %27 = fir.array_coor %26(%24) %c1_1 {test.ptr="t_obj%arr(1).fir"} : (!fir.ref<!fir.array<2xf32>>, !fir.shape<1>, index) -> !fir.ref<f32>
+  %28 = fir.field_index alloc, !fir.type<_QMmTty{p0:!fir.box<!fir.ptr<f32>>,p1:!fir.box<!fir.ptr<f32>>,arr:!fir.array<2xf32>,alloc:!fir.box<!fir.heap<f32>>}>
+  %29 = fir.coordinate_of %5, %28 {test.ptr="t_obj%alloc.fir"} : (!fir.ref<!fir.type<_QMmTty{p0:!fir.box<!fir.ptr<f32>>,p1:!fir.box<!fir.ptr<f32>>,arr:!fir.array<2xf32>,alloc:!fir.box<!fir.heap<f32>>}>>, !fir.field) -> !fir.ref<!fir.box<!fir.heap<f32>>>
+  %30 = fir.load %29 : !fir.ref<!fir.box<!fir.heap<f32>>>
+  %31 = fir.box_addr %30 {test.ptr = "t_obj%alloc.tgt.fir"} : (!fir.box<!fir.heap<f32>>) -> !fir.heap<f32>
+  return
+}
+
+// -----
+
+// Repeat above test except composites are dummy args instead of locals.
+
+// module m
+//   type :: ty
+//     real, pointer :: p0, p1
+//     real :: arr(2)
+//     real, allocatable :: alloc
+//     ! target attribute on components is not supported
+//   end type ty
+// end module m
+// subroutine test(obj, t_obj)
+//   use m
+//   type(ty) :: obj
+//   type(ty), target :: t_obj
+//   real, target :: t
+//   real :: v
+// end subroutine test
+
+// CHECK-LABEL: Testing : "_QPtest"
+
+// The address in a pointer can alias the address in another pointer or the
+// address of a target but not the address of other variables.
+// CHECK-DAG: obj%p0.tgt#0 <-> obj%p1.tgt#0: MayAlias
+// CHECK-DAG: t#0 <-> obj%p0.tgt#0: MayAlias
+// CHECK-DAG: t#0 <-> obj%p1.tgt#0: MayAlias
+// CHECK-DAG: v#0 <-> obj%p0.tgt#0: NoAlias
+// CHECK-DAG: v#0 <-> obj%p1.tgt#0: NoAlias
+// CHECK-DAG: obj%p0.tgt.fir#0 <-> obj%p1.tgt.fir#0: MayAlias
+// CHECK-DAG: t.fir#0 <-> obj%p0.tgt.fir#0: MayAlias
+// CHECK-DAG: t.fir#0 <-> obj%p1.tgt.fir#0: MayAlias
+// CHECK-DAG: v.fir#0 <-> obj%p0.tgt.fir#0: NoAlias
+// CHECK-DAG: v.fir#0 <-> obj%p1.tgt.fir#0: NoAlias
+
+// The address in a pointer cannot alias the address of a pointer.
+// CHECK-DAG: obj%p0#0 <-> obj%p0.tgt#0: NoAlias
+// CHECK-DAG: obj%p0#0 <-> obj%p1.tgt#0: NoAlias
+// CHECK-DAG: obj%p0.tgt#0 <-> obj%p1#0: NoAlias
+// CHECK-DAG: obj%p1#0 <-> obj%p1.tgt#0: NoAlias
+// CHECK-DAG: obj%p0.fir#0 <-> obj%p0.tgt.fir#0: NoAlias
+// CHECK-DAG: obj%p0.fir#0 <-> obj%p1.tgt.fir#0: NoAlias
+// CHECK-DAG: obj%p0.tgt.fir#0 <-> obj%p1.fir#0: NoAlias
+// CHECK-DAG: obj%p1.fir#0 <-> obj%p1.tgt.fir#0: NoAlias
+
+// For some cases, AliasAnalysis analyzes hlfir.designate like fir.box_addr, so
+// make sure it doesn't mistakenly see the address of obj%arr(1) as an address
+// that was loaded from a pointer and that could alias something.  However,
+// t_obj%arr is a target.
+// TODO: Thus, we expect the first case (and corresponding .fir case) below to
+// be NoAlias.  However, the addresses obj%p0.tgt and obj%arr(1) are analyzed as
+// MayAlias because they have the same source and both are data.
+// CHECK-DAG: obj%p0.tgt#0 <-> obj%arr(1)#0: MayAlias
+// CHECK-DAG: obj%p0.tgt#0 <-> t_obj%arr(1)#0: MayAlias
+// CHECK-DAG: obj%p0.tgt.fir#0 <-> obj%arr(1).fir#0: MayAlias
+// CHECK-DAG: obj%p0.tgt.fir#0 <-> t_obj%arr(1).fir#0: MayAlias
+
+// Like a pointer, an allocatable contains an address, but an allocatable is not
+// a pointer and so cannot alias pointers.  However, t_obj%alloc is a target.
+// TODO: Thus, we expect the first case (and corresponding .fir case) below to
+// be NoAlias.  However, the addresses obj%p0.tgt and obj%alloc.tgt are anal...
[truncated]

@jdenny-ornl
Copy link
Collaborator Author

The changes in this PR are probably further motivation for the idea of replacing the getOriginalDef call with a getSource call. However, before that rewrite, I'm hoping this PR is simple enough that we can land it first as it is the last PR in my series and achieves an actual performance benefit.

@Renaud-K
Copy link
Contributor

It is further motivation for using getSource but I would be ok to have this change first.

It is also further motivation to introduce a SourceKind::Emboxed because the SourceKind of the aggregate has probably nothing to do with the SourceKind of the data.
SourceKind::Emboxed would be an indirect source kind for which we are comfortable relying on attributes to determine aliasing.

I don't believe we can meet this guaranty for low level FIR or compiler generated FIR with non-emboxed pointers. Meaning for instance, that some FIR could introduce aliasing without the !fir.ptr type. @jeanPerier , would this be accurate?

I will approve but please wait for Jean's approval as well.

@Renaud-K Renaud-K self-requested a review February 14, 2025 02:29
@Renaud-K
Copy link
Contributor

Actually, there are a lot of TODOs.
You are getting identical source because of the reassignment of v and defOp in

            else if (auto allocOp =
                         def.template getDefiningOp<fir::AllocaOp>()) {
              v = def;
              defOp = v.getDefiningOp();
              type = SourceKind::Allocate;

which makes the aggregate the source. Would you not be getting better results commenting these 2 lines out?

            else if (auto allocOp =
                         def.template getDefiningOp<fir::AllocaOp>()) {
              //v = def;
              //defOp = v.getDefiningOp();
              type = SourceKind::Allocate;

@jdenny-ornl
Copy link
Collaborator Author

Would you not be getting better results commenting these 2 lines out?

            else if (auto allocOp =
                         def.template getDefiningOp<fir::AllocaOp>()) {
              //v = def;
              //defOp = v.getDefiningOp();
              type = SourceKind::Allocate;

I've just added new tests for allocas whose results should be MayAlias (or possibly even MustAlias with more analysis) but become NoAlias if I apply that suggested change. For completeness, I also added similar tests for dummy args and global variables, but that suggested change only affects allocas of course.

For a dummy arg or global variable, the source of a load from it is the dummy arg or the global variable itself. That suggested change adjusts the alloca case so the source is the load instead. That means two different loads from the same alloca have different sources, and so we don't detect aliasing as we should.

@Renaud-K
Copy link
Contributor

Looks good. Jean is off this week but I think you can merge your change.

@jdenny-ornl jdenny-ornl merged commit e7bf54d into llvm:main Feb 19, 2025
8 checks passed
@jdenny-ornl
Copy link
Collaborator Author

Thanks for the review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:fir-hlfir flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants