Skip to content

[Flang][OpenMP] Fix crash when loop index var is pointer or allocatable #129717

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 4 commits into from
Mar 6, 2025

Conversation

kiranchandramohan
Copy link
Contributor

@kiranchandramohan kiranchandramohan commented Mar 4, 2025

Use hlfir dereferencing for pointers and allocatables and use hlfir assign. Also, change the code updating IV in lastprivate.

Note: This is a small change. Modifications in existing tests are changes from fir.store to hlfir.assign.

Fixes #121290

Use hlfir dereferencing for pointers and allocatables and use hlfir
assign.
Note: This is a small change. Modifications in existing tests are
changes from fir.store to hlfir.assign.

Fixes llvm#121290
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir flang:openmp labels Mar 4, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 4, 2025

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

@llvm/pr-subscribers-flang-openmp

Author: Kiran Chandramohan (kiranchandramohan)

Changes

Use hlfir dereferencing for pointers and allocatables and use hlfir assign.
Note: This is a small change. Modifications in existing tests are changes from fir.store to hlfir.assign.

Fixes #121290


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

60 Files Affected:

  • (modified) flang/lib/Lower/OpenMP/OpenMP.cpp (+6-2)
  • (modified) flang/test/Lower/OpenMP/DelayedPrivatization/distribute-standalone-private.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/DelayedPrivatization/wsloop.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/Todo/omp-default-clause-inner-loop.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/copyin.f90 (+2-2)
  • (modified) flang/test/Lower/OpenMP/critical.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/default-clause.f90 (+2-2)
  • (modified) flang/test/Lower/OpenMP/generic-loop-rewriting.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/lastprivate-iv.f90 (+2-2)
  • (modified) flang/test/Lower/OpenMP/loop-directive.f90 (+1-1)
  • (added) flang/test/Lower/OpenMP/loop-pointer-variable.f90 (+43)
  • (modified) flang/test/Lower/OpenMP/parallel-private-clause-fixes.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/parallel-private-clause.f90 (+4-4)
  • (modified) flang/test/Lower/OpenMP/parallel-reduction-allocatable-array.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/parallel-reduction-pointer-array.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/parallel-reduction3.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/parallel-wsloop-firstpriv.f90 (+2-2)
  • (modified) flang/test/Lower/OpenMP/parallel-wsloop-lastpriv.f90 (+7-7)
  • (modified) flang/test/Lower/OpenMP/parallel-wsloop.f90 (+8-8)
  • (modified) flang/test/Lower/OpenMP/simd.f90 (+9-9)
  • (modified) flang/test/Lower/OpenMP/stop-stmt-in-region.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/target.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/unstructured.f90 (+3-3)
  • (modified) flang/test/Lower/OpenMP/wsloop-chunks.f90 (+3-3)
  • (modified) flang/test/Lower/OpenMP/wsloop-collapse.f90 (+3-3)
  • (modified) flang/test/Lower/OpenMP/wsloop-monotonic.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/wsloop-nonmonotonic.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-add-byref.f90 (+7-7)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-add.f90 (+7-7)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-allocatable-array-minmax.f90 (+2-2)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-allocatable.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-array-assumed-shape.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-array.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-array2.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-iand-byref.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-iand.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-ieor-byref.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-ieor.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-ior-byref.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-ior.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-logical-and-byref.f90 (+3-3)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-logical-and.f90 (+3-3)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-logical-eqv-byref.f90 (+3-3)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-logical-eqv.f90 (+3-3)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-logical-neqv-byref.f90 (+3-3)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-logical-neqv.f90 (+3-3)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-logical-or-byref.f90 (+3-3)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-logical-or.f90 (+3-3)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-max-byref.f90 (+3-3)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-max.f90 (+3-3)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-min-byref.f90 (+3-3)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-min.f90 (+3-3)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-min2.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-mul-byref.f90 (+7-7)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-mul.f90 (+7-7)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-multiple-clauses.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-pointer.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/wsloop-schedule.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/wsloop-variable.f90 (+6-6)
  • (modified) flang/test/Lower/OpenMP/wsloop.f90 (+3-3)
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 7970d77da8a6b..b1568cc12a05a 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -789,8 +789,12 @@ createAndSetPrivatizedLoopVar(lower::AbstractConverter &converter,
 
   firOpBuilder.restoreInsertionPoint(insPt);
   mlir::Value cvtVal = firOpBuilder.createConvert(loc, tempTy, indexVal);
-  mlir::Operation *storeOp = firOpBuilder.create<fir::StoreOp>(
-      loc, cvtVal, converter.getSymbolAddress(*sym));
+  hlfir::Entity lhs{converter.getSymbolAddress(*sym)};
+
+  lhs = hlfir::derefPointersAndAllocatables(loc, firOpBuilder, lhs);
+
+  mlir::Operation *storeOp =
+      firOpBuilder.create<hlfir::AssignOp>(loc, cvtVal, lhs);
   return storeOp;
 }
 
