[NFC][flang] Introduce FortranObjectViewOpInterface. #166841

vzakhari · 2025-11-06T20:48:15Z

This patch adds initial version of FortranObjectViewOpInterface
that helps walking def-use chains containing "pass-through"
operations (like fir.convert, etc.). The new interface is used
in FIR AliasAnalysis to demonstrate potential usage (I know we have
such walks elsewhere in Flang, but I am only changing FIR AliasAnalysis
in this patch).

This is an NFC change. I noticed that if I remove followBoxData
code there are no failing LIT tests, but I decided to keep it
in order to keep the change looking more like NFC.

This change is a follow-up on the discussion in #164020:
it is unclear if the FortranObjectViewOpInterface methods and their
usage, as in this patch, apply to the ViewLike operations that
use the core MLIR ViewLikeOpInterface. So this patch is the path
towards simplifying Flang code while also enabling a future discussion
about having such an interface in core MLIR.

This patch adds initial version of `FortranObjectViewOpInterface` that helps walking def-use chains containing "pass-through" operations (like `fir.convert`, etc.). The new interface is used in FIR AliasAnalysis to demonstrate potential usage (I know we have such walks elsewhere in Flang, but I am only changing FIR AliasAnalysis in this patch). This is an NFC change. I noticed that if I remove followBoxData code there are no failing LIT tests, but I decided to keep it in order to keep the change looking more like NFC. This change is a follow-up on the discussion in llvm#164020: it is unclear if the `FortranObjectViewOpInterface` methods and their usage, as in this patch, apply to the ViewLike operations that use the core MLIR `ViewLikeOpInterface`. So this patch is the path towards simplifying Flang code while also enabling a future discussion about having such an interface in core MLIR.

llvmbot · 2025-11-06T20:48:53Z

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

Author: Slava Zakharin (vzakhari)

Changes

This patch adds initial version of FortranObjectViewOpInterface
that helps walking def-use chains containing "pass-through"
operations (like fir.convert, etc.). The new interface is used
in FIR AliasAnalysis to demonstrate potential usage (I know we have
such walks elsewhere in Flang, but I am only changing FIR AliasAnalysis
in this patch).

This is an NFC change. I noticed that if I remove followBoxData
code there are no failing LIT tests, but I decided to keep it
in order to keep the change looking more like NFC.

This change is a follow-up on the discussion in #164020:
it is unclear if the FortranObjectViewOpInterface methods and their
usage, as in this patch, apply to the ViewLike operations that
use the core MLIR ViewLikeOpInterface. So this patch is the path
towards simplifying Flang code while also enabling a future discussion
about having such an interface in core MLIR.

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

6 Files Affected:

(modified) flang/include/flang/Optimizer/Dialect/FIROps.td (+53-8)
(modified) flang/include/flang/Optimizer/Dialect/FortranVariableInterface.td (+55)
(modified) flang/include/flang/Optimizer/HLFIR/HLFIROps.td (+22-7)
(modified) flang/lib/Optimizer/Analysis/AliasAnalysis.cpp (+61-47)
(modified) flang/lib/Optimizer/Dialect/FIROps.cpp (+33)
(modified) flang/lib/Optimizer/HLFIR/IR/HLFIROps.cpp (+6)

