Skip to content
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

[flang][hlfir] refine hlfir.assign side effects #113319

Merged
merged 1 commit into from
Oct 23, 2024

Conversation

jeanPerier
Copy link
Contributor

hlfir.assign currently has the MemoryEffects<[MemWrite] which makes it look like it can write to anything. This is good for some cases where the assign effect cannot be precisely described through the MLIR side effect API (e.g., when the LHS is a descriptor and it is not possible to get an OpOperand describing the data address, or when derived type are involved and finalization could be called, or user defined assignment for some components). For the most common case of hlfir.assign on intrinsic types without whole allocatable LHS, this is pessimistic.

This patch implements a finer description of the side effects when possible, and also adds the proper read/allocate/free effects when relevant.

The ultimate goal is to suppress the generation of temporary for the LHS address when dealing with an assignment to a vector subscripted LHS where the vector subscript is an array constructor that does not refer to the LHS (as in x([a,b]) = y).

Two more patches will follow to enable this.

@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir labels Oct 22, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 22, 2024

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

Author: None (jeanPerier)

Changes

hlfir.assign currently has the MemoryEffects&lt;[MemWrite] which makes it look like it can write to anything. This is good for some cases where the assign effect cannot be precisely described through the MLIR side effect API (e.g., when the LHS is a descriptor and it is not possible to get an OpOperand describing the data address, or when derived type are involved and finalization could be called, or user defined assignment for some components). For the most common case of hlfir.assign on intrinsic types without whole allocatable LHS, this is pessimistic.

This patch implements a finer description of the side effects when possible, and also adds the proper read/allocate/free effects when relevant.

The ultimate goal is to suppress the generation of temporary for the LHS address when dealing with an assignment to a vector subscripted LHS where the vector subscript is an array constructor that does not refer to the LHS (as in x([a,b]) = y).

Two more patches will follow to enable this.


Full diff: https://github.com/llvm/llvm-project/pull/113319.diff

3 Files Affected:

  • (modified) flang/include/flang/Optimizer/HLFIR/HLFIROps.td (+2-2)
  • (modified) flang/lib/Optimizer/HLFIR/IR/HLFIROps.cpp (+56)
  • (added) flang/test/HLFIR/assign-side-effects.fir (+31)
diff --git a/flang/include/flang/Optimizer/HLFIR/HLFIROps.td b/flang/include/flang/Optimizer/HLFIR/HLFIROps.td
index fdf0db9d3c75de..b162af55d66bb4 100644
--- a/flang/include/flang/Optimizer/HLFIR/HLFIROps.td
+++ b/flang/include/flang/Optimizer/HLFIR/HLFIROps.td
@@ -130,7 +130,7 @@ def hlfir_DeclareOp : hlfir_Op<"declare", [AttrSizedOperandSegments,
   let hasVerifier = 1;
 }
 