diff --git a/flang/test/Lower/OpenMP/DelayedPrivatization/distribute-standalone-private.f90 b/flang/test/Lower/OpenMP/DelayedPrivatization/distribute-standalone-private.f90
index 8098cd53e9d2e..d8ec69120468b 100644
--- a/flang/test/Lower/OpenMP/DelayedPrivatization/distribute-standalone-private.f90
+++ b/flang/test/Lower/OpenMP/DelayedPrivatization/distribute-standalone-private.f90
@@ -31,7 +31,7 @@ end subroutine standalone_distribute
 ! CHECK:               %[[VAR_PRIV_DECL:.*]]:2 = hlfir.declare %[[VAR_ARG]]
 ! CHECK:               %[[I_PRIV_DECL:.*]]:2 = hlfir.declare %[[I_ARG]]
 
-! CHECK:               fir.store %{{.*}} to %[[I_PRIV_DECL]]#1 : !fir.ref<i32>
+! CHECK:               hlfir.assign %{{.*}} to %[[I_PRIV_DECL]]#1 : i32, !fir.ref<i32>
 ! CHECK:               %{{.*}} = fir.load %[[VAR_PRIV_DECL]]#0 : !fir.ref<i32>
 ! CHECK:               %{{.*}} = fir.load %[[I_PRIV_DECL]]#0 : !fir.ref<i32>
 ! CHECK:               arith.addi %{{.*}}, %{{.*}} : i32
