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

[via flang-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

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

Author: None (jeanPerier)

<details>
<summary>Changes</summary>

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. 

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


2 Files Affected:

- (modified) flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp (+3-1) 
- (modified) flang/test/HLFIR/declare-codegen.fir (+18) 


``````````diff
diff --git a/flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp b/flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp
index f7efaa736a279..fee46a772dcd4 100644
--- a/flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp
+++ b/flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp
@@ -326,11 +326,13 @@ class DeclareOpConversion : public mlir::OpRewritePattern<hlfir::DeclareOp> {
       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{});
diff --git a/flang/test/HLFIR/declare-codegen.fir b/flang/test/HLFIR/declare-codegen.fir
index bd0d61a2559db..a4edb630c4adb 100644
--- a/flang/test/HLFIR/declare-codegen.fir
+++ b/flang/test/HLFIR/declare-codegen.fir
@@ -219,3 +219,21 @@ func.func @assumed_rank_declare(%arg0: !fir.box<!fir.array<*:f32>>) {
 // 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

``````````

</details>


https://github.com/llvm/llvm-project/pull/145507