-def fir_AssignOp : hlfir_Op<"assign", [MemoryEffects<[MemWrite]>]> {
+def fir_AssignOp : hlfir_Op<"assign", [DeclareOpInterfaceMethods<MemoryEffectsOpInterface>]> {
   let summary = "Assign an expression or variable value to a Fortran variable";
 
   let description = [{
@@ -166,7 +166,7 @@ def fir_AssignOp : hlfir_Op<"assign", [MemoryEffects<[MemWrite]>]> {
   }];
 
   let arguments = (ins AnyFortranEntity:$rhs,
-                   Arg<AnyFortranVariable, "", [MemWrite]>:$lhs,
+                   AnyFortranVariable:$lhs,
                    UnitAttr:$realloc,
                    UnitAttr:$keep_lhs_length_if_realloc,
                    UnitAttr:$temporary_lhs);
diff --git a/flang/lib/Optimizer/HLFIR/IR/HLFIROps.cpp b/flang/lib/Optimizer/HLFIR/IR/HLFIROps.cpp
index ed301c74c9eded..b593383ff2848d 100644
--- a/flang/lib/Optimizer/HLFIR/IR/HLFIROps.cpp
+++ b/flang/lib/Optimizer/HLFIR/IR/HLFIROps.cpp
@@ -90,6 +90,62 @@ llvm::LogicalResult hlfir::AssignOp::verify() {
   return mlir::success();
 }
 
+void hlfir::AssignOp::getEffects(
+    llvm::SmallVectorImpl<
+        mlir::SideEffects::EffectInstance<mlir::MemoryEffects::Effect>>
+        &effects) {
+  mlir::OpOperand &rhs = getRhsMutable();
+  mlir::OpOperand &lhs = getLhsMutable();
+  mlir::Type rhsType = getRhs().getType();
+  mlir::Type lhsType = getLhs().getType();
+  if (mlir::isa<fir::RecordType>(hlfir::getFortranElementType(lhsType))) {
+    // For derived type assignments, set unknown read/write effects since it
+    // is not known here if user defined finalization is needed, and also
+    // because allocatable components may lead to "deeper" read/write effects
+    // that cannot be described with this API.
+    effects.emplace_back(mlir::MemoryEffects::Read::get(),
+                         mlir::SideEffects::DefaultResource::get());
+    effects.emplace_back(mlir::MemoryEffects::Write::get(),
+                         mlir::SideEffects::DefaultResource::get());
+  } else {
+    // Read effect when RHS is a variable.
+    if (hlfir::isFortranVariableType(rhsType)) {
+      if (hlfir::isBoxAddressType(rhsType)) {
+        // Unknown read effect if the RHS is a descriptor since the read effect
+        // on the data cannot be described.
+        effects.emplace_back(mlir::MemoryEffects::Read::get(),
+                             mlir::SideEffects::DefaultResource::get());
+      } else {
+        effects.emplace_back(mlir::MemoryEffects::Read::get(), &rhs,
+                             mlir::SideEffects::DefaultResource::get());
+      }
+    }
+
+    // Write effects on LHS.
+    if (hlfir::isBoxAddressType(lhsType)) {
+      //  If the LHS is a descriptor, the descriptor will be read and the data
+      //  write cannot be described in this API (and the descriptor may be
+      //  written to in case of realloc, which is covered by the unknown write
+      //  effect.
+      effects.emplace_back(mlir::MemoryEffects::Read::get(), &lhs,
+                           mlir::SideEffects::DefaultResource::get());
+      effects.emplace_back(mlir::MemoryEffects::Write::get(),
+                           mlir::SideEffects::DefaultResource::get());
+    } else {
+      effects.emplace_back(mlir::MemoryEffects::Write::get(), &lhs,
+                           mlir::SideEffects::DefaultResource::get());
+    }
+  }
+
+  if (getRealloc()) {
+    // Reallocation of the data cannot be precisely described by this API.
+    effects.emplace_back(mlir::MemoryEffects::Free::get(),
+                         mlir::SideEffects::DefaultResource::get());
+    effects.emplace_back(mlir::MemoryEffects::Allocate::get(),
+                         mlir::SideEffects::DefaultResource::get());
+  }
+}
+
 //===----------------------------------------------------------------------===//
 // DeclareOp
 //===----------------------------------------------------------------------===//
diff --git a/flang/test/HLFIR/assign-side-effects.fir b/flang/test/HLFIR/assign-side-effects.fir
new file mode 100644
index 00000000000000..dfd1c5886e4fa2
--- /dev/null
+++ b/flang/test/HLFIR/assign-side-effects.fir
@@ -0,0 +1,31 @@
+// Test side effects of hlfir.assign op.
+// RUN: fir-opt %s --test-side-effects --verify-diagnostics
+
+func.func @test1(%x: !fir.ref<i32>, %i: i32) {
+  // expected-remark @below {{found an instance of 'write' on a op operand, on resource '<Default>'}}
+  hlfir.assign %i to %x : i32, !fir.ref<i32>
+  return
+}
+
+func.func @test2(%x: !fir.ref<i32>, %y: !fir.ref<i32>) {
+  // expected-remark @below {{found an instance of 'write' on a op operand, on resource '<Default>'}}
+  // expected-remark @below {{found an instance of 'read' on a op operand, on resource '<Default>'}}
+  hlfir.assign %y to %x : !fir.ref<i32>, !fir.ref<i32>
+  return
+}
+
+func.func @test3(%x: !fir.ref<!fir.type<t>>, %y: !fir.ref<!fir.type<t>>) {
+  // expected-remark @below {{found an instance of 'write' on resource '<Default>'}}
+  // expected-remark @below {{found an instance of 'read' on resource '<Default>'}}
+  hlfir.assign %y to %x : !fir.ref<!fir.type<t>>, !fir.ref<!fir.type<t>>
+  return
+}
+
+func.func @test4(%x: !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>, %y: !fir.box<!fir.array<?xi32>>) {
+  // expected-remark @below {{found an instance of 'read' on a op operand, on resource '<Default>'}}
+  // expected-remark @below {{found an instance of 'write' on resource '<Default>'}}
+  // expected-remark @below {{found an instance of 'free' on resource '<Default>'}}
+  // expected-remark @below {{found an instance of 'allocate' on resource '<Default>'}}
+  hlfir.assign %y to %x realloc : !fir.box<!fir.array<?xi32>>, !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>
+  return
+}

Copy link
Contributor

@tblah tblah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@jeanPerier jeanPerier merged commit d89c1db into llvm:main Oct 23, 2024
11 checks passed
@jeanPerier jeanPerier deleted the optim-vec-sub-1 branch October 23, 2024 10:33
@frobtech frobtech mentioned this pull request Oct 25, 2024
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.

4 participants