diff --git a/flang/test/Lower/OpenMP/DelayedPrivatization/wsloop.f90 b/flang/test/Lower/OpenMP/DelayedPrivatization/wsloop.f90
index 221d08669f899..1634e00d34be1 100644
--- a/flang/test/Lower/OpenMP/DelayedPrivatization/wsloop.f90
+++ b/flang/test/Lower/OpenMP/DelayedPrivatization/wsloop.f90
@@ -28,7 +28,7 @@ end subroutine wsloop_private
 ! CHECK:       omp.loop_nest (%[[IV:.*]]) : i32 = {{.*}} {
 ! CHECK:         %[[X_PRIV_DECL:.*]]:2 = hlfir.declare %[[X_ARG]] {uniq_name = "{{.*}}x"}
 ! CHECK:         %[[I_PRIV_DECL:.*]]:2 = hlfir.declare %[[I_ARG]] {uniq_name = "{{.*}}i"}
-! CHECK:         fir.store %[[IV]] to %[[I_PRIV_DECL]]#1
+! CHECK:         hlfir.assign %[[IV]] to %[[I_PRIV_DECL]]#1
 ! CHECK:         %[[X_VAL:.*]] = fir.load %[[X_PRIV_DECL]]#0
 ! CHECK:         %[[I_VAL:.*]] = fir.load %[[I_PRIV_DECL]]#0
 ! CHECK:         %[[ADD_VAL:.*]] = arith.addi %[[X_VAL]], %[[I_VAL]]
diff --git a/flang/test/Lower/OpenMP/Todo/omp-default-clause-inner-loop.f90 b/flang/test/Lower/OpenMP/Todo/omp-default-clause-inner-loop.f90
index 10879c53dc0c5..7447e24d236fa 100644
--- a/flang/test/Lower/OpenMP/Todo/omp-default-clause-inner-loop.f90
+++ b/flang/test/Lower/OpenMP/Todo/omp-default-clause-inner-loop.f90
@@ -14,7 +14,7 @@
 ! CHECK: %[[const_3:.*]] = arith.constant 1 : i32
 ! CHECK: omp.wsloop private(@{{.*}} %{{.*}} -> %[[TEMP:.*]] : !fir.ref<i32>) {
 ! CHECK-NEXT: omp.loop_nest (%[[ARG:.*]]) : i32 = (%[[const_1]]) to (%[[const_2]]) inclusive step (%[[const_3]]) {
-! CHECK: fir.store %[[ARG]] to %[[TEMP]] : !fir.ref<i32>
+! CHECK: hlfir.assign %[[ARG]] to %[[TEMP]] : i32, !fir.ref<i32>
 ! EXPECTED: %[[temp_1:.*]] = fir.load %[[PRIVATE_Z]] : !fir.ref<i32>
 ! CHECK: %[[temp_1:.*]] = fir.load %{{.*}} : !fir.ref<i32>
 ! CHECK: %[[temp_2:.*]] = fir.load %[[TEMP]] : !fir.ref<i32>
diff --git a/flang/test/Lower/OpenMP/copyin.f90 b/flang/test/Lower/OpenMP/copyin.f90
index 0e8fe0eaeed87..f2c7d8c441ccb 100644
--- a/flang/test/Lower/OpenMP/copyin.f90
+++ b/flang/test/Lower/OpenMP/copyin.f90
@@ -161,7 +161,7 @@ subroutine copyin_derived_type()
 ! CHECK:             omp.wsloop private(@{{.*}} %{{.*}} -> %[[VAL_6:.*]] : !fir.ref<i32>) {
 ! CHECK-NEXT:          omp.loop_nest (%[[VAL_14:.*]]) : i32 = (%[[VAL_11]]) to (%[[VAL_12]]) inclusive step (%[[VAL_13]]) {
 ! CHECK:                 %[[VAL_7:.*]]:2 = hlfir.declare %[[VAL_6]] {uniq_name = "_QFcombined_parallel_worksharing_loopEi"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
-! CHECK:                 fir.store %[[VAL_14]] to %[[VAL_7]]#1 : !fir.ref<i32>
+! CHECK:                 hlfir.assign %[[VAL_14]] to %[[VAL_7]]#1 : i32, !fir.ref<i32>
 ! CHECK:                 fir.call @_QPsub4(%[[VAL_9]]#1) fastmath<contract> : (!fir.ref<i32>) -> ()
 ! CHECK:                 omp.yield
 ! CHECK:               }
@@ -326,7 +326,7 @@ subroutine common_1()
 ! CHECK:             omp.wsloop private(@{{.*}} %{{.*}} -> %[[VAL_19:.*]] : !fir.ref<i32>) {
 ! CHECK-NEXT:          omp.loop_nest (%[[VAL_37:.*]]) : i32 = (%[[VAL_34]]) to (%[[VAL_35]]) inclusive step (%[[VAL_36]]) {
 ! CHECK:             %[[VAL_20:.*]]:2 = hlfir.declare %[[VAL_19]] {uniq_name = "_QFcommon_2Ei"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
-! CHECK:                 fir.store %[[VAL_37]] to %[[VAL_20]]#1 : !fir.ref<i32>
+! CHECK:                 hlfir.assign %[[VAL_37]] to %[[VAL_20]]#1 : i32, !fir.ref<i32>
 ! CHECK:                 %[[VAL_38:.*]] = fir.load %[[VAL_31]]#0 : !fir.ref<i32>
 ! CHECK:                 %[[VAL_39:.*]] = fir.load %[[VAL_20]]#0 : !fir.ref<i32>
 ! CHECK:                 %[[VAL_40:.*]] = arith.addi %[[VAL_38]], %[[VAL_39]] : i32
diff --git a/flang/test/Lower/OpenMP/critical.f90 b/flang/test/Lower/OpenMP/critical.f90
index 99a4426ab0453..50844aa7d2f2d 100644
--- a/flang/test/Lower/OpenMP/critical.f90
+++ b/flang/test/Lower/OpenMP/critical.f90
@@ -42,7 +42,7 @@ subroutine predetermined_privatization()
     !CHECK: omp.wsloop private(@{{.*}} %{{.*}} -> %[[PRIV_I_ALLOC:.*]] : !fir.ref<i32>)
     !CHECK: omp.loop_nest (%[[IV:[^[:space:]]+]])
     !CHECK: %[[PRIV_I_DECL:.*]]:2 = hlfir.declare %[[PRIV_I_ALLOC]]
-    !CHECK: fir.store %[[IV]] to %[[PRIV_I_DECL]]#1
+    !CHECK: hlfir.assign %[[IV]] to %[[PRIV_I_DECL]]#1
     !CHECK: omp.critical
     !$omp critical
     a(i) = a(i-1) + 1
diff --git a/flang/test/Lower/OpenMP/default-clause.f90 b/flang/test/Lower/OpenMP/default-clause.f90
index fcc8d033eea0f..ef4f4a416286f 100644
--- a/flang/test/Lower/OpenMP/default-clause.f90
+++ b/flang/test/Lower/OpenMP/default-clause.f90
@@ -291,7 +291,7 @@ subroutine nested_default_clause_test4
 !CHECK: omp.loop_nest (%[[ARG:.*]]) : i32 = (%[[CONST_LB]]) to (%[[CONST_UB]]) inclusive step (%[[CONST_STEP]]) {
 !CHECK: %[[X_DECLARE:.*]]:2 = hlfir.declare %[[X_ALLOCA]] {{.*}}
 !CHECK: %[[LOOP_VAR_DECLARE:.*]]:2 = hlfir.declare %[[LOOP_VAR_ALLOCA]] {{.*}}
-!CHECK: fir.store %[[ARG]] to %[[LOOP_VAR_DECLARE]]#1 : !fir.ref<i32>
+!CHECK: hlfir.assign %[[ARG]] to %[[LOOP_VAR_DECLARE]]#1 : i32, !fir.ref<i32>
 !CHECK: %[[LOADED_X:.*]] = fir.load %[[X_DECLARE]]#0 : !fir.ref<i32>
 !CHECK: %[[CONST:.*]] = arith.constant 1 : i32
 !CHECK: %[[RESULT:.*]] = arith.addi %[[LOADED_X]], %[[CONST]] : i32
@@ -324,7 +324,7 @@ subroutine nested_default_clause_test5
 ! CHECK: omp.wsloop private(@{{.*}} %{{.*}} -> %[[LOOP_VAR:.*]] : !fir.ref<i32>) {
 !CHECK: omp.loop_nest (%[[ARG:.*]]) : i32 = (%[[CONST_LB]]) to (%[[CONST_UB]]) inclusive step (%[[CONST_STEP]]) {
 !CHECK: %[[LOOP_VAR_DECLARE:.*]]:2 = hlfir.declare %[[LOOP_VAR]] {{.*}}
-!CHECK: fir.store %[[ARG]] to %[[LOOP_VAR_DECLARE]]#1 : !fir.ref<i32>
+!CHECK: hlfir.assign %[[ARG]] to %[[LOOP_VAR_DECLARE]]#1 : i32, !fir.ref<i32>
 !CHECK: %[[LOADED_X:.*]] = fir.load %[[X_VAR_DECLARE]]#0 : !fir.ref<i32>
 !CHECK: %[[CONST:.*]] = arith.constant 1 : i32
 !CHECK: %[[ADD:.*]] = arith.addi %[[LOADED_X]], %[[CONST]] : i32
diff --git a/flang/test/Lower/OpenMP/generic-loop-rewriting.f90 b/flang/test/Lower/OpenMP/generic-loop-rewriting.f90
index 0699c36c69519..fcecc6e808d73 100644
--- a/flang/test/Lower/OpenMP/generic-loop-rewriting.f90
+++ b/flang/test/Lower/OpenMP/generic-loop-rewriting.f90
@@ -50,7 +50,7 @@ end subroutine target_teams_loop
 !CHECK:                   omp.loop_nest (%{{.*}}) : i32 = 
 !CHECK-SAME:                (%[[LB]]) to (%[[UB]]) inclusive step (%[[STEP]]) {
 !CHECK:                     %[[I_PRIV_DECL:.*]]:2 = hlfir.declare %[[I_PRIV_ARG]]
-!CHECK:                     fir.store %{{.*}} to %[[I_PRIV_DECL]]#1 : !fir.ref<i32>
+!CHECK:                     hlfir.assign %{{.*}} to %[[I_PRIV_DECL]]#1 : i32, !fir.ref<i32>
 !CHECK:                   }
 !CHECK:                 }
 !CHECK:               }
diff --git a/flang/test/Lower/OpenMP/lastprivate-iv.f90 b/flang/test/Lower/OpenMP/lastprivate-iv.f90
index aacefd8b59c0f..1b352634c3cea 100644
--- a/flang/test/Lower/OpenMP/lastprivate-iv.f90
+++ b/flang/test/Lower/OpenMP/lastprivate-iv.f90
@@ -12,7 +12,7 @@
 !CHECK:      omp.wsloop private(@{{.*}} %{{.*}} -> %[[I_MEM:.*]] : !fir.ref<i32>) {
 !CHECK-NEXT:   omp.loop_nest (%[[IV:.*]]) : i32 = (%[[LB]]) to (%[[UB]]) inclusive step (%[[STEP]]) {
 !CHECK:          %[[I:.*]]:2 = hlfir.declare %[[I_MEM]] {uniq_name = "_QFlastprivate_iv_incEi"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
-!CHECK:          fir.store %[[IV]] to %[[I]]#1 : !fir.ref<i32>
+!CHECK:          hlfir.assign %[[IV]] to %[[I]]#1 : i32, !fir.ref<i32>
 !CHECK:          %[[V:.*]] = arith.addi %[[IV]], %[[STEP]] : i32
 !CHECK:          %[[C0:.*]] = arith.constant 0 : i32
 !CHECK:          %[[STEP_NEG:.*]] = arith.cmpi slt, %[[STEP]], %[[C0]] : i32
@@ -46,7 +46,7 @@ subroutine lastprivate_iv_inc()
 !CHECK:      omp.wsloop private(@{{.*}} %{{.*}} -> %[[I_MEM:.*]] : !fir.ref<i32>) {
 !CHECK-NEXT:   omp.loop_nest (%[[IV:.*]]) : i32 = (%[[LB]]) to (%[[UB]]) inclusive step (%[[STEP]]) {
 !CHECK:          %[[I:.*]]:2 = hlfir.declare %[[I_MEM]] {uniq_name = "_QFlastprivate_iv_decEi"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
-!CHECK:          fir.store %[[IV]] to %[[I]]#1 : !fir.ref<i32>
+!CHECK:          hlfir.assign %[[IV]] to %[[I]]#1 : i32, !fir.ref<i32>
 !CHECK:          %[[V:.*]] = arith.addi %[[IV]], %[[STEP]] : i32
 !CHECK:          %[[C0:.*]] = arith.constant 0 : i32
 !CHECK:          %[[STEP_NEG:.*]] = arith.cmpi slt, %[[STEP]], %[[C0]] : i32
diff --git a/flang/test/Lower/OpenMP/loop-directive.f90 b/flang/test/Lower/OpenMP/loop-directive.f90
index 795f2a440fd0d..123dfffa350d7 100644
--- a/flang/test/Lower/OpenMP/loop-directive.f90
+++ b/flang/test/Lower/OpenMP/loop-directive.f90
@@ -15,7 +15,7 @@ subroutine test_no_clauses()
   ! CHECK: omp.simd private(@[[I_PRIV]] %{{.*}}#0 -> %[[ARG:.*]] : !fir.ref<i32>) {
   ! CHECK-NEXT:   omp.loop_nest (%[[IV:.*]]) : i32 = (%{{.*}}) to (%{{.*}}) {{.*}} {
   ! CHECK:          %[[ARG_DECL:.*]]:2 = hlfir.declare %[[ARG]]
-  ! CHECK:          fir.store %[[IV]] to %[[ARG_DECL]]#1 : !fir.ref<i32>
+  ! CHECK:          hlfir.assign %[[IV]] to %[[ARG_DECL]]#1 : i32, !fir.ref<i32>
   ! CHECK:        }
   ! CHECK: }
   !$omp loop
diff --git a/flang/test/Lower/OpenMP/loop-pointer-variable.f90 b/flang/test/Lower/OpenMP/loop-pointer-variable.f90
new file mode 100644
index 0000000000000..67a179a3b79ee
--- /dev/null
+++ b/flang/test/Lower/OpenMP/loop-pointer-variable.f90
@@ -0,0 +1,43 @@
+! This test checks lowering of OpenMP loops with pointer or allocatable loop index variable
+
+!RUN: bbc -emit-hlfir -fopenmp %s -o - 2>&1 | FileCheck %s
+!RUN: %flang_fc1 -emit-hlfir -fopenmp %s -o - 2>&1 | FileCheck %s
+
+!CHECK-LABEL: QQmain
+program loop_var
+  integer, pointer :: ip1, ip2
+  integer, allocatable :: ia1
+
+!CHECK:  omp.wsloop private(@_QFEip1_private_box_ptr_i32 %{{.*}}#0 -> %[[IP1_PVT:.*]], @_QFEip2_private_box_ptr_i32 %{{.*}}#0 -> %[[IP2_PVT:.*]] : !fir.ref<!fir.box<!fir.ptr<i32>>>, !fir.ref<!fir.box<!fir.ptr<i32>>>)
+!CHECK:  omp.loop_nest (%[[IP1_INDX:.*]], %[[IP2_INDX:.*]]) : i64 = ({{.*}}) to ({{.*}}) inclusive step ({{.*}})
+!CHECK:    %[[IP1_PVT_DECL:.*]]:2 = hlfir.declare %[[IP1_PVT]] {fortran_attrs = #fir.var_attrs<pointer>, uniq_name = "_QFEip1"} : (!fir.ref<!fir.box<!fir.ptr<i32>>>) -> (!fir.ref<!fir.box<!fir.ptr<i32>>>, !fir.ref<!fir.box<!fir.ptr<i32>>>)
+!CHECK:    %[[IP2_PVT_DECL:.*]]:2 = hlfir.declare %[[IP2_PVT]] {fortran_attrs = #fir.var_attrs<pointer>, uniq_name = "_QFEip2"} : (!fir.ref<!fir.box<!fir.ptr<i32>>>) -> (!fir.ref<!fir.box<!fir.ptr<i32>>>, !fir.ref<!fir.box<!fir.ptr<i32>>>)
+!CHECK:    %[[IP1:.*]] = fir.convert %[[IP1_INDX]] : (i64) -> i32
+!CHECK:    %[[IP1_BOX:.*]] = fir.load %[[IP1_PVT_DECL]]#1 : !fir.ref<!fir.box<!fir.ptr<i32>>>
+!CHECK:    %[[IP1_ADDR:.*]] = fir.box_addr %[[IP1_BOX]] : (!fir.box<!fir.ptr<i32>>) -> !fir.ptr<i32>
+!CHECK:    hlfir.assign %[[IP1]] to %[[IP1_ADDR]] : i32, !fir.ptr<i32>
+!CHECK:    %[[IP2:.*]] = fir.convert %[[IP2_INDX]] : (i64) -> i32
+!CHECK:    %[[IP2_BOX:.*]] = fir.load %[[IP2_PVT_DECL]]#1 : !fir.ref<!fir.box<!fir.ptr<i32>>>
+!CHECK:    %[[IP2_ADDR:.*]] = fir.box_addr %[[IP2_BOX]] : (!fir.box<!fir.ptr<i32>>) -> !fir.ptr<i32>
+!CHECK:    hlfir.assign %[[IP2]] to %[[IP2_ADDR]] : i32, !fir.ptr<i32>
+!CHECK:    omp.yield
+  !$omp do collapse(2)
+  do ip1 = 1, 10
+    do ip2 = 1, 20
+    end do
+  end do
+  !$omp end do
+
+!CHECK:    omp.simd private(@_QFEia1_private_box_heap_i32 %{{.*}}#0 -> %[[IA1_PVT:.*]] : !fir.ref<!fir.box<!fir.heap<i32>>>)
+!CHECK:      omp.loop_nest (%[[IA1_INDX:.*]]) : i64 = ({{.*}}) to ({{.*}}) inclusive step ({{.*}})
+!CHECK:        %[[IA1_PVT_DECL:.*]]:2 = hlfir.declare %[[IA1_PVT]] {fortran_attrs = #fir.var_attrs<allocatable>, uniq_name = "_QFEia1"} : (!fir.ref<!fir.box<!fir.heap<i32>>>) -> (!fir.ref<!fir.box<!fir.heap<i32>>>, !fir.ref<!fir.box<!fir.heap<i32>>>)
+!CHECK:        %[[IA1:.*]] = fir.convert %[[IA1_INDX]] : (i64) -> i32
+!CHECK:        %[[IA1_BOX:.*]] = fir.load %[[IA1_PVT_DECL]]#1 : !fir.ref<!fir.box<!fir.heap<i32>>>
+!CHECK:        %[[IA1_ADDR:.*]] = fir.box_addr %[[IA1_BOX]] : (!fir.box<!fir.heap<i32>>) -> !fir.heap<i32>
+!CHECK:        hlfir.assign %[[IA1]] to %[[IA1_ADDR]] : i32, !fir.heap<i32>
+!CHECK:        omp.yield
+  !$omp simd
+  do ia1 = 1, 10
+  end do
+  !$omp end simd
+end program
diff --git a/flang/test/Lower/OpenMP/parallel-private-clause-fixes.f90 b/flang/test/Lower/OpenMP/parallel-private-clause-fixes.f90
index 2c34e33a0f4c1..2bd394eb265c2 100644
--- a/flang/test/Lower/OpenMP/parallel-private-clause-fixes.f90
+++ b/flang/test/Lower/OpenMP/parallel-private-clause-fixes.f90
@@ -52,7 +52,7 @@
 ! CHECK-DAG:           %[[PRIV_I_DECL:.*]]:2 = hlfir.declare %[[PRIV_I]] {uniq_name = "_QFmultiple_private_fixEi"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
 ! CHECK-DAG:           %[[PRIV_J_DECL:.*]]:2 = hlfir.declare %[[PRIV_J]] {uniq_name = "_QFmultiple_private_fixEj"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
 ! CHECK-DAG:           %[[PRIV_X_DECL:.*]]:2 = hlfir.declare %[[PRIV_X]] {uniq_name = "_QFmultiple_private_fixEx"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
-! CHECK:               fir.store %[[VAL_6]] to %[[PRIV_I_DECL]]#1 : !fir.ref<i32>
+! CHECK:               hlfir.assign %[[VAL_6]] to %[[PRIV_I_DECL]]#1 : i32, !fir.ref<i32>
 ! CHECK:               %[[VAL_7:.*]] = arith.constant 1 : i32
 ! CHECK:               %[[VAL_8:.*]] = fir.convert %[[VAL_7]] : (i32) -> index
 ! CHECK:               %[[VAL_9:.*]] = fir.load %[[GAMA_DECL]]#0 : !fir.ref<i32>
diff --git a/flang/test/Lower/OpenMP/parallel-private-clause.f90 b/flang/test/Lower/OpenMP/parallel-private-clause.f90
index e4dd4189c5e87..f05443372c48d 100644
--- a/flang/test/Lower/OpenMP/parallel-private-clause.f90
+++ b/flang/test/Lower/OpenMP/parallel-private-clause.f90
@@ -274,7 +274,7 @@ subroutine simple_loop_1
   !$OMP DO
   do i=1, 9
   ! FIRDialect:      %[[ALLOCA_IV_DECL:.*]]:2 = hlfir.declare %[[ALLOCA_IV]] {uniq_name = "_QFsimple_loop_1Ei"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
-  ! FIRDialect:      fir.store %[[I]] to %[[ALLOCA_IV_DECL]]#1 : !fir.ref<i32>
+  ! FIRDialect:      hlfir.assign %[[I]] to %[[ALLOCA_IV_DECL]]#1 : i32, !fir.ref<i32>
   ! FIRDialect:      %[[LOAD_IV:.*]] = fir.load %[[ALLOCA_IV_DECL]]#0 : !fir.ref<i32>
   ! FIRDialect:      fir.call @_FortranAioOutputInteger32({{.*}}, %[[LOAD_IV]]) {{.*}} : (!fir.ref<i8>, i32) -> i1
     print*, i
@@ -302,7 +302,7 @@ subroutine simple_loop_2
   do i=1, 9
   ! FIRDialect:     %[[R_DECL:.*]]:2 = hlfir.declare %[[R]] {fortran_attrs = #fir.var_attrs<allocatable>, uniq_name = "{{.*}}Er"} : (!fir.ref<!fir.box<!fir.heap<f32>>>) -> (!fir.ref<!fir.box<!fir.heap<f32>>>, !fir.ref<!fir.box<!fir.heap<f32>>>)
   ! FIRDialect:     %[[ALLOCA_IV_DECL:.*]]:2 = hlfir.declare %[[ALLOCA_IV]] {uniq_name = "{{.*}}Ei"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
-  ! FIRDialect:     fir.store %[[I]] to %[[ALLOCA_IV_DECL]]#1 : !fir.ref<i32>
+  ! FIRDialect:     hlfir.assign %[[I]] to %[[ALLOCA_IV_DECL]]#1 : i32, !fir.ref<i32>
   ! FIRDialect:     %[[LOAD_IV:.*]] = fir.load %[[ALLOCA_IV_DECL]]#0 : !fir.ref<i32>
   ! FIRDialect:     fir.call @_FortranAioOutputInteger32({{.*}}, %[[LOAD_IV]]) {{.*}}: (!fir.ref<i8>, i32) -> i1
     print*, i
@@ -330,7 +330,7 @@ subroutine simple_loop_3
 
   ! FIRDialect:      %[[ALLOCA_IV_DECL:.*]]:2 = hlfir.declare %[[ALLOCA_IV]] {uniq_name = "{{.*}}Ei"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
 
-  ! FIRDialect:      fir.store %[[I]] to %[[ALLOCA_IV_DECL:.*]]#1 : !fir.ref<i32>
+  ! FIRDialect:      hlfir.assign %[[I]] to %[[ALLOCA_IV_DECL:.*]]#1 : i32, !fir.ref<i32>
   ! FIRDialect:      %[[LOAD_IV:.*]] = fir.load %[[ALLOCA_IV_DECL]]#0 : !fir.ref<i32>
   ! FIRDialect:      fir.call @_FortranAioOutputInteger32({{.*}}, %[[LOAD_IV]]) {{.*}}: (!fir.ref<i8>, i32) -> i1
     print*, i
@@ -354,7 +354,7 @@ subroutine simd_loop_1
   ! FIRDialect-NEXT: omp.loop_nest (%[[I:.*]]) : i32 = (%[[LB]]) to (%[[UB]]) inclusive step (%[[STEP]]) {
   !$OMP SIMD PRIVATE(r)
   do i=1, 9
-  ! FIRDialect:     fir.store %[[I]] to %[[LOCAL:.*]]#1 : !fir.ref<i32>
+  ! FIRDialect:     hlfir.assign %[[I]] to %[[LOCAL:.*]]#1 : i32, !fir.ref<i32>
   ! FIRDialect:     %[[LOAD_IV:.*]] = fir.load %[[LOCAL]]#0 : !fir.ref<i32>
   ! FIRDialect:     fir.call @_FortranAioOutputInteger32({{.*}}, %[[LOAD_IV]]) {{.*}}: (!fir.ref<i8>, i32) -> i1
     print*, i
diff --git a/flang/test/Lower/OpenMP/parallel-reduction-allocatable-array.f90 b/flang/test/Lower/OpenMP/parallel-reduction-allocatable-array.f90
index 021f5d1ba2ef7..47fcd3d8941b1 100644
--- a/flang/test/Lower/OpenMP/parallel-reduction-allocatable-array.f90
+++ b/flang/test/Lower/OpenMP/parallel-reduction-allocatable-array.f90
@@ -101,7 +101,7 @@ program reduce
 ! CHECK-NEXT:          omp.loop_nest (%[[VAL_18:.*]]) : i32 = (%[[VAL_14]]) to (%[[VAL_15]]) inclusive step (%[[VAL_16]]) {
 ! CHECK:                 %[[VAL_13:.*]]:2 = hlfir.declare %[[VAL_12]] {uniq_name = "_QFEi"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
 ! CHECK:                 %[[VAL_19:.*]]:2 = hlfir.declare %[[VAL_17]] {fortran_attrs = {{.*}}<allocatable>, uniq_name = "_QFEr"} : (!fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>) -> (!fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>, !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>)
-! CHECK:                 fir.store %[[VAL_18]] to %[[VAL_13]]#1 : !fir.ref<i32>
+! CHECK:                 hlfir.assign %[[VAL_18]] to %[[VAL_13]]#1 : i32, !fir.ref<i32>
 ! CHECK:                 %[[VAL_20:.*]] = fir.load %[[VAL_13]]#0 : !fir.ref<i32>
 ! CHECK:                 %[[VAL_21:.*]] = fir.load %[[VAL_19]]#0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>
 ! CHECK:                 %[[VAL_22:.*]] = arith.constant 1 : index
diff --git a/flang/test/Lower/OpenMP/parallel-reduction-pointer-array.f90 b/flang/test/Lower/OpenMP/parallel-reduction-pointer-array.f90
index fd620c4080dbc..a3ad7c67971f2 100644
--- a/flang/test/Lower/OpenMP/parallel-reduction-pointer-array.f90
+++ b/flang/test/Lower/OpenMP/parallel-reduction-pointer-array.f90
@@ -113,7 +113,7 @@ program reduce
 ! CHECK:               omp.loop_nest (%[[VAL_28:.*]]) : i32 = (%[[VAL_24]]) to (%[[VAL_25]]) inclusive step (%[[VAL_26]]) {
 ! CHECK:                 %[[VAL_23:.*]]:2 = hlfir.declare %[[VAL_22]] {uniq_name = "_QFEi"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
 ! CHECK:                 %[[VAL_29:.*]]:2 = hlfir.declare %[[VAL_27]] {fortran_attrs = {{.*}}<pointer>, uniq_name = "_QFEr"} : (!fir.ref<!fir.box<!fir.ptr<!fir.array<?xi32>>>>) -> (!fir.ref<!fir.box<!fir.ptr<!fir.array<?xi32>>>>, !fir.ref<!fir.box<!fir.ptr<!fir.array<?xi32>>>>)
-! CHECK:                 fir.store %[[VAL_28]] to %[[VAL_23]]#1 : !fir.ref<i32>
+! CHECK:                 hlfir.assign %[[VAL_28]] to %[[VAL_23]]#1 : i32, !fir.ref<i32>
 ! CHECK:                 %[[V...
[truncated]

@kiranchandramohan kiranchandramohan changed the title [Flang][OpenMP] Fix crash when loop index var is private or allocatable [Flang][OpenMP] Fix crash when loop index var is pointer or allocatable Mar 4, 2025
@kiranchandramohan kiranchandramohan marked this pull request as draft March 4, 2025 15:03
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.

The approach looks sensible to me. What is needed to bring this out of draft?

@kiranchandramohan
Copy link
Contributor Author

The approach looks sensible to me. What is needed to bring this out of draft?

There is one fix required for the lastprivate case. I will add that and remove it out of draft.

@kiranchandramohan kiranchandramohan requested a review from luporl March 5, 2025 14:02
@kiranchandramohan kiranchandramohan marked this pull request as ready for review March 5, 2025 14:03
Copy link
Contributor

@luporl luporl left a comment

Choose a reason for hiding this comment

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

LGTM

@kiranchandramohan kiranchandramohan merged commit e2911aa into llvm:main Mar 6, 2025
11 checks passed
jph-13 pushed a commit to jph-13/llvm-project that referenced this pull request Mar 21, 2025
…le (llvm#129717)

Use hlfir dereferencing for pointers and allocatables and use hlfir
assign. Also, change the code updating IV in lastprivate.

Note: This is a small change. Modifications in existing tests are
changes from fir.store to hlfir.assign.

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

Successfully merging this pull request may close these issues.

[Flang][OpenMP] verification of lowering to FIR failed with "'fir.store' op store value type must match memory reference type"
4 participants