diff --git a/flang/include/flang/Optimizer/Dialect/FIROps.td b/flang/include/flang/Optimizer/Dialect/FIROps.td
index bae52d63fda45..87e0ce33fa78d 100644
--- a/flang/include/flang/Optimizer/Dialect/FIROps.td
+++ b/flang/include/flang/Optimizer/Dialect/FIROps.td
@@ -806,7 +806,8 @@ def fir_HasValueOp : fir_Op<"has_value", [Terminator, HasParent<"GlobalOp">]> {
 // Operations on !fir.box<T> type objects
 //===----------------------------------------------------------------------===//
 
-def fir_EmboxOp : fir_Op<"embox", [NoMemoryEffect, AttrSizedOperandSegments]> {
+def fir_EmboxOp : fir_Op<"embox", [NoMemoryEffect, AttrSizedOperandSegments,
+                                   fir_FortranObjectViewOpInterface]> {
   let summary = "boxes a given reference and (optional) dimension information";
 
   let description = [{
@@ -870,10 +871,14 @@ def fir_EmboxOp : fir_Op<"embox", [NoMemoryEffect, AttrSizedOperandSegments]> {
       return 1 + (getShape() ? 1 : 0) + (getSlice() ? 1 : 0)
           + numLenParams();
     }
+    // FortranObjectViewOpInterface methods:
+    mlir::Value getViewSource(mlir::OpResult) { return getMemref(); }
+    std::optional<std::int64_t> getViewOffset(mlir::OpResult);
   }];
 }
 
-def fir_ReboxOp : fir_Op<"rebox", [NoMemoryEffect, AttrSizedOperandSegments]> {
+def fir_ReboxOp : fir_Op<"rebox", [NoMemoryEffect, AttrSizedOperandSegments,
+                                   fir_FortranObjectViewOpInterface]> {
   let summary =
       "create a box given another box and (optional) dimension information";
 
@@ -923,6 +928,12 @@ def fir_ReboxOp : fir_Op<"rebox", [NoMemoryEffect, AttrSizedOperandSegments]> {
   }];
 
   let hasVerifier = 1;
+
+  let extraClassDeclaration = [{
+    // FortranObjectViewOpInterface methods:
+    mlir::Value getViewSource(mlir::OpResult) { return getBox(); }
+    std::optional<std::int64_t> getViewOffset(mlir::OpResult);
+  }];
 }
 
 def fir_ReboxAssumedRankOp : fir_Op<"rebox_assumed_rank",
@@ -1071,7 +1082,9 @@ def fir_UnboxProcOp : fir_SimpleOp<"unboxproc", [NoMemoryEffect]> {
   let results = (outs FunctionType, fir_ReferenceType:$refTuple);
 }
 
-def fir_BoxAddrOp : fir_SimpleOneResultOp<"box_addr", [NoMemoryEffect]> {
+def fir_BoxAddrOp
+    : fir_SimpleOneResultOp<"box_addr", [NoMemoryEffect,
+                                         fir_FortranObjectViewOpInterface]> {
   let summary = "return a memory reference to the boxed value";
 
   let description = [{
@@ -1094,6 +1107,12 @@ def fir_BoxAddrOp : fir_SimpleOneResultOp<"box_addr", [NoMemoryEffect]> {
   let hasFolder = 1;
 
   let builders = [OpBuilder<(ins "mlir::Value":$val)>];
+
+  let extraClassDeclaration = [{
+    // FortranObjectViewOpInterface methods:
+    mlir::Value getViewSource(mlir::OpResult) { return getVal(); }
+    std::optional<std::int64_t> getViewOffset(mlir::OpResult);
+  }];
 }
 
 def fir_BoxCharLenOp : fir_SimpleOp<"boxchar_len", [NoMemoryEffect]> {
@@ -1766,8 +1785,9 @@ def fir_ArrayMergeStoreOp : fir_Op<"array_merge_store",
 // Record and array type operations
 //===----------------------------------------------------------------------===//
 
-def fir_ArrayCoorOp : fir_Op<"array_coor",
-    [NoMemoryEffect, AttrSizedOperandSegments]> {
+def fir_ArrayCoorOp
+    : fir_Op<"array_coor", [NoMemoryEffect, AttrSizedOperandSegments,
+                            fir_FortranObjectViewOpInterface]> {
 
   let summary = "Find the coordinate of an element of an array";
 
@@ -1810,9 +1830,16 @@ def fir_ArrayCoorOp : fir_Op<"array_coor",
 
   let hasVerifier = 1;
   let hasCanonicalizer = 1;
+  let extraClassDeclaration = [{
+    // FortranObjectViewOpInterface methods:
+    mlir::Value getViewSource(mlir::OpResult) { return getMemref(); }
+    std::optional<std::int64_t> getViewOffset(mlir::OpResult);
+  }];
 }
 
-def fir_CoordinateOp : fir_Op<"coordinate_of", [NoMemoryEffect]> {
+def fir_CoordinateOp
+    : fir_Op<"coordinate_of", [NoMemoryEffect,
+                               fir_FortranObjectViewOpInterface]> {
 
   let summary = "Finds the coordinate (location) of a value in memory";
 
@@ -1864,6 +1891,10 @@ def fir_CoordinateOp : fir_Op<"coordinate_of", [NoMemoryEffect]> {
   let extraClassDeclaration = [{
     constexpr static int32_t kDynamicIndex = std::numeric_limits<int32_t>::min();
     CoordinateIndicesAdaptor getIndices();
+
+    // FortranObjectViewOpInterface methods:
+    mlir::Value getViewSource(mlir::OpResult) { return getRef(); }
+    std::optional<std::int64_t> getViewOffset(mlir::OpResult);
   }];
 }
 
@@ -2830,7 +2861,8 @@ def fir_VolatileCastOp : fir_SimpleOneResultOp<"volatile_cast", [Pure]> {
 }
 
 def fir_ConvertOp
-    : fir_SimpleOneResultOp<"convert", [NoMemoryEffect, ViewLikeOpInterface]> {
+    : fir_SimpleOneResultOp<"convert", [NoMemoryEffect, ViewLikeOpInterface,
+                                        fir_FortranObjectViewOpInterface]> {
   let summary = "encapsulates all Fortran entity type conversions";
 
   let description = [{
@@ -2868,7 +2900,13 @@ def fir_ConvertOp
     static bool isPointerCompatible(mlir::Type ty);
     static bool canBeConverted(mlir::Type inType, mlir::Type outType);
     static bool areVectorsCompatible(mlir::Type inTy, mlir::Type outTy);
+
+    // ViewLikeOpInterface methods:
     mlir::Value getViewSource() { return getValue(); }
+
+    // FortranObjectViewOpInterface methods:
+    mlir::Value getViewSource(mlir::OpResult) { return getValue(); }
+    std::optional<std::int64_t> getViewOffset(mlir::OpResult) { return 0; }
   }];
   let hasCanonicalizer = 1;
 }
@@ -3221,7 +3259,8 @@ def fir_DeclareOp
     : fir_Op<"declare", [AttrSizedOperandSegments,
                          MemoryEffects<[MemAlloc<DebuggingResource>]>,
                          DeclareOpInterfaceMethods<
-                             fir_FortranVariableStorageOpInterface>]> {
+                             fir_FortranVariableStorageOpInterface>,
+                         fir_FortranObjectViewOpInterface]> {
   let summary = "declare a variable";
 
   let description = [{
@@ -3285,6 +3324,12 @@ def fir_DeclareOp
     attr-dict `:` functional-type(operands, results)
   }];
 
+  let extraClassDeclaration = [{
+    // FortranObjectViewOpInterface methods:
+    mlir::Value getViewSource(mlir::OpResult) { return getMemref(); }
+    std::optional<std::int64_t> getViewOffset(mlir::OpResult) { return 0; }
+  }];
+
   let hasVerifier = 1;
 }
 
diff --git a/flang/include/flang/Optimizer/Dialect/FortranVariableInterface.td b/flang/include/flang/Optimizer/Dialect/FortranVariableInterface.td
index bd65a042e6dba..60a7470813321 100644
--- a/flang/include/flang/Optimizer/Dialect/FortranVariableInterface.td
+++ b/flang/include/flang/Optimizer/Dialect/FortranVariableInterface.td
@@ -265,4 +265,59 @@ def fir_FortranVariableStorageOpInterface
       [{ return detail::verifyFortranVariableStorageOpInterface($_op); }];
 }
 
+def fir_FortranObjectViewOpInterface
+    : OpInterface<"FortranObjectViewOpInterface"> {
+  let description = [{
+    Interface for operations that may produce results that allow accessing
+    objects in memory based on the operands of these operations.
+    For example:
+    ```
+    %0 = fir.convert %x : (!llvm.ptr) -> !fir.llvm_ptr<i32>
+    ```
+    When the result of `fir.convert` is used to access an object in memory,
+    FIR alias analysis may want to pass through `fir.convert` to be able
+    to find the original Fortran object that is being accessed.
+    This interface provides methods that allow discovering information
+    about the accessed object and the characteristics of the resulting
+    "view" of the object, e.g. whether the result and the input
+    access the object from the same location within the object or
+    whether they are offsetted (which may happen, for example, in case of
+    `fir.array_coor` operation).
+  }];
+  let cppNamespace = "::fir";
+
+  let methods =
+      [InterfaceMethod<
+           /*desc=*/
+           [{ Returns the operand which the given OpResult is based on. }],
+           /*retTy=*/"::mlir::Value",
+           /*methodName=*/"getViewSource",
+           /*args=*/(ins "::mlir::OpResult":$resultView),
+           /*methodBody=*/"",
+           /*defaultImplementation=*/[{
+        assert($_op.getOperation() == resultView.getOwner() &&
+               "resultView must be a result of this operation");
+        return $_op.getViewSource(resultView);
+      }]>,
+       InterfaceMethod<
+           /*desc=*/[{
+        Returns the constant offset (in bytes) applied to the corresponding
+        operand to produce the resulting view.
+        If it is not possible to identify the constant offset,
+        the result in nullopt.
+        Note that the offset may be negative, e.g. when addressing
+        an array section with a negative stride.
+      }],
+           /*retTy=*/"std::optional<std::int64_t>",
+           /*methodName=*/"getViewOffset",
+           /*args=*/(ins "::mlir::OpResult":$resultView),
+           /*methodBody=*/"",
+           /*defaultImplementation=*/[{
+        assert($_op.getOperation() == resultView.getOwner() &&
+               "resultView must be a result of this operation");
+        return $_op.getViewOffset(resultView);
+      }]>,
+  ];
+}
+
 #endif  // FORTRANVARIABLEINTERFACE
diff --git a/flang/include/flang/Optimizer/HLFIR/HLFIROps.td b/flang/include/flang/Optimizer/HLFIR/HLFIROps.td
index b7563a2f752f0..7cdf6646babb1 100644
--- a/flang/include/flang/Optimizer/HLFIR/HLFIROps.td
+++ b/flang/include/flang/Optimizer/HLFIR/HLFIROps.td
@@ -39,7 +39,8 @@ def hlfir_DeclareOp
     : hlfir_Op<"declare", [AttrSizedOperandSegments,
                            MemoryEffects<[MemAlloc<DebuggingResource>]>,
                            DeclareOpInterfaceMethods<
-                               fir_FortranVariableStorageOpInterface>]> {
+                               fir_FortranVariableStorageOpInterface>,
+                           fir_FortranObjectViewOpInterface]> {
   let summary = "declare a variable and produce an SSA value that can be used as a variable in HLFIR operations";
 
   let description = [{
@@ -140,6 +141,10 @@ def hlfir_DeclareOp
     /// Given a FIR memory type, and information about non default lower
     /// bounds, get the related HLFIR variable type.
     static mlir::Type getHLFIRVariableType(mlir::Type type, bool hasLowerBounds);
+
+    // FortranObjectViewOpInterface methods:
+    mlir::Value getViewSource(mlir::OpResult) { return getMemref(); }
+    std::optional<std::int64_t> getViewOffset(mlir::OpResult) { return 0; }
   }];
 
   let hasVerifier = 1;
@@ -213,8 +218,11 @@ def fir_AssignOp : hlfir_Op<"assign", [DeclareOpInterfaceMethods<MemoryEffectsOp
   let hasVerifier = 1;
 }
 
-def hlfir_DesignateOp : hlfir_Op<"designate", [AttrSizedOperandSegments,
-    DeclareOpInterfaceMethods<fir_FortranVariableOpInterface>, NoMemoryEffect]> {
+def hlfir_DesignateOp
+    : hlfir_Op<"designate",
+               [AttrSizedOperandSegments,
+                DeclareOpInterfaceMethods<fir_FortranVariableOpInterface>,
+                NoMemoryEffect, fir_FortranObjectViewOpInterface]> {
   let summary = "Designate a Fortran variable";
 
   let description = [{
@@ -278,6 +286,9 @@ def hlfir_DesignateOp : hlfir_Op<"designate", [AttrSizedOperandSegments,
     void setFortranAttrs(fir::FortranVariableFlagsEnum flags) {
       this->setFortranAttrs(std::optional<fir::FortranVariableFlagsEnum>(flags));
     }
+    // FortranObjectViewOpInterface methods:
+    mlir::Value getViewSource(mlir::OpResult) { return getMemref(); }
+    std::optional<std::int64_t> getViewOffset(mlir::OpResult);
   }];
 
   let builders = [
@@ -938,8 +949,9 @@ def hlfir_EndAssociateOp : hlfir_Op<"end_associate", [MemoryEffects<[MemFree]>]>
   let hasVerifier = 1;
 }
 
-def hlfir_AsExprOp : hlfir_Op<"as_expr",
-    [DeclareOpInterfaceMethods<MemoryEffectsOpInterface>]> {
+def hlfir_AsExprOp
+    : hlfir_Op<"as_expr", [DeclareOpInterfaceMethods<MemoryEffectsOpInterface>,
+                           fir_FortranObjectViewOpInterface]> {
   let summary = "Take the value of an array, character or derived variable";
 
   let description = [{
@@ -961,8 +973,11 @@ def hlfir_AsExprOp : hlfir_Op<"as_expr",
   let results = (outs hlfir_ExprType);
 
   let extraClassDeclaration = [{
-      // Is this a "move" ?
-      bool isMove() { return getMustFree() != mlir::Value{}; }
+    // Is this a "move" ?
+    bool isMove() { return getMustFree() != mlir::Value{}; }
+    // FortranObjectViewOpInterface methods:
+    mlir::Value getViewSource(mlir::OpResult) { return getVar(); }
+    std::optional<std::int64_t> getViewOffset(mlir::OpResult) { return 0; }
   }];
 
   let assemblyFormat = [{
diff --git a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
index 73ddd1ff80126..a45f6f7b8ea89 100644
--- a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
+++ b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
@@ -214,6 +214,17 @@ AliasResult AliasAnalysis::alias(Source lhsSrc, Source rhsSrc, mlir::Value lhs,
                  << "  aliasing because same source kind and origin\n");
       if (approximateSource)
         return AliasResult::MayAlias;
+      // One should be careful about relying on MustAlias.
+      // The LLVM definition implies that the two MustAlias
+      // memory objects start at exactly the same location.
+      // With Fortran array slices two objects may have
+      // the same starting location, but otherwise represent
+      // partially overlapping memory locations, e.g.:
+      //   integer :: a(10)
+      //   ... a(5:1:-1) ! starts at a(5) and addresses a(5), ..., a(1)
+      //   ... a(5:10:1) ! starts at a(5) and addresses a(5), ..., a(10)
+      // The current implementation of FIR alias analysis will always
+      // return MayAlias for such cases.
       return AliasResult::MustAlias;
     }
     // If one value is the address of a composite, and if the other value is the
@@ -534,13 +545,16 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v,
   Source::Attributes attributes;
   mlir::Operation *instantiationPoint{nullptr};
   while (defOp && !breakFromLoop) {
-    ty = defOp->getResultTypes()[0];
+    // Operations may have multiple results, so we need to analyze
+    // the result for which the source is queried.
+    auto opResult = mlir::cast<OpResult>(v);
+    assert(opResult.getOwner() == defOp && "v must be a result of defOp");
+    ty = opResult.getType();
     llvm::TypeSwitch<Operation *>(defOp)
-        .Case<hlfir::AsExprOp>([&](auto op) {
-          v = op.getVar();
-          defOp = v.getDefiningOp();
-        })
         .Case<hlfir::AssociateOp>([&](auto op) {
+          assert(opResult != op.getMustFreeStrorageFlag() &&
+                 "MustFreeStorageFlag result is not an aliasing candidate");
+
           mlir::Value source = op.getSource();
           if (fir::isa_trivial(source.getType())) {
             // Trivial values will always use distinct temp memory,
@@ -559,11 +573,6 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v,
           type = SourceKind::Allocate;
           breakFromLoop = true;
         })
-        .Case<fir::ConvertOp>([&](auto op) {
-          // Skip ConvertOp's and track further through the operand.
-          v = op->getOperand(0);
-          defOp = v.getDefiningOp();
-        })
         .Case<fir::PackArrayOp>([&](auto op) {
           // The packed array is not distinguishable from the original
           // array, so skip PackArrayOp and track further through
@@ -572,28 +581,6 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v,
           defOp = v.getDefiningOp();
           approximateSource = true;
         })
-        .Case<fir::BoxAddrOp>([&](auto op) {
-          v = op->getOperand(0);
-          defOp = v.getDefiningOp();
-          if (mlir::isa<fir::BaseBoxType>(v.getType()))
-            followBoxData = true;
-        })
-        .Case<fir::ArrayCoorOp, fir::CoordinateOp>([&](auto op) {
-          if (isPointerReference(ty))
-            attributes.set(Attribute::Pointer);
-          v = op->getOperand(0);
-          defOp = v.getDefiningOp();
-          if (mlir::isa<fir::BaseBoxType>(v.getType()))
-            followBoxData = true;
-          approximateSource = true;
-        })
-        .Case<fir::EmboxOp, fir::ReboxOp>([&](auto op) {
-          if (followBoxData) {
-            v = op->getOperand(0);
-            defOp = v.getDefiningOp();
-          } else
-            breakFromLoop = true;
-        })
         .Case<fir::LoadOp>([&](auto op) {
           // If load is inside target and it points to mapped item,
           // continue tracking.
@@ -663,6 +650,9 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v,
           breakFromLoop = true;
         })
         .Case<hlfir::DeclareOp, fir::DeclareOp>([&](auto op) {
+          // The declare operations support FortranObjectViewOpInterface,
+          // but their handling is more complex. Maybe we can find better
+          // abstractions to handle them in a general fashion.
           bool isPrivateItem = false;
           if (omp::BlockArgOpenMPOpInterface argIface =
                   dyn_cast<omp::BlockArgOpenMPOpInterface>(op->getParentOp())) {
@@ -713,7 +703,7 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v,
             // currently provide any useful information. The host associated
             // access will end up dereferencing the host association tuple,
             // so we may as well stop right now.
-            v = defOp->getResult(0);
+            v = opResult;
             // TODO: if the host associated variable is a dummy argument
             // of the host, I think, we can treat it as SourceKind::Argument
             // for the purpose of alias analysis inside the internal procedure.
@@ -748,21 +738,45 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v,
           v = op.getMemref();
           defOp = v.getDefiningOp();
         })
-        .Case<hlfir::DesignateOp>([&](auto op) {
-          auto varIf = llvm::cast<fir::FortranVariableOpInterface>(defOp);
-          attributes |= getAttrsFromVariable(varIf);
-          // Track further through the memory indexed into
-          // => if the source arrays/structures don't alias then nor do the
-          //    results of hlfir.designate
-          v = op.getMemref();
+        .Case<fir::FortranObjectViewOpInterface>([&](auto op) {
+          // This case must be located after the cases for concrete
+          // operations that support FortraObjectViewOpInterface,
+          // so that their special handling kicks in.
+
+          // fir.embox/rebox case: this is the only case where we check
+          // for followBoxData.
+          // TODO: it looks like we do not have LIT tests that fail
+          // upon removal of the followBoxData code. We should come up
+          // with a test or remove this code.
+          if (!followBoxData &&
+              (mlir::isa<fir::EmboxOp>(op) || mlir::isa<fir::ReboxOp>(op))) {
+            breakFromLoop = true;
+            return;
+          }
+
+          // Collect attributes from FortranVariableOpInterface operations.
+          if (auto varIf =
+                  mlir::dyn_cast<fir::FortranVariableOpInterface>(defOp))
+            attributes |= getAttrsFromVariable(varIf);
+          // Set Pointer attribute based on the reference type.
+          if (isPointerReference(ty))
+            attributes.set(Attribute::Pointer);
+
+          // Update v to point to the operand that represents the object
+          // referenced by the operation's result.
+          v = op.getViewSource(opResult);
           defOp = v.getDefiningOp();
-          // TODO: there will be some cases which provably don't alias if one
-          // takes into account the component or indices, which are currently
-          // ignored here - leading to false positives
-          // because of this limitation, we need to make sure we never return
-          // MustAlias after going through a designate operation
-          approximateSource = true;
-          if (mlir::isa<fir::BaseBoxType>(v.getType()))
+          // If the input the resulting object references are offsetted,
+          // then set approximateSource.
+          auto offset = op.getViewOffset(opResult);
+          if (!offset || *offset != 0)
+            approximateSource = true...
[truncated]

razvanlupusoru · 2025-11-06T22:06:15Z

flang/include/flang/Optimizer/Dialect/FIROps.td

+  let extraClassDeclaration = [{
+    // FortranObjectViewOpInterface methods:
+    mlir::Value getViewSource(mlir::OpResult) { return getMemref(); }
+    std::optional<std::int64_t> getViewOffset(mlir::OpResult) { return 0; }


Is this correct for common block variables?

Good question! Yes, it should be valid.

For example:

program main common /blk/ a, b real :: a, b a = 1.0 b = 2.0 print *, a, b end program main

%1 = fir.address_of(@blk_) : !fir.ref<!fir.array<8xi8>> %c0 = arith.constant 0 : index %2 = fir.coordinate_of %1, %c0 : (!fir.ref<!fir.array<8xi8>>, index) -> !fir.ref<i8> %3 = fir.convert %2 : (!fir.ref<i8>) -> !fir.ref<f32> %4:2 = hlfir.declare %3 storage(%1[0]) {uniq_name = "_QFEa"} : (!fir.ref<f32>, !fir.ref<!fir.array<8xi8>>) -> (!fir.ref<f32>, !fir.ref<f32>) %c4 = arith.constant 4 : index %5 = fir.coordinate_of %1, %c4 : (!fir.ref<!fir.array<8xi8>>, index) -> !fir.ref<i8> %6 = fir.convert %5 : (!fir.ref<i8>) -> !fir.ref<f32> %7:2 = hlfir.declare %6 storage(%1[4]) {uniq_name = "_QFEb"} : (!fir.ref<f32>, !fir.ref<!fir.array<8xi8>>) -> (!fir.ref<f32>, !fir.ref<f32>)

Basically, [hl]fir.declare does not apply any offset to its operand on its own. The offset is applied to the operands of [hl]fir.declare by fir.coordinate_of. It is also the case for EQUIVALENCE variables.

jeanPerier

Thanks Slava, I just have some concern adding this interface to hlfir.as_expr, otherwise LGTM.

jeanPerier · 2025-11-07T14:43:44Z

flang/include/flang/Optimizer/HLFIR/HLFIROps.td

-    [DeclareOpInterfaceMethods<MemoryEffectsOpInterface>]> {
+def hlfir_AsExprOp
+    : hlfir_Op<"as_expr", [DeclareOpInterfaceMethods<MemoryEffectsOpInterface>,
+                           fir_FortranObjectViewOpInterface]> {


Is this needed?

I do not think AsExpr is a view op, It result should be regarded as a value/tensor with no memory connection to the input.

When it has not been eliminated, it may end-up being implemented as a copy or as use the original storage if it is proven safe.

I think as_expr may have been given some handling in aliasing so that it could have an easier code generation.

I expect this was done so that hlfir.assign optimization would not optimize hlfir.asssign of hlfir.elemental using hlfir.as_expr in case the as_expr source overlaps with the LHS assuming that as_expr codegen could end up using the source.

But I feel the problem should be seen in the other direction, and it is up to the as_expr codegen to prove that it can safely replace the copy by its source.

If possible, I would prefer keeping the ad-hoc handling in FIR aliasing so that we can try to fix this instead of enshrining the assumption that as_expr result is based on its source in the operation definition.

vzakhari requested review from jeanPerier, matthias-springer, razvanlupusoru and tblah November 6, 2025 20:48

llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir labels Nov 6, 2025

razvanlupusoru reviewed Nov 6, 2025

View reviewed changes

jeanPerier reviewed Nov 7, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[NFC][flang] Introduce FortranObjectViewOpInterface. #166841

[NFC][flang] Introduce FortranObjectViewOpInterface. #166841

vzakhari commented Nov 6, 2025

Uh oh!

llvmbot commented Nov 6, 2025

Uh oh!

razvanlupusoru Nov 6, 2025

Uh oh!

vzakhari Nov 7, 2025

Uh oh!

jeanPerier left a comment

Uh oh!

jeanPerier Nov 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[NFC][flang] Introduce FortranObjectViewOpInterface. #166841

Are you sure you want to change the base?

[NFC][flang] Introduce FortranObjectViewOpInterface. #166841

Conversation

vzakhari commented Nov 6, 2025

Uh oh!

llvmbot commented Nov 6, 2025

Uh oh!

razvanlupusoru Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

vzakhari Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

jeanPerier left a comment

Choose a reason for hiding this comment

Uh oh!

jeanPerier Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants