[flang-commits] [flang] Use the RHS of a pointer assignment inside of FORALL if it is already of `boxType` instead of `convertToBox` again. (PR #165771)

Daniel Chen via flang-commits flang-commits at lists.llvm.org
Fri Oct 31 07:22:48 PDT 2025


https://github.com/DanielCChen updated https://github.com/llvm/llvm-project/pull/165771

>From c4b4b43d4f916836fb1c84a4793225499d1b44c0 Mon Sep 17 00:00:00 2001
From: cdchen-ca <cdchen at ca.ibm.com>
Date: Thu, 30 Oct 2025 15:48:37 -0400
Subject: [PATCH 1/2] Use the RHS of a pointer assignment inside of FORALL if
 it is already a box instead of convertToBox again.

---
 .../Optimizer/Builder/TemporaryStorage.cpp    | 20 ++++++---
 .../forall-pointer-assignment-codegen.fir     |  6 +--
 ...phic.f90 => forall-pointer-assignment.f90} | 44 ++++++++++++++++++-
 3 files changed, 58 insertions(+), 12 deletions(-)
 rename flang/test/Lower/{forall-polymorphic.f90 => forall-pointer-assignment.f90} (82%)

diff --git a/flang/lib/Optimizer/Builder/TemporaryStorage.cpp b/flang/lib/Optimizer/Builder/TemporaryStorage.cpp
index 7e329e357d7b3..4fb546e9cb95d 100644
--- a/flang/lib/Optimizer/Builder/TemporaryStorage.cpp
+++ b/flang/lib/Optimizer/Builder/TemporaryStorage.cpp
@@ -258,13 +258,19 @@ void fir::factory::AnyVariableStack::pushValue(mlir::Location loc,
                                                fir::FirOpBuilder &builder,
                                                mlir::Value variable) {
   hlfir::Entity entity{variable};
-  mlir::Type storageElementType =
-      hlfir::getFortranElementType(retValueBox.getType());
-  auto [box, maybeCleanUp] =
-      hlfir::convertToBox(loc, builder, entity, storageElementType);
-  fir::runtime::genPushDescriptor(loc, builder, opaquePtr, fir::getBase(box));
-  if (maybeCleanUp)
-    (*maybeCleanUp)();
+  if (mlir::isa<fir::BaseBoxType>(entity.getType())) {
+    mlir::Value box =
+        hlfir::genVariableBox(loc, builder, entity, entity.getBoxType());
+    fir::runtime::genPushDescriptor(loc, builder, opaquePtr, fir::getBase(box));
+  } else {
+    mlir::Type storageElementType =
+        hlfir::getFortranElementType(retValueBox.getType());
+    auto [box, maybeCleanUp] =
+        hlfir::convertToBox(loc, builder, entity, storageElementType);
+    fir::runtime::genPushDescriptor(loc, builder, opaquePtr, fir::getBase(box));
+    if (maybeCleanUp)
+      (*maybeCleanUp)();
+  }
 }
 
 void fir::factory::AnyVariableStack::resetFetchPosition(
diff --git a/flang/test/HLFIR/order_assignments/forall-pointer-assignment-codegen.fir b/flang/test/HLFIR/order_assignments/forall-pointer-assignment-codegen.fir
index 1d198765aff9e..855b62ca0ed39 100644
--- a/flang/test/HLFIR/order_assignments/forall-pointer-assignment-codegen.fir
+++ b/flang/test/HLFIR/order_assignments/forall-pointer-assignment-codegen.fir
@@ -91,10 +91,8 @@ func.func @test_need_to_save_rhs(%n: i64, %arg1: !fir.box<!fir.array<?x!ptr_wrap
 // CHECK:             %[[VAL_21:.*]] = hlfir.designate %[[VAL_6]]#0 (%[[VAL_20]])  : (!fir.box<!fir.array<?x!fir.type<ptr_wrapper{p:!fir.box<!fir.ptr<!fir.type<t{i:i64}>>>}>>>, i64) -> !fir.ref<!fir.type<ptr_wrapper{p:!fir.box<!fir.ptr<!fir.type<t{i:i64}>>>}>>
 // CHECK:             %[[VAL_22:.*]] = hlfir.designate %[[VAL_21]]{"p"}   {fortran_attrs = #fir.var_attrs<pointer>} : (!fir.ref<!fir.type<ptr_wrapper{p:!fir.box<!fir.ptr<!fir.type<t{i:i64}>>>}>>) -> !fir.ref<!fir.box<!fir.ptr<!fir.type<t{i:i64}>>>>
 // CHECK:             %[[VAL_23:.*]] = fir.load %[[VAL_22]] : !fir.ref<!fir.box<!fir.ptr<!fir.type<t{i:i64}>>>>
-// CHECK:             %[[VAL_24:.*]] = fir.box_addr %[[VAL_23]] : (!fir.box<!fir.ptr<!fir.type<t{i:i64}>>>) -> !fir.ptr<!fir.type<t{i:i64}>>
-// CHECK:             %[[VAL_25:.*]] = fir.embox %[[VAL_24]] : (!fir.ptr<!fir.type<t{i:i64}>>) -> !fir.box<!fir.type<t{i:i64}>>
-// CHECK:             %[[VAL_26:.*]] = fir.convert %[[VAL_25]] : (!fir.box<!fir.type<t{i:i64}>>) -> !fir.box<none>
-// CHECK:             fir.call @_FortranAPushDescriptor(%[[VAL_16]], %[[VAL_26]]) : (!fir.llvm_ptr<i8>, !fir.box<none>) -> ()
+// CHECK:             %[[VAL_24:.*]] = fir.convert %[[VAL_23]] : (!fir.box<!fir.ptr<!fir.type<t{i:i64}>>>) -> !fir.box<none>
+// CHECK:             fir.call @_FortranAPushDescriptor(%[[VAL_16]], %[[VAL_24]]) : (!fir.llvm_ptr<i8>, !fir.box<none>) -> ()
 // CHECK:           }
 // CHECK:           %[[VAL_27:.*]] = fir.convert %[[VAL_4]] : (i64) -> index
 // CHECK:           %[[VAL_28:.*]] = fir.convert %[[VAL_0]] : (i64) -> index
diff --git a/flang/test/Lower/forall-polymorphic.f90 b/flang/test/Lower/forall-pointer-assignment.f90
similarity index 82%
rename from flang/test/Lower/forall-polymorphic.f90
rename to flang/test/Lower/forall-pointer-assignment.f90
index 2b7a51f9b549a..64e5eafd75a2e 100644
--- a/flang/test/Lower/forall-polymorphic.f90
+++ b/flang/test/Lower/forall-pointer-assignment.f90
@@ -1,4 +1,4 @@
-! Test lower of FORALL polymorphic pointer assignment 
+! Test lower of FORALL pointer assignment 
 ! RUN: bbc -emit-fir %s -o - | FileCheck %s
 
 !! Test when LHS is polymorphic and RHS is not polymorphic
@@ -87,3 +87,45 @@ subroutine forallPolymorphic2(Tar1)
 
   end subroutine forallPolymorphic2
 
+!! Test the LHS of a pointer assignment gets the isAllocatable flag from the
+!! RHS that is a function reference.
+! CHECK-LABEL: c.func @_QPforallpointerassignment1
+  subroutine forallPointerAssignment1()
+    type base
+        real, pointer :: data => null()
+    end type
+
+    interface
+      pure function makeData (i)
+        real, pointer :: makeData
+        integer*4, intent(in) :: i
+      end function
+    end interface
+
+    type(base) :: co1(10)
+
+    forall (i=1:10)
+        co1(i)%data => makeData (i)
+    end forall
+
+! CHECK: %[[V_3:[0-9]+]] = fir.alloca i64
+! CHECK: %[[V_3:[0-9]+]] = fir.alloca i32 {bindc_name = "i"}
+! CHECK: %[[V_4:[0-9]+]] = fir.alloca !fir.box<!fir.ptr<f32>> {bindc_name = ".result"}
+! CHECK: %[[V_25:[0-9]+]] = fir.convert %c1_i32 : (i32) -> index
+! CHECK: %[[V_26:[0-9]+]] = fir.convert %c10_i32 : (i32) -> index
+! CHECK: %[[V_27:[0-9]+]] = fir.address_of(
+! CHECK: %[[V_28:[0-9]+]] = fir.convert %[[V_27]] : (!fir.ref<!fir.char<1,82>>) -> !fir.ref<i8>
+! CHECK: %[[V_29:[0-9]+]] = fir.call @_FortranACreateDescriptorStack(%[[V_28]], %c{{.*}}) : (!fir.ref<i8>, i32) -> !fir.llvm_ptr<i8>
+! CHECK: fir.do_loop %arg0 = %[[V_25]] to %[[V_26]] step %c1
+! CHECK: {
+! CHECK: %[[V_32:[0-9]+]] = fir.convert %arg0 : (index) -> i32
+! CHECK: fir.store %[[V_32]] to %[[V_3]] : !fir.ref<i32>
+! CHECK: %[[V_33:[0-9]+]] = fir.call @_QPmakedata(%[[V_3]]) proc_attrs<pure> fastmath<contract> : (!fir.ref<i32>) -> !fir.box<!fir.ptr<f32>>
+! CHECK: fir.save_result %[[V_33]] to %[[V_4]] : !fir.box<!fir.ptr<f32>>, !fir.ref<!fir.box<!fir.ptr<f32>>>
+! CHECK: %[[V_34:[0-9]+]] = fir.declare %[[V_4]] {uniq_name = ".tmp.func_result"} : (!fir.ref<!fir.box<!fir.ptr<f32>>>) -> !fir.ref<!fir.box<!fir.ptr<f32>>>
+! CHECK: %[[V_35:[0-9]+]] = fir.load %[[V_34]] : !fir.ref<!fir.box<!fir.ptr<f32>>>
+! CHECK: %[[V_36:[0-9]+]] = fir.convert %[[V_35]] : (!fir.box<!fir.ptr<f32>>) -> !fir.box<none>
+! CHECK: fir.call @_FortranAPushDescriptor(%[[V_29]], %[[V_36]]) : (!fir.llvm_ptr<i8>, !fir.box<none>) -> ()
+! CHECK: }
+
+  end subroutine forallPointerAssignment1

>From 4427cdbeaf787e59bc6553b969e3f4b0c92e3211 Mon Sep 17 00:00:00 2001
From: cdchen-ca <cdchen at ca.ibm.com>
Date: Fri, 31 Oct 2025 10:20:14 -0400
Subject: [PATCH 2/2] To address review comments to simplify the code.

---
 flang/lib/Optimizer/Builder/TemporaryStorage.cpp | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/flang/lib/Optimizer/Builder/TemporaryStorage.cpp b/flang/lib/Optimizer/Builder/TemporaryStorage.cpp
index 4fb546e9cb95d..5db40aff91878 100644
--- a/flang/lib/Optimizer/Builder/TemporaryStorage.cpp
+++ b/flang/lib/Optimizer/Builder/TemporaryStorage.cpp
@@ -258,19 +258,9 @@ void fir::factory::AnyVariableStack::pushValue(mlir::Location loc,
                                                fir::FirOpBuilder &builder,
                                                mlir::Value variable) {
   hlfir::Entity entity{variable};
-  if (mlir::isa<fir::BaseBoxType>(entity.getType())) {
-    mlir::Value box =
-        hlfir::genVariableBox(loc, builder, entity, entity.getBoxType());
-    fir::runtime::genPushDescriptor(loc, builder, opaquePtr, fir::getBase(box));
-  } else {
-    mlir::Type storageElementType =
-        hlfir::getFortranElementType(retValueBox.getType());
-    auto [box, maybeCleanUp] =
-        hlfir::convertToBox(loc, builder, entity, storageElementType);
-    fir::runtime::genPushDescriptor(loc, builder, opaquePtr, fir::getBase(box));
-    if (maybeCleanUp)
-      (*maybeCleanUp)();
-  }
+  mlir::Value box =
+      hlfir::genVariableBox(loc, builder, entity, entity.getBoxType());
+  fir::runtime::genPushDescriptor(loc, builder, opaquePtr, fir::getBase(box));
 }
 
 void fir::factory::AnyVariableStack::resetFetchPosition(



More information about the flang-commits mailing list