commit aeaf319b8c2207eb747079529f28d44d54bf1fd5
author jeanPerier <jperier@nvidia.com> Wed Jun 25 09:41:33 2025 +0200
committer GitHub <noreply@github.com> Wed Jun 25 09:41:33 2025 +0200
tree 8bbc9630b562aa63f4a6edb211081845476b8ba4
parent 31545ca5f467fb0860ed3d09313451d547e41f5d

[flang] avoid useless rebox of polymorphic scalars (#145507)

Do not create new descriptor for polymorphic scalars when lowering
hlfir.declare.

hlfir.declare of box/class is lowered to a fir.rebox to ensure that
local lower bounds and descriptor attributes (Pointer/Allocatable/None)
are properly set-up in the descriptor associated to the symbol.

For polymorphic scalar, this created a useless temporary descriptor.
This was breaking invalid code #145256 that violates OPTIONAL usage
rules. I am not fixing it primarily to support this invalid code, but
rather because it is dumb to create a useless fir.rebox.

flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp

diff --git a/flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp b/flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp
index f7efaa7..33f687d 100644
--- a/flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp
+++ b/flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp
@@ -326,11 +326,14 @@
       auto genHlfirBox = [&]() -> mlir::Value {
         if (auto baseBoxType =
                 mlir::dyn_cast<fir::BaseBoxType>(firBase.getType())) {
-          // Rebox so that lower bounds are correct.
+          // Rebox so that lower bounds and attributes are correct.
           if (baseBoxType.isAssumedRank())
             return builder.create<fir::ReboxAssumedRankOp>(
                 loc, hlfirBaseType, firBase,
                 fir::LowerBoundModifierAttribute::SetToOnes);
+          if (!fir::extractSequenceType(baseBoxType.getEleTy()) &&
+              baseBoxType == hlfirBaseType)
+            return firBase;
           return builder.create<fir::ReboxOp>(loc, hlfirBaseType, firBase,
                                               declareOp.getShape(),
                                               /*slice=*/mlir::Value{});

flang/test/HLFIR/declare-codegen.fir

diff --git a/flang/test/HLFIR/declare-codegen.fir b/flang/test/HLFIR/declare-codegen.fir
index bd0d61a..a4edb63 100644
--- a/flang/test/HLFIR/declare-codegen.fir
+++ b/flang/test/HLFIR/declare-codegen.fir
@@ -219,3 +219,21 @@
 // CHECK-SAME: %[[VAL_0:.*]]: !fir.box<!fir.array<*:f32>>) {
 // CHECK:    %[[VAL_1:.*]] = fir.declare %[[VAL_0]] {uniq_name = "x"} : (!fir.box<!fir.array<*:f32>>) -> !fir.box<!fir.array<*:f32>>
 // CHECK:    %[[VAL_2:.*]] = fir.rebox_assumed_rank %[[VAL_1]] lbs ones : (!fir.box<!fir.array<*:f32>>) -> !fir.box<!fir.array<*:f32>>
+
+func.func @no_useless_rebox(%arg0: !fir.class<!fir.type<sometype{i:i32}>>) {
+  %0:2 = hlfir.declare %arg0 {uniq_name = "x"} : (!fir.class<!fir.type<sometype{i:i32}>>) -> (!fir.class<!fir.type<sometype{i:i32}>>, !fir.class<!fir.type<sometype{i:i32}>>)
+  fir.call @takes_class(%0#0) : (!fir.class<!fir.type<sometype{i:i32}>>) -> ()
+  return
+}
+// CHECK-LABEL: @no_useless_rebox
+// CHECK-NOT: fir.rebox
+// CHECK: return
+
+func.func @rebox_scalar_attrs(%arg0: !fir.class<!fir.ptr<!fir.type<sometype{i:i32}>>>) {
+  %0:2 = hlfir.declare %arg0 {uniq_name = "x"} : (!fir.class<!fir.ptr<!fir.type<sometype{i:i32}>>>) -> (!fir.class<!fir.type<sometype{i:i32}>>, !fir.class<!fir.type<sometype{i:i32}>>)
+  fir.call @takes_class(%0#0) : (!fir.class<!fir.type<sometype{i:i32}>>) -> ()
+  return
+}
+// CHECK-LABEL: @rebox_scalar_attrs
+// CHECK: fir.rebox %{{.*}} : (!fir.class<!fir.ptr<!fir.type<sometype{i:i32}>>>) -> !fir.class<!fir.type<sometype{i:i32}>>
+// CHECK: return