Skip to content

Conversation

@ashermancinelli
Copy link
Contributor

Component references inherit volatility from their base derived types. Moved the base type volatility check before the box type is built, and merge it (instead of overwrite it) with the volatility of the base type.

Component references inherit volatility from their base derived types.
Moved the base type volatility check before the box type is built, and
merge it (instead of overwrite it) with the volatility of the base type.
@ashermancinelli ashermancinelli self-assigned this May 2, 2025
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir labels May 2, 2025
@llvmbot
Copy link
Member

llvmbot commented May 2, 2025

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

Author: Asher Mancinelli (ashermancinelli)

Changes

Component references inherit volatility from their base derived types. Moved the base type volatility check before the box type is built, and merge it (instead of overwrite it) with the volatility of the base type.


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

2 Files Affected:

  • (modified) flang/lib/Lower/ConvertExprToHLFIR.cpp (+6-6)
  • (added) flang/test/Lower/volatile-derived-type.f90 (+48)
diff --git a/flang/lib/Lower/ConvertExprToHLFIR.cpp b/flang/lib/Lower/ConvertExprToHLFIR.cpp
index a3be50ac072d4..5981116a6d3f7 100644
--- a/flang/lib/Lower/ConvertExprToHLFIR.cpp
+++ b/flang/lib/Lower/ConvertExprToHLFIR.cpp
@@ -236,6 +236,12 @@ class HlfirDesignatorBuilder {
         isVolatile = true;
     }
 
+    // Check if the base type is volatile
+    if (partInfo.base.has_value()) {
+      mlir::Type baseType = partInfo.base.value().getType();
+      isVolatile = isVolatile || fir::isa_volatile_type(baseType);
+    }
+
     // Arrays with non default lower bounds or dynamic length or dynamic extent
     // need a fir.box to hold the dynamic or lower bound information.
     if (fir::hasDynamicSize(resultValueType) ||
@@ -249,12 +255,6 @@ class HlfirDesignatorBuilder {
             /*namedConstantSectionsAreAlwaysContiguous=*/false))
       return fir::BoxType::get(resultValueType, isVolatile);
 
-    // Check if the base type is volatile
-    if (partInfo.base.has_value()) {
-      mlir::Type baseType = partInfo.base.value().getType();
-      isVolatile = fir::isa_volatile_type(baseType);
-    }
-
     // Other designators can be handled as raw addresses.
     return fir::ReferenceType::get(resultValueType, isVolatile);
   }
