Skip to content

Commit

Permalink
[not-for-merge] Experiment with using alloc's first component
Browse files Browse the repository at this point in the history 
An experimental diff to discuss using only the first component of
`hlfir.declare`'s result for allocatables. This is just to have a more
informed discussion on #84033.

This is not meant to be merged, at least for now.
  • Loading branch information
@ergawy
ergawy committed Mar 6, 2024
1 parent 7bad74e commit 65091f5
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 35 deletions.
5 changes: 3 additions & 2 deletions flang/include/flang/Optimizer/Builder/HLFIRTools.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ class Entity : public mlir::Value {
// information, or has access to it in other ways. Its advantage is that
// it will never be a fir.box for explicit shape arrays, leading to simpler
// FIR code generation.
mlir::Value getFirBase() const;
mlir::Value getFirBase(bool forceDeclBaseHACK = false) const;
};

/// Wrapper over an mlir::Value that can be viewed as a Fortran entity.
Expand Down Expand Up @@ -230,7 +230,8 @@ translateToExtendedValue(mlir::Location loc, fir::FirOpBuilder &builder,
/// on the IR.
fir::ExtendedValue
translateToExtendedValue(mlir::Location loc, fir::FirOpBuilder &builder,
fir::FortranVariableOpInterface fortranVariable);
fir::FortranVariableOpInterface fortranVariable,
bool forceDeclBaseHACK = false);

/// Generate declaration for a fir::ExtendedValue in memory.
fir::FortranVariableOpInterface
Expand Down
11 changes: 6 additions & 5 deletions flang/lib/Lower/Bridge.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -618,7 +618,7 @@ class FirConverter : public Fortran::lower::AbstractConverter {
assert(details && "No host-association found");
const Fortran::semantics::Symbol &hsym = details->symbol();
mlir::Type hSymType = genType(hsym);
Fortran::lower::SymbolBox hsb = lookupSymbol(hsym);
Fortran::lower::SymbolBox hsb = lookupSymbol(hsym, nullptr, true);

auto allocate = [&](llvm::ArrayRef<mlir::Value> shape,
llvm::ArrayRef<mlir::Value> typeParams) -> mlir::Value {
Expand Down Expand Up @@ -727,7 +727,7 @@ class FirConverter : public Fortran::lower::AbstractConverter {
void createHostAssociateVarCloneDealloc(
const Fortran::semantics::Symbol &sym) override final {
mlir::Location loc = genLocation(sym.name());
Fortran::lower::SymbolBox hsb = lookupSymbol(sym);
Fortran::lower::SymbolBox hsb = lookupSymbol(sym, nullptr, true);

fir::ExtendedValue hexv = symBoxToExtendedValue(hsb);
hexv.match(
Expand Down Expand Up @@ -960,13 +960,14 @@ class FirConverter : public Fortran::lower::AbstractConverter {
/// Find the symbol in the local map or return null.
Fortran::lower::SymbolBox
lookupSymbol(const Fortran::semantics::Symbol &sym,
Fortran::lower::SymMap *symMap = nullptr) {
Fortran::lower::SymMap *symMap = nullptr,
bool forceDeclBaseHACK = false) {
symMap = symMap ? symMap : &localSymbols;
if (lowerToHighLevelFIR()) {
if (std::optional<fir::FortranVariableOpInterface> var =
symMap->lookupVariableDefinition(sym)) {
auto exv =
hlfir::translateToExtendedValue(toLocation(), *builder, *var);
auto exv = hlfir::translateToExtendedValue(toLocation(), *builder, *var,
forceDeclBaseHACK);
return exv.match(
[](mlir::Value x) -> Fortran::lower::SymbolBox {
return Fortran::lower::SymbolBox::Intrinsic{x};
Expand Down
15 changes: 9 additions & 6 deletions flang/lib/Optimizer/Builder/HLFIRTools.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,11 +183,12 @@ static fir::CharBoxValue genUnboxChar(mlir::Location loc,
return {addr, len};
}

mlir::Value hlfir::Entity::getFirBase() const {
mlir::Value hlfir::Entity::getFirBase(bool forceDeclBaseHACK) const {
if (fir::FortranVariableOpInterface variable = getIfVariableInterface()) {
if (auto declareOp =
mlir::dyn_cast<hlfir::DeclareOp>(variable.getOperation()))
return declareOp.getOriginalBase();
return forceDeclBaseHACK ? declareOp.getBase()
: declareOp.getOriginalBase();
if (auto associateOp =
mlir::dyn_cast<hlfir::AssociateOp>(variable.getOperation()))
return associateOp.getFirBase();
Expand Down Expand Up @@ -848,11 +849,12 @@ hlfir::LoopNest hlfir::genLoopNest(mlir::Location loc,

static fir::ExtendedValue
translateVariableToExtendedValue(mlir::Location loc, fir::FirOpBuilder &builder,
hlfir::Entity variable) {
hlfir::Entity variable,
bool forceDeclBaseHACK = false) {
assert(variable.isVariable() && "must be a variable");
/// When going towards FIR, use the original base value to avoid
/// introducing descriptors at runtime when they are not required.
mlir::Value firBase = variable.getFirBase();
mlir::Value firBase = variable.getFirBase(forceDeclBaseHACK);
if (variable.isMutableBox())
return fir::MutableBoxValue(firBase, getExplicitTypeParams(variable),
fir::MutableProperties{});
Expand Down Expand Up @@ -900,8 +902,9 @@ translateVariableToExtendedValue(mlir::Location loc, fir::FirOpBuilder &builder,

fir::ExtendedValue
hlfir::translateToExtendedValue(mlir::Location loc, fir::FirOpBuilder &builder,
fir::FortranVariableOpInterface var) {
return translateVariableToExtendedValue(loc, builder, var);
fir::FortranVariableOpInterface var,
bool forceDeclBaseHACK) {
return translateVariableToExtendedValue(loc, builder, var, forceDeclBaseHACK);
}

std::pair<fir::ExtendedValue, std::optional<hlfir::CleanupFunction>>
Expand Down
10 changes: 5 additions & 5 deletions flang/test/Lower/OpenMP/parallel-private-clause-str.f90
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@
!CHECK: %[[C_DECL:.*]]:2 = hlfir.declare %[[C_BOX_REF]] typeparams %{{.*}} {fortran_attrs = #fir.var_attrs<allocatable>, uniq_name = "_QFtest_allocatable_stringEc"} : (!fir.ref<!fir.box<!fir.heap<!fir.char<1,?>>>>, i32) -> (!fir.ref<!fir.box<!fir.heap<!fir.char<1,?>>>>, !fir.ref<!fir.box<!fir.heap<!fir.char<1,?>>>>)
!CHECK: omp.parallel {
!CHECK: %[[C_PVT_BOX_REF:.*]] = fir.alloca !fir.box<!fir.heap<!fir.char<1,?>>> {bindc_name = "c", pinned, uniq_name = "_QFtest_allocatable_stringEc"}
!CHECK: %[[C_BOX:.*]] = fir.load %[[C_DECL]]#1 : !fir.ref<!fir.box<!fir.heap<!fir.char<1,?>>>>
!CHECK: %[[C_BOX:.*]] = fir.load %[[C_DECL]]#0 : !fir.ref<!fir.box<!fir.heap<!fir.char<1,?>>>>
!CHECK: fir.if %{{.*}} {
!CHECK: %[[C_PVT_MEM:.*]] = fir.allocmem !fir.char<1,?>(%{{.*}} : index) {fir.must_be_heap = true, uniq_name = "_QFtest_allocatable_stringEc.alloc"}
!CHECK: %[[C_PVT_BOX:.*]] = fir.embox %[[C_PVT_MEM]] typeparams %{{.*}} : (!fir.heap<!fir.char<1,?>>, index) -> !fir.box<!fir.heap<!fir.char<1,?>>>
!CHECK: fir.store %[[C_PVT_BOX]] to %[[C_PVT_BOX_REF]] : !fir.ref<!fir.box<!fir.heap<!fir.char<1,?>>>>
!CHECK: }
!CHECK: %[[C_PVT_DECL:.*]]:2 = hlfir.declare %[[C_PVT_BOX_REF]] {fortran_attrs = #fir.var_attrs<allocatable>, uniq_name = "_QFtest_allocatable_stringEc"} : (!fir.ref<!fir.box<!fir.heap<!fir.char<1,?>>>>) -> (!fir.ref<!fir.box<!fir.heap<!fir.char<1,?>>>>, !fir.ref<!fir.box<!fir.heap<!fir.char<1,?>>>>)
!CHECK: fir.if %{{.*}} {
!CHECK: %[[C_PVT_BOX:.*]] = fir.load %[[C_PVT_DECL]]#1 : !fir.ref<!fir.box<!fir.heap<!fir.char<1,?>>>>
!CHECK: %[[C_PVT_BOX:.*]] = fir.load %[[C_PVT_DECL]]#0 : !fir.ref<!fir.box<!fir.heap<!fir.char<1,?>>>>
!CHECK: %[[C_PVT_BOX_ADDR:.*]] = fir.box_addr %[[C_PVT_BOX]] : (!fir.box<!fir.heap<!fir.char<1,?>>>) -> !fir.heap<!fir.char<1,?>>
!CHECK: fir.freemem %[[C_PVT_BOX_ADDR]] : !fir.heap<!fir.char<1,?>>
!CHECK: }
Expand All @@ -38,16 +38,16 @@ subroutine test_allocatable_string(n)
!CHECK: %[[C_DECL:.*]]:2 = hlfir.declare %[[C_BOX_REF]] typeparams %{{.*}} {fortran_attrs = #fir.var_attrs<allocatable>, uniq_name = "_QFtest_allocatable_string_arrayEc"} : (!fir.ref<!fir.box<!fir.heap<!fir.array<?x!fir.char<1,?>>>>>, i32) -> (!fir.ref<!fir.box<!fir.heap<!fir.array<?x!fir.char<1,?>>>>>, !fir.ref<!fir.box<!fir.heap<!fir.array<?x!fir.char<1,?>>>>>)
!CHECK: omp.parallel {
!CHECK: %[[C_PVT_BOX_REF:.*]] = fir.alloca !fir.box<!fir.heap<!fir.array<?x!fir.char<1,?>>>> {bindc_name = "c", pinned, uniq_name = "_QFtest_allocatable_string_arrayEc"}
!CHECK: %{{.*}} = fir.load %[[C_DECL]]#1 : !fir.ref<!fir.box<!fir.heap<!fir.array<?x!fir.char<1,?>>>>>
!CHECK: %{{.*}} = fir.load %[[C_DECL]]#0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?x!fir.char<1,?>>>>>
!CHECK: fir.if %{{.*}} {
!CHECK: %[[C_PVT_ALLOC:.*]] = fir.allocmem !fir.array<?x!fir.char<1,?>>(%{{.*}} : index), %{{.*}} {fir.must_be_heap = true, uniq_name = "_QFtest_allocatable_string_arrayEc.alloc"}
!CHECK: %[[C_PVT_BOX:.*]] = fir.embox %[[C_PVT_ALLOC]](%{{.*}}) typeparams %{{.*}} : (!fir.heap<!fir.array<?x!fir.char<1,?>>>, !fir.shapeshift<1>, index) -> !fir.box<!fir.heap<!fir.array<?x!fir.char<1,?>>>>
!CHECK: fir.store %[[C_PVT_BOX]] to %[[C_PVT_BOX_REF]] : !fir.ref<!fir.box<!fir.heap<!fir.array<?x!fir.char<1,?>>>>>
!CHECK: }
!CHECK: %[[C_PVT_DECL:.*]]:2 = hlfir.declare %[[C_PVT_BOX_REF]] {fortran_attrs = #fir.var_attrs<allocatable>, uniq_name = "_QFtest_allocatable_string_arrayEc"} : (!fir.ref<!fir.box<!fir.heap<!fir.array<?x!fir.char<1,?>>>>>) -> (!fir.ref<!fir.box<!fir.heap<!fir.array<?x!fir.char<1,?>>>>>, !fir.ref<!fir.box<!fir.heap<!fir.array<?x!fir.char<1,?>>>>>)
!CHECK: %{{.*}} = fir.load %[[C_PVT_DECL]]#1 : !fir.ref<!fir.box<!fir.heap<!fir.array<?x!fir.char<1,?>>>>>
!CHECK: %{{.*}} = fir.load %[[C_PVT_DECL]]#0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?x!fir.char<1,?>>>>>
!CHECK: fir.if %{{.*}} {
!CHECK: %[[C_PVT_BOX:.*]] = fir.load %[[C_PVT_DECL]]#1 : !fir.ref<!fir.box<!fir.heap<!fir.array<?x!fir.char<1,?>>>>>
!CHECK: %[[C_PVT_BOX:.*]] = fir.load %[[C_PVT_DECL]]#0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?x!fir.char<1,?>>>>>
!CHECK: %[[C_PVT_ADDR:.*]] = fir.box_addr %[[C_PVT_BOX]] : (!fir.box<!fir.heap<!fir.array<?x!fir.char<1,?>>>>) -> !fir.heap<!fir.array<?x!fir.char<1,?>>>
!CHECK: fir.freemem %[[C_PVT_ADDR]] : !fir.heap<!fir.array<?x!fir.char<1,?>>>
!CHECK: }
Expand Down
34 changes: 17 additions & 17 deletions flang/test/Lower/OpenMP/parallel-private-clause.f90
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,8 @@ subroutine private_clause_derived_type()
!FIRDialect-DAG: %[[X4_PVT:.*]] = fir.alloca !fir.box<!fir.heap<!fir.array<?xi32>>> {bindc_name = "x4", pinned, uniq_name = "{{.*}}Ex4"}
!FIRDialect-DAG: %[[X4_PVT_DECL:.*]]:2 = hlfir.declare %[[X4_PVT]] {fortran_attrs = #fir.var_attrs<allocatable>, uniq_name = "{{.*}}Ex4"} : (!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>>>>)

!FIRDialect-DAG: %[[TMP58:.*]] = fir.load %[[X4_DECL]]#1 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>
!FIRDialect-DAG: %[[TMP97:.*]] = fir.load %[[X4_DECL]]#1 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>
!FIRDialect-DAG: %[[TMP58:.*]] = fir.load %[[X4_DECL]]#0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>
!FIRDialect-DAG: %[[TMP97:.*]] = fir.load %[[X4_DECL]]#0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>
!FIRDialect-DAG: %[[TMP98:.*]]:3 = fir.box_dims %[[TMP97]], {{.*}} : (!fir.box<!fir.heap<!fir.array<?xi32>>>, index) -> (index, index, index)

!FIRDialect-DAG: %[[TMP101:.*]] = fir.allocmem !fir.array<?xi32>, {{.*}} {fir.must_be_heap = true, uniq_name = "{{.*}}Ex4.alloc"}
Expand Down Expand Up @@ -192,12 +192,12 @@ subroutine private_clause_allocatable()
!FIRDialect-DAG: }
!FIRDialect-DAG: %[[X5_PVT_DECL:.*]]:2 = hlfir.declare %[[X5_PVT]] {fortran_attrs = #fir.var_attrs<allocatable>, uniq_name = "_QFprivate_clause_real_call_allocatableEx5"} : (!fir.ref<!fir.box<!fir.heap<f32>>>) -> (!fir.ref<!fir.box<!fir.heap<f32>>>, !fir.ref<!fir.box<!fir.heap<f32>>>)
!FIRDialect-DAG: fir.call @_QFprivate_clause_real_call_allocatablePhelper_private_clause_real_call_allocatable(%[[X5_PVT_DECL]]#0) fastmath<contract> : (!fir.ref<!fir.box<!fir.heap<f32>>>) -> ()
!FIRDialect-DAG: %{{.*}} = fir.load %[[X5_PVT_DECL]]#1 : !fir.ref<!fir.box<!fir.heap<f32>>>
!FIRDialect-DAG: %{{.*}} = fir.load %[[X5_PVT_DECL]]#0 : !fir.ref<!fir.box<!fir.heap<f32>>>

!FIRDialect-DAG: fir.if %{{.*}} {
!FIRDialect-DAG: %{{.*}} = fir.load %[[X5_PVT_DECL]]#1 : !fir.ref<!fir.box<!fir.heap<f32>>>
!FIRDialect-DAG: %{{.*}} = fir.load %[[X5_PVT_DECL]]#0 : !fir.ref<!fir.box<!fir.heap<f32>>>

!FIRDialect-DAG: fir.store %{{.*}} to %[[X5_PVT_DECL]]#1 : !fir.ref<!fir.box<!fir.heap<f32>>>
!FIRDialect-DAG: fir.store %{{.*}} to %[[X5_PVT_DECL]]#0 : !fir.ref<!fir.box<!fir.heap<f32>>>
!FIRDialect-DAG: }
!FIRDialect-DAG: omp.terminator
!FIRDialect-DAG: }
Expand Down Expand Up @@ -313,12 +313,12 @@ subroutine simple_loop_1
print*, i
end do
! FIRDialect: omp.yield
! FIRDialect: {{%.*}} = fir.load %[[R_DECL]]#1 : !fir.ref<!fir.box<!fir.heap<f32>>>
! FIRDialect: {{%.*}} = fir.load %[[R_DECL]]#0 : !fir.ref<!fir.box<!fir.heap<f32>>>
! FIRDialect: fir.if {{%.*}} {
! FIRDialect: [[LD:%.*]] = fir.load %[[R_DECL]]#1 : !fir.ref<!fir.box<!fir.heap<f32>>>
! FIRDialect: [[LD:%.*]] = fir.load %[[R_DECL]]#0 : !fir.ref<!fir.box<!fir.heap<f32>>>
! FIRDialect: [[AD:%.*]] = fir.box_addr [[LD]] : (!fir.box<!fir.heap<f32>>) -> !fir.heap<f32>
! FIRDialect: fir.freemem [[AD]] : !fir.heap<f32>
! FIRDialect: fir.store {{%.*}} to %[[R_DECL]]#1 : !fir.ref<!fir.box<!fir.heap<f32>>>
! FIRDialect: fir.store {{%.*}} to %[[R_DECL]]#0 : !fir.ref<!fir.box<!fir.heap<f32>>>
!$OMP END DO
! FIRDialect: omp.terminator
!$OMP END PARALLEL
Expand Down Expand Up @@ -351,12 +351,12 @@ subroutine simple_loop_2
print*, i
end do
! FIRDialect: omp.yield
! FIRDialect: {{%.*}} = fir.load %[[R_DECL]]#1 : !fir.ref<!fir.box<!fir.heap<f32>>>
! FIRDialect: {{%.*}} = fir.load %[[R_DECL]]#0 : !fir.ref<!fir.box<!fir.heap<f32>>>
! FIRDialect: fir.if {{%.*}} {
! FIRDialect: [[LD:%.*]] = fir.load %[[R_DECL]]#1 : !fir.ref<!fir.box<!fir.heap<f32>>>
! FIRDialect: [[LD:%.*]] = fir.load %[[R_DECL]]#0 : !fir.ref<!fir.box<!fir.heap<f32>>>
! FIRDialect: [[AD:%.*]] = fir.box_addr [[LD]] : (!fir.box<!fir.heap<f32>>) -> !fir.heap<f32>
! FIRDialect: fir.freemem [[AD]] : !fir.heap<f32>
! FIRDialect: fir.store {{%.*}} to %[[R_DECL]]#1 : !fir.ref<!fir.box<!fir.heap<f32>>>
! FIRDialect: fir.store {{%.*}} to %[[R_DECL]]#0 : !fir.ref<!fir.box<!fir.heap<f32>>>
!$OMP END DO
! FIRDialect: omp.terminator
!$OMP END PARALLEL
Expand Down Expand Up @@ -388,12 +388,12 @@ subroutine simple_loop_3
print*, i
end do
! FIRDialect: omp.yield
! FIRDialect: {{%.*}} = fir.load [[R_DECL]]#1 : !fir.ref<!fir.box<!fir.heap<f32>>>
! FIRDialect: {{%.*}} = fir.load [[R_DECL]]#0 : !fir.ref<!fir.box<!fir.heap<f32>>>
! FIRDialect: fir.if {{%.*}} {
! FIRDialect: [[LD:%.*]] = fir.load [[R_DECL]]#1 : !fir.ref<!fir.box<!fir.heap<f32>>>
! FIRDialect: [[LD:%.*]] = fir.load [[R_DECL]]#0 : !fir.ref<!fir.box<!fir.heap<f32>>>
! FIRDialect: [[AD:%.*]] = fir.box_addr [[LD]] : (!fir.box<!fir.heap<f32>>) -> !fir.heap<f32>
! FIRDialect: fir.freemem [[AD]] : !fir.heap<f32>
! FIRDialect: fir.store {{%.*}} to [[R_DECL]]#1 : !fir.ref<!fir.box<!fir.heap<f32>>>
! FIRDialect: fir.store {{%.*}} to [[R_DECL]]#0 : !fir.ref<!fir.box<!fir.heap<f32>>>
!$OMP END PARALLEL DO
! FIRDialect: omp.terminator
end subroutine
Expand Down Expand Up @@ -421,10 +421,10 @@ subroutine simd_loop_1
end do
!$OMP END SIMD
! FIRDialect: omp.yield
! FIRDialect: {{%.*}} = fir.load [[R_DECL]]#1 : !fir.ref<!fir.box<!fir.heap<f32>>>
! FIRDialect: {{%.*}} = fir.load [[R_DECL]]#0 : !fir.ref<!fir.box<!fir.heap<f32>>>
! FIRDialect: fir.if {{%.*}} {
! FIRDialect: [[LD:%.*]] = fir.load [[R_DECL]]#1 : !fir.ref<!fir.box<!fir.heap<f32>>>
! FIRDialect: [[LD:%.*]] = fir.load [[R_DECL]]#0 : !fir.ref<!fir.box<!fir.heap<f32>>>
! FIRDialect: [[AD:%.*]] = fir.box_addr [[LD]] : (!fir.box<!fir.heap<f32>>) -> !fir.heap<f32>
! FIRDialect: fir.freemem [[AD]] : !fir.heap<f32>
! FIRDialect: fir.store {{%.*}} to [[R_DECL]]#1 : !fir.ref<!fir.box<!fir.heap<f32>>>
! FIRDialect: fir.store {{%.*}} to [[R_DECL]]#0 : !fir.ref<!fir.box<!fir.heap<f32>>>
end subroutine

0 comments on commit 65091f5

Please sign in to comment.