Get the BoxType from the RHS instead of LHS for polymorphic pointer assignment inside FORALL. #164279

DanielCChen · 2025-10-20T16:30:20Z

Fixes #153220

llvmbot · 2025-10-20T16:30:54Z

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

Author: Daniel Chen (DanielCChen)

Changes

Fixes #153220

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

2 Files Affected:

(modified) flang/lib/Lower/Bridge.cpp (+4-3)
(added) flang/test/Lower/forall-polymorphic.f90 (+45)

diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index 0595ca063f407..316a1830a5d5d 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -4730,13 +4730,14 @@ class FirConverter : public Fortran::lower::AbstractConverter {
     if (Fortran::evaluate::UnwrapExpr<Fortran::evaluate::NullPointer>(
             assign.rhs))
       return fir::factory::createUnallocatedBox(*builder, loc, lhsBoxType, {});
-    hlfir::Entity rhs = Fortran::lower::convertExprToHLFIR(
-        loc, *this, assign.rhs, localSymbols, rhsContext);
+    hlfir::Entity rhs{fir::getBase(genExprBox(loc, assign.rhs, rhsContext))};
+    auto rhsBoxType =
+        llvm::cast<fir::BaseBoxType>(fir::unwrapRefType(rhs.getType()));
     // Create pointer descriptor value from the RHS.
     if (rhs.isMutableBox())
       rhs = hlfir::Entity{fir::LoadOp::create(*builder, loc, rhs)};
     mlir::Value rhsBox = hlfir::genVariableBox(
-        loc, *builder, rhs, lhsBoxType.getBoxTypeWithNewShape(rhs.getRank()));
+        loc, *builder, rhs, rhsBoxType.getBoxTypeWithNewShape(rhs.getRank()));
     // Apply lower bounds or reshaping if any.
     if (const auto *lbExprs =
             std::get_if<Fortran::evaluate::Assignment::BoundsSpec>(&assign.u);
diff --git a/flang/test/Lower/forall-polymorphic.f90 b/flang/test/Lower/forall-polymorphic.f90
new file mode 100644
index 0000000000000..33bf4e5dea1a5
--- /dev/null
+++ b/flang/test/Lower/forall-polymorphic.f90
@@ -0,0 +1,45 @@
+! Test lower of elemental user defined assignments
+! RUN: bbc -emit-fir %s -o - | FileCheck %s
+
+! CHECK-LABEL: c.func @_QPforallpolymorphic
+  subroutine forallPolymorphic()
+  TYPE :: DT
+    CLASS(DT), POINTER    :: Ptr(:) => NULL()
+  END TYPE
+
+  TYPE, EXTENDS(DT) :: DT1
+  END TYPE
+
+  TYPE(DT1), TARGET  :: Tar1(10)
+  CLASS(DT), POINTER :: T(:)
+  integer :: I
+
+  FORALL (I=1:10)
+    T(I)%Ptr => Tar1
+  END FORALL
+
+! CHECK: %[[V_11:[0-9]+]] = fir.alloca !fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt{ptr:!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt>>>>}>>>> {bindc_name = "t", uniq_name = "_QFforallpolymorphicEt"}
+! CHECK: %[[V_15:[0-9]+]] = fir.declare %[[V_11]] {fortran_attrs = #fir.var_attrs<pointer>, uniq_name = "_QFforallpolymorphicEt"} : (!fir.ref<!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt{ptr:!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt>>>>}>>>>>) -> !fir.ref<!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt{ptr:!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt>>>>}>>>>>
+! CHECK: %[[V_16:[0-9]+]] = fir.alloca !fir.array<10x!fir.type<_QFforallpolymorphicTdt1{dt:!fir.type<_QFforallpolymorphicTdt{ptr:!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt>>>>}>}>> {bindc_name = "tar1", fir.target, uniq_name = "_QFforallpolymorphicEtar1"}
+! CHECK: %[[V_17:[0-9]+]] = fir.shape %c10 : (index) -> !fir.shape<1>
+! CHECK: %[[V_18:[0-9]+]] = fir.declare %[[V_16]](%[[V_17]]) {fortran_attrs = #fir.var_attrs<target>, uniq_name = "_QFforallpolymorphicEtar1"} : (!fir.ref<!fir.array<10x!fir.type<_QFforallpolymorphicTdt1{dt:!fir.type<_QFforallpolymorphicTdt{ptr:!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt>>>>}>}>>>, !fir.shape<1>) -> !fir.ref<!fir.array<10x!fir.type<_QFforallpolymorphicTdt1{dt:!fir.type<_QFforallpolymorphicTdt{ptr:!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt>>>>}>}>>>
+! CHECK: %[[V_19:[0-9]+]] = fir.embox %[[V_18]](%[[V_17]]) : (!fir.ref<!fir.array<10x!fir.type<_QFforallpolymorphicTdt1{dt:!fir.type<_QFforallpolymorphicTdt{ptr:!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt>>>>}>}>>>, !fir.shape<1>) -> !fir.box<!fir.array<10x!fir.type<_QFforallpolymorphicTdt1{dt:!fir.type<_QFforallpolymorphicTdt{ptr:!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt>>>>}>}>>>
+! CHECK: %[[V_34:[0-9]+]] = fir.convert %c1_i32 : (i32) -> index
+! CHECK: %[[V_35:[0-9]+]] = fir.convert %c10_i32 : (i32) -> index
+! CHECK: fir.do_loop %arg0 = %[[V_34]] to %[[V_35]] step %c1
+! CHECK: {
+! CHECK: %[[V_36:[0-9]+]] = fir.convert %arg0 : (index) -> i32
+! CHECK: %[[V_37:[0-9]+]] = fir.load %[[V_15]] : !fir.ref<!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt{ptr:!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt>>>>}>>>>>
+! CHECK: %[[V_38:[0-9]+]] = fir.convert %[[V_36]] : (i32) -> i64
+! CHECK: %[[C0:.*]] = arith.constant 0 : index
+! CHECK: %[[V_39:[0-9]+]]:3 = fir.box_dims %37, %[[C0]] : (!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt{ptr:!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt>>>>}>>>>, index) -> (index, index, index)
+! CHECK: %[[V_40:[0-9]+]] = fir.shift %[[V_39]]#0 : (index) -> !fir.shift<1>
+! CHECK: %[[V_41:[0-9]+]] = fir.array_coor %[[V_37]](%[[V_40]]) %[[V_38]] : (!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt{ptr:!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt>>>>}>>>>, !fir.shift<1>, i64) -> !fir.ref<!fir.type<_QFforallpolymorphicTdt{ptr:!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt>>>>}>>
+! CHECK: %[[V_42:[0-9]+]] = fir.embox %[[V_41]] source_box %[[V_37]] : (!fir.ref<!fir.type<_QFforallpolymorphicTdt{ptr:!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt>>>>}>>, !fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt{ptr:!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt>>>>}>>>>) -> !fir.class<!fir.type<_QFforallpolymorphicTdt{ptr:!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt>>>>}>>
+! CHECK: %[[V_43:[0-9]+]] = fir.field_index ptr, !fir.type<_QFforallpolymorphicTdt{ptr:!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt>>>>}>
+! CHECK: %[[V_44:[0-9]+]] = fir.coordinate_of %[[V_42]], ptr : (!fir.class<!fir.type<_QFforallpolymorphicTdt{ptr:!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt>>>>}>>) -> !fir.ref<!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt{ptr:!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt>>>>}>>>>>
+! CHECK: %[[V_45:[0-9]+]] = fir.rebox %[[V_19]] : (!fir.box<!fir.array<10x!fir.type<_QFforallpolymorphicTdt1{dt:!fir.type<_QFforallpolymorphicTdt{ptr:!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt>>>>}>}>>>) -> !fir.box<!fir.array<?x!fir.type<_QFforallpolymorphicTdt1{dt:!fir.type<_QFforallpolymorphicTdt{ptr:!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt>>>>}>}>>>
+! CHECK: %[[V_46:[0-9]+]] = fir.convert %[[V_45]] : (!fir.box<!fir.array<?x!fir.type<_QFforallpolymorphicTdt1{dt:!fir.type<_QFforallpolymorphicTdt{ptr:!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt>>>>}>}>>>) -> !fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt{ptr:!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt>>>>}>>>>
+! CHECK: fir.store %[[V_46]] to %[[V_44]] : !fir.ref<!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt{ptr:!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt>>>>}>>>>>
+! CHECK: }
+  end subroutine forallPolymorphic

…ssignment for FORALL.

jeanPerier

Thanks for working on this fix!

jeanPerier · 2025-10-21T07:42:08Z

flang/test/Lower/forall-polymorphic.f90

+! CHECK: %[[V_46:[0-9]+]] = fir.convert %[[V_45]] : (!fir.box<!fir.array<?x!fir.type<_QFforallpolymorphicTdt1{dt:!fir.type<_QFforallpolymorphicTdt{ptr:!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt>>>>}>}>>>) -> !fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt{ptr:!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt>>>>}>>>>
+! CHECK: fir.store %[[V_46]] to %[[V_44]] : !fir.ref<!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt{ptr:!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt>>>>}>>>>>


This will end-up erasing the pointer attribute in the runtime descriptor for the pointers because it was not set in the descriptor value that V_45, and convert is a reinterpret_cast for descriptor (it does not change/ensure that the runtime attributes match the type attributes).
Suggestion fix in code.

jeanPerier · 2025-10-21T07:57:22Z

flang/lib/Lower/Bridge.cpp

-        loc, *this, assign.rhs, localSymbols, rhsContext);
+    hlfir::Entity rhs{fir::getBase(genExprBox(loc, assign.rhs, rhsContext))};
+    auto rhsBoxType =
+        llvm::cast<fir::BaseBoxType>(fir::unwrapRefType(rhs.getType()));


I see why you are doing this, HLFIR is lacking a helper here.

Could you add a new method to the Entity class definition in HLFIRTools.h after getElementOrSequenceType def:

/// Return the fir.class or fir.box type needed to describe this entity. fir::BaseBoxType getBoxType() const { if (isBoxAddressOrValue()) return llvm::cast<fir::BaseBoxType>(fir::unwrapRefType(getType())); const bool isVolatile = fir::isa_volatile_type(getType()); mlir::Type boxType = return fir::BoxType::get(getElementOrSequenceType(), isVolatile); }

That way, you can keep convertExprToHLFIR and use it to get the rhsBoxType.
Then, you should replace rhsBoxType.getBoxTypeWithNewShape(rhs.getRank()) with rhsBoxType.getBoxTypeWithNewAttr(fir::BaseBoxType::Attribute::Pointer) in the hlfir::genVariableBox to get the right attribute set via embox/rebox in the runtime descriptor that will be stored.

@jeanPerier
Thanks for the quick review and the suggesion!
I tried your suggestion, but got core dump as

flang: /scratch/cdchen/FLANG/llvm-project/llvm/include/llvm/Support/Casting.h:560: decltype(auto) llvm::cast(const From &) [To = fir::BaseBoxType, From = mlir::Type]: Assertion `isa<To>(Val) && "cast<Ty>() argument of incompatible type!"' failed.

The expected rhsBoxType should be !fir.box<!fir.char<1>>, which is what the the current patch returns.
However, with convertExprToHLFIR, I got !fir.ref<!fir.char<1>>.

The code fir::getBase(genExprBox(loc, assign.rhs, rhsContext)) is also used by the exact same pointer assignment inside a DO loop (in comparison to FORALL).

The expected rhsBoxType should be !fir.box<!fir.char<1>>, which is what the the current patch returns.
However, with convertExprToHLFIR, I got !fir.ref<!fir.char<1>>.

Yes, that is why I suggested adding the getBoxType() helper. Did you get the rhsBoxType via this new rhs.getBoxType() ?

Sorry that I mis-spoke the error.

I indeed have the correct BoxType for rhsBoxType as !fir.box<!fir.char<1>>. I also got rhsBox as

%189 = "fir.embox"(%188) <{operandSegmentSizes = array<i32: 1, 0, 0, 0, 0>}> : (!fir.ref<!fir.char<1>>) -> !fir.box<!fir.ptr<!fir.char<1>>>

However, fir.convert now complains

error: 'fir.convert' op invalid type conversion'!fir.box<!fir.ptr<!fir.char<1>>>' / '!fir.class<!fir.ptr<none>>' error: Lowering to LLVM IR failed

Update: The error is actually from a modified code. While it is a bug, but it is not in the scope of this PR.
Sorry about the confusion. I will update the patch.

jeanPerier

Actually, I now remember why this code was using the LHS type, and this is to cover the case where the RHS is polymorphic and the LHS is not... (the opposite use case).

Here is a modified test from the issue that passes with flang-trunk but would fail after this patch:

module m
  TYPE :: DT
    TYPE(DT), POINTER    :: Ptr(:) => NULL()
  END TYPE

  TYPE, EXTENDS(DT) :: DT1
  END TYPE
contains

subroutine test(Tar1)
  CLASS(DT), TARGET  :: Tar1(:)
  TYPE(DT) :: T(10)
  integer :: I


  !DO I = 1, 10
  FORALL (I=1:10)            
    T(I)%Ptr => Tar1
  END FORALL
  !end do

  call test_type(T(1)%Ptr)
  DO I = 1, 10

  END DO
end subroutine

subroutine test_type(Tptr)
  class(DT) :: Tptr(:)
    SELECT TYPE (aa => Tptr)
    TYPE IS (DT1)
      print*, "in type is"
    CLASS DEFAULT
      print*, "in class default"
    END SELECT
end subroutine
end module

  use m
  TYPE(DT1), TARGET :: Tar1(10)
  call test(Tar1)

  END

So I think you need to BaseBoxType change the type passed to genVariableBox only when the LHS is polymorphic.

…cide which type to use.

… LHS is not polymorphic.

jeanPerier

LGTM, thank you.

…ssignment inside FORALL. (llvm#164279) Fixes llvm#153220

…res to rebox the RHS to the LHS type iif LHS is polymorphic.

…ssignment inside FORALL. (llvm#164279) Fixes llvm#153220

…so requires to rebox the RHS to the LHS type iif LHS is polymorphic." This reverts commit 1582702.

…ssignment inside FORALL. (llvm#164279) Fixes llvm#153220

DanielCChen requested review from jeanPerier and kkwli October 20, 2025 16:30

DanielCChen self-assigned this Oct 20, 2025

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

Get the BoxType from the RHS instead of LHS for polymorphic pointer a…

0807368

…ssignment for FORALL.

DanielCChen force-pushed the daniel_forall branch from abc93c7 to 0807368 Compare October 20, 2025 18:55

jeanPerier reviewed Oct 21, 2025

View reviewed changes

jeanPerier requested changes Oct 21, 2025

View reviewed changes

DanielCChen added 2 commits October 21, 2025 17:32

To address review comments and also check if LHS is polymorphic to de…

8949f51

…cide which type to use.

To update the LIT test and also add a new test to cover the case that…

d34dd0b

… LHS is not polymorphic.

jeanPerier approved these changes Oct 22, 2025

View reviewed changes

[NFC] Correct the description of the LIT test.

04a9886

DanielCChen merged commit 2dbe959 into llvm:main Oct 22, 2025
10 checks passed

DanielCChen deleted the daniel_forall branch October 22, 2025 14:24

dvbuka pushed a commit to dvbuka/llvm-project that referenced this pull request Oct 27, 2025

Get the BoxType from the RHS instead of LHS for polymorphic pointer a…

1a994aa

…ssignment inside FORALL. (llvm#164279) Fixes llvm#153220

DanielCChen added a commit to DanielCChen/llvm-project that referenced this pull request Oct 28, 2025

Fix a regression that was exposed by PR llvm#164279, which also requi…

1582702

…res to rebox the RHS to the LHS type iif LHS is polymorphic.

Lukacma pushed a commit to Lukacma/llvm-project that referenced this pull request Oct 29, 2025

Get the BoxType from the RHS instead of LHS for polymorphic pointer a…

836fe07

…ssignment inside FORALL. (llvm#164279) Fixes llvm#153220

DanielCChen added a commit to DanielCChen/llvm-project that referenced this pull request Oct 29, 2025

Revert "Fix a regression that was exposed by PR llvm#164279, which al…

3d99863

…so requires to rebox the RHS to the LHS type iif LHS is polymorphic." This reverts commit 1582702.

aokblast pushed a commit to aokblast/llvm-project that referenced this pull request Oct 30, 2025

Get the BoxType from the RHS instead of LHS for polymorphic pointer a…

2d0cdb6

…ssignment inside FORALL. (llvm#164279) Fixes llvm#153220

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Get the BoxType from the RHS instead of LHS for polymorphic pointer assignment inside FORALL. #164279

Get the BoxType from the RHS instead of LHS for polymorphic pointer assignment inside FORALL. #164279

Uh oh!

DanielCChen commented Oct 20, 2025

Uh oh!

llvmbot commented Oct 20, 2025

Uh oh!

jeanPerier left a comment

Uh oh!

jeanPerier Oct 21, 2025

Uh oh!

jeanPerier Oct 21, 2025

Uh oh!

DanielCChen Oct 21, 2025

Uh oh!

jeanPerier Oct 21, 2025

Uh oh!

DanielCChen Oct 21, 2025 •

edited

Loading

Uh oh!

jeanPerier left a comment

Uh oh!

jeanPerier left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		! CHECK: %[[V_46:[0-9]+]] = fir.convert %[[V_45]] : (!fir.box<!fir.array<?x!fir.type<_QFforallpolymorphicTdt1{dt:!fir.type<_QFforallpolymorphicTdt{ptr:!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt>>>>}>}>>>) -> !fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt{ptr:!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt>>>>}>>>>
		! CHECK: fir.store %[[V_46]] to %[[V_44]] : !fir.ref<!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt{ptr:!fir.class<!fir.ptr<!fir.array<?x!fir.type<_QFforallpolymorphicTdt>>>>}>>>>>

Get the BoxType from the RHS instead of LHS for polymorphic pointer assignment inside FORALL. #164279

Get the BoxType from the RHS instead of LHS for polymorphic pointer assignment inside FORALL. #164279

Uh oh!

Conversation

DanielCChen commented Oct 20, 2025

Uh oh!

llvmbot commented Oct 20, 2025

Uh oh!

jeanPerier left a comment

Choose a reason for hiding this comment

Uh oh!

jeanPerier Oct 21, 2025

Choose a reason for hiding this comment

Uh oh!

jeanPerier Oct 21, 2025

Choose a reason for hiding this comment

Uh oh!

DanielCChen Oct 21, 2025

Choose a reason for hiding this comment

Uh oh!

jeanPerier Oct 21, 2025

Choose a reason for hiding this comment

Uh oh!

DanielCChen Oct 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeanPerier left a comment

Choose a reason for hiding this comment

Uh oh!

jeanPerier left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

DanielCChen Oct 21, 2025 •

edited

Loading