diff --git a/flang/test/Lower/volatile-derived-type.f90 b/flang/test/Lower/volatile-derived-type.f90
new file mode 100644
index 0000000000000..edd77a9265530
--- /dev/null
+++ b/flang/test/Lower/volatile-derived-type.f90
@@ -0,0 +1,48 @@
+! RUN: bbc --strict-fir-volatile-verifier %s -o - | FileCheck %s
+! Ensure member access of a volatile derived type is volatile.
+  type t
+    integer :: e(4)=2
+  end type t
+  type(t), volatile :: f
+  call test (f%e(::2))
+contains
+  subroutine test(v)
+    integer, asynchronous :: v(:)
+  end subroutine
+end
+! CHECK-LABEL:   func.func @_QQmain() {
+! CHECK:           %[[VAL_0:.*]] = arith.constant 4 : index
+! CHECK:           %[[VAL_1:.*]] = arith.constant 1 : index
+! CHECK:           %[[VAL_2:.*]] = arith.constant 2 : index
+! CHECK:           %[[VAL_3:.*]] = arith.constant 0 : index
+! CHECK:           %[[VAL_4:.*]] = fir.dummy_scope : !fir.dscope
+! CHECK:           %[[VAL_5:.*]] = fir.address_of(@_QFE.b.t.e) : !fir.ref<!fir.array<2x1x!fir.type<_QM__fortran_type_infoTvalue{genre:i8,__padding0:!fir.array<7xi8>,value:i64}>>>
+! CHECK:           %[[VAL_6:.*]] = fir.shape_shift %[[VAL_3]], %[[VAL_2]], %[[VAL_3]], %[[VAL_1]] : (index, index, index, index) -> !fir.shapeshift<2>
+! CHECK:           %[[VAL_7:.*]]:2 = hlfir.declare %[[VAL_5]](%[[VAL_6]]) {fortran_attrs = #fir.var_attrs<target>, uniq_name = "_QFE.b.t.e"} :
+! CHECK:           %[[VAL_8:.*]] = fir.address_of(@_QFE.n.e) : !fir.ref<!fir.char<1>>
+! CHECK:           %[[VAL_9:.*]]:2 = hlfir.declare %[[VAL_8]] typeparams %[[VAL_1]] {fortran_attrs = #fir.var_attrs<target>, uniq_name = "_QFE.n.e"} : (!fir.ref<!fir.char<1>>, index) -> (!fir.ref<!fir.char<1>>, !fir.ref<!fir.char<1>>)
+! CHECK:           %[[VAL_10:.*]] = fir.address_of(@_QFE.di.t.e) : !fir.ref<!fir.array<4xi32>>
+! CHECK:           %[[VAL_11:.*]] = fir.shape %[[VAL_0]] : (index) -> !fir.shape<1>
+! CHECK:           %[[VAL_12:.*]]:2 = hlfir.declare %[[VAL_10]](%[[VAL_11]]) {fortran_attrs = #fir.var_attrs<target>, uniq_name = "_QFE.di.t.e"} : (!fir.ref<!fir.array<4xi32>>, !fir.shape<1>) -> (!fir.ref<!fir.array<4xi32>>, !fir.ref<!fir.array<4xi32>>)
+! CHECK:           %[[VAL_13:.*]] = fir.address_of(@_QFE.n.t) : !fir.ref<!fir.char<1>>
+! CHECK:           %[[VAL_14:.*]]:2 = hlfir.declare %[[VAL_13]] typeparams %[[VAL_1]] {fortran_attrs = #fir.var_attrs<target>, uniq_name = "_QFE.n.t"} : (!fir.ref<!fir.char<1>>, index) -> (!fir.ref<!fir.char<1>>, !fir.ref<!fir.char<1>>)
+! CHECK:           %[[VAL_15:.*]] = fir.alloca !fir.type<_QFTt{e:!fir.array<4xi32>}> {bindc_name = "f", uniq_name = "_QFEf"}
+! CHECK:           %[[VAL_16:.*]] = fir.volatile_cast %[[VAL_15]] : (!fir.ref<!fir.type<_QFTt{e:!fir.array<4xi32>}>>) -> !fir.ref<!fir.type<_QFTt{e:!fir.array<4xi32>}>, volatile>
+! CHECK:           %[[VAL_17:.*]]:2 = hlfir.declare %[[VAL_16]] {fortran_attrs = #fir.var_attrs<volatile>, uniq_name = "_QFEf"} : (!fir.ref<!fir.type<_QFTt{e:!fir.array<4xi32>}>, volatile>) -> (!fir.ref<!fir.type<_QFTt{e:!fir.array<4xi32>}>, volatile>, !fir.ref<!fir.type<_QFTt{e:!fir.array<4xi32>}>, volatile>)
+! CHECK:           %[[VAL_18:.*]] = fir.address_of(@_QQ_QFTt.DerivedInit) : !fir.ref<!fir.type<_QFTt{e:!fir.array<4xi32>}>>
+! CHECK:           fir.copy %[[VAL_18]] to %[[VAL_17]]#0 no_overlap : !fir.ref<!fir.type<_QFTt{e:!fir.array<4xi32>}>>, !fir.ref<!fir.type<_QFTt{e:!fir.array<4xi32>}>, volatile>
+! CHECK:           %[[VAL_20:.*]] = fir.shape_shift %[[VAL_3]], %[[VAL_1]] : (index, index) -> !fir.shapeshift<1>
+! CHECK:           %[[VAL_21:.*]]:2 = hlfir.declare %{{.+}}(%[[VAL_20]]) {fortran_attrs = #fir.var_attrs<target>, uniq_name = "_QFE.c.t"} :
+! CHECK:           %[[VAL_24:.*]] = fir.shape %[[VAL_2]] : (index) -> !fir.shape<1>
+! CHECK:           %[[VAL_25:.*]] = hlfir.designate %[[VAL_17]]#0{"e"} <%[[VAL_11]]> (%[[VAL_1]]:%[[VAL_0]]:%[[VAL_2]])  shape %[[VAL_24]] : (!fir.ref<!fir.type<_QFTt{e:!fir.array<4xi32>}>, volatile>, !fir.shape<1>, index, index, index, !fir.shape<1>) -> !fir.box<!fir.array<2xi32>, volatile>
+! CHECK:           %[[VAL_26:.*]] = fir.volatile_cast %[[VAL_25]] : (!fir.box<!fir.array<2xi32>, volatile>) -> !fir.box<!fir.array<2xi32>>
+! CHECK:           %[[VAL_27:.*]] = fir.convert %[[VAL_26]] : (!fir.box<!fir.array<2xi32>>) -> !fir.box<!fir.array<?xi32>>
+! CHECK:           fir.call @_QFPtest(%[[VAL_27]]) fastmath<contract> : (!fir.box<!fir.array<?xi32>>) -> ()
+! CHECK:           return
+! CHECK:         }
+! CHECK-LABEL:   func.func private @_QFPtest(
+! CHECK-SAME:                                %[[VAL_0:[0-9]+|[a-zA-Z$._-][a-zA-Z0-9$._-]*]]: !fir.box<!fir.array<?xi32>> {fir.asynchronous, fir.bindc_name = "v"}) attributes {fir.host_symbol = @_QQmain, llvm.linkage = #llvm.linkage<internal>} {
+! CHECK:           %[[VAL_1:.*]] = fir.dummy_scope : !fir.dscope
+! CHECK:           %[[VAL_2:.*]]:2 = hlfir.declare %[[VAL_0]] dummy_scope %[[VAL_1]] {fortran_attrs = #fir.var_attrs<asynchronous>, uniq_name = "_QFFtestEv"} : (!fir.box<!fir.array<?xi32>>, !fir.dscope) -> (!fir.box<!fir.array<?xi32>>, !fir.box<!fir.array<?xi32>>)
+! CHECK:           return
+! CHECK:         }

Copy link
Contributor

@jeanPerier jeanPerier left a comment

Choose a reason for hiding this comment

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

LGTM

@ashermancinelli ashermancinelli merged commit c0e52f3 into llvm:main May 5, 2025
14 checks passed
ashermancinelli added a commit to ashermancinelli/llvm-project that referenced this pull request May 5, 2025
Test added in llvm#138339 unneccesarily had CHECK lines with the
type layout, which fails on aix.
ashermancinelli added a commit that referenced this pull request May 5, 2025
Test added in #138339 unneccesarily had CHECK lines with the type
layout, which fails on aix.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…8339)

Component references inherit volatility from their base derived types.
Moved the base type volatility check before the box type is built, and
merge it (instead of overwrite it) with the volatility of the base type.
GeorgeARM pushed a commit to GeorgeARM/llvm-project that referenced this pull request May 7, 2025
…8339)

Component references inherit volatility from their base derived types.
Moved the base type volatility check before the box type is built, and
merge it (instead of overwrite it) with the volatility of the base type.
GeorgeARM pushed a commit to GeorgeARM/llvm-project that referenced this pull request May 7, 2025
Test added in llvm#138339 unneccesarily had CHECK lines with the type
layout, which fails on aix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

flang:fir-hlfir flang Flang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants