[flang-commits] [flang] [flang] Move `genCommonBlockMember` from OpenMP to ConvertVariable, NFC (PR #74488)
Krzysztof Parzyszek via flang-commits
flang-commits at lists.llvm.org
Tue Dec 5 09:54:05 PST 2023
https://github.com/kparzysz updated https://github.com/llvm/llvm-project/pull/74488
>From 769a37bdb7fdad3f7176715354068d97e912275f Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Tue, 5 Dec 2023 09:37:57 -0600
Subject: [PATCH 1/2] [flang] Move `genCommonBlockMember` from OpenMP to
ConvertVariable, NFC
The function `genCommonBlockMember` is not specific to OpenMP, and it
could very well be a common utility. Move it to ConvertVariable.cpp
where it logically belongs.
---
flang/include/flang/Lower/ConvertVariable.h | 18 +++++++++++--
flang/lib/Lower/ConvertVariable.cpp | 21 +++++++++++++++
flang/lib/Lower/OpenMP.cpp | 29 ++-------------------
3 files changed, 39 insertions(+), 29 deletions(-)
diff --git a/flang/include/flang/Lower/ConvertVariable.h b/flang/include/flang/Lower/ConvertVariable.h
index 7da04fea35167..970353b5fed93 100644
--- a/flang/include/flang/Lower/ConvertVariable.h
+++ b/flang/include/flang/Lower/ConvertVariable.h
@@ -19,6 +19,7 @@
#include "flang/Lower/Support/Utils.h"
#include "flang/Optimizer/Dialect/FIRAttr.h"
+#include "flang/Semantics/symbol.h"
#include "mlir/IR/Value.h"
#include "llvm/ADT/DenseMap.h"
@@ -29,7 +30,12 @@ class GlobalOp;
class FortranVariableFlagsAttr;
} // namespace fir
-namespace Fortran ::lower {
+namespace Fortran {
+namespace semantics {
+class Scope;
+} // namespace semantics
+
+namespace lower {
class AbstractConverter;
class CallerInterface;
class StatementContext;
@@ -66,6 +72,13 @@ void defineCommonBlocks(
const std::vector<std::pair<semantics::SymbolRef, std::size_t>>
&commonBlocks);
+/// The COMMON block is a global structure. \p commonValue is the base address
+/// of the COMMON block. As the offset from the symbol \p sym, generate the
+/// COMMON block member value (commonValue + offset) for the symbol.
+mlir::Value genCommonBlockMember(AbstractConverter &converter,
+ const Fortran::semantics::Symbol &sym,
+ mlir::Value commonValue);
+
/// Lower a symbol attributes given an optional storage \p and add it to the
/// provided symbol map. If \preAlloc is not provided, a temporary storage will
/// be allocated. This is a low level function that should only be used if
@@ -138,5 +151,6 @@ void genDeclareSymbol(Fortran::lower::AbstractConverter &converter,
/// Cray pointer symbol. Assert if the pointer symbol cannot be found.
Fortran::semantics::SymbolRef getCrayPointer(Fortran::semantics::SymbolRef sym);
-} // namespace Fortran::lower
+} // namespace lower
+} // namespace Fortran
#endif // FORTRAN_LOWER_CONVERT_VARIABLE_H
diff --git a/flang/lib/Lower/ConvertVariable.cpp b/flang/lib/Lower/ConvertVariable.cpp
index 7bdb501e757cc..b36a8a0caa1f0 100644
--- a/flang/lib/Lower/ConvertVariable.cpp
+++ b/flang/lib/Lower/ConvertVariable.cpp
@@ -1331,6 +1331,27 @@ void Fortran::lower::defineCommonBlocks(
finalizeCommonBlockDefinition(loc, converter, global, cmnBlkMems);
}
+/// FIXME: Share the code with `instantiateCommon` in ConvertVariable.cpp.
+mlir::Value Fortran::lower::genCommonBlockMember(
+ Fortran::lower::AbstractConverter &converter,
+ const Fortran::semantics::Symbol &sym, mlir::Value commonValue) {
+ fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
+ mlir::Location currentLocation = converter.getCurrentLocation();
+ mlir::IntegerType i8Ty = firOpBuilder.getIntegerType(8);
+ mlir::Type i8Ptr = firOpBuilder.getRefType(i8Ty);
+ mlir::Type seqTy = firOpBuilder.getRefType(firOpBuilder.getVarLenSeqTy(i8Ty));
+ mlir::Value base =
+ firOpBuilder.createConvert(currentLocation, seqTy, commonValue);
+ std::size_t byteOffset = sym.GetUltimate().offset();
+ mlir::Value offs = firOpBuilder.createIntegerConstant(
+ currentLocation, firOpBuilder.getIndexType(), byteOffset);
+ mlir::Value varAddr = firOpBuilder.create<fir::CoordinateOp>(
+ currentLocation, i8Ptr, base, mlir::ValueRange{offs});
+ mlir::Type symType = converter.genType(sym);
+ return firOpBuilder.createConvert(currentLocation,
+ firOpBuilder.getRefType(symType), varAddr);
+}
+
/// The COMMON block is a global structure. `var` will be at some offset
/// within the COMMON block. Adds the address of `var` (COMMON + offset) to
/// the symbol map.
diff --git a/flang/lib/Lower/OpenMP.cpp b/flang/lib/Lower/OpenMP.cpp
index adbc277d6b019..9906bcff548e8 100644
--- a/flang/lib/Lower/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP.cpp
@@ -1959,31 +1959,6 @@ static mlir::Operation *getCompareFromReductionOp(mlir::Operation *reductionOp,
return nullptr;
}
-/// The COMMON block is a global structure. \p commonValue is the base address
-/// of the COMMON block. As the offset from the symbol \p sym, generate the
-/// COMMON block member value (commonValue + offset) for the symbol.
-/// FIXME: Share the code with `instantiateCommon` in ConvertVariable.cpp.
-static mlir::Value
-genCommonBlockMember(Fortran::lower::AbstractConverter &converter,
- const Fortran::semantics::Symbol &sym,
- mlir::Value commonValue) {
- fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
- mlir::Location currentLocation = converter.getCurrentLocation();
- mlir::IntegerType i8Ty = firOpBuilder.getIntegerType(8);
- mlir::Type i8Ptr = firOpBuilder.getRefType(i8Ty);
- mlir::Type seqTy = firOpBuilder.getRefType(firOpBuilder.getVarLenSeqTy(i8Ty));
- mlir::Value base =
- firOpBuilder.createConvert(currentLocation, seqTy, commonValue);
- std::size_t byteOffset = sym.GetUltimate().offset();
- mlir::Value offs = firOpBuilder.createIntegerConstant(
- currentLocation, firOpBuilder.getIndexType(), byteOffset);
- mlir::Value varAddr = firOpBuilder.create<fir::CoordinateOp>(
- currentLocation, i8Ptr, base, mlir::ValueRange{offs});
- mlir::Type symType = converter.genType(sym);
- return firOpBuilder.createConvert(currentLocation,
- firOpBuilder.getRefType(symType), varAddr);
-}
-
// Get the extended value for \p val by extracting additional variable
// information from \p base.
static fir::ExtendedValue getExtendedValue(fir::ExtendedValue base,
@@ -2049,8 +2024,8 @@ static void threadPrivatizeVars(Fortran::lower::AbstractConverter &converter,
converter.bindSymbol(*common, commonThreadprivateValue);
commonSyms.insert(common);
}
- symThreadprivateValue =
- genCommonBlockMember(converter, *sym, commonThreadprivateValue);
+ symThreadprivateValue = Fortran::lower::genCommonBlockMember(
+ converter, *sym, commonThreadprivateValue);
} else {
symThreadprivateValue = genThreadprivateOp(*sym);
}
>From 59465ad8821df4fc391d231bc4b4f8096ea83df1 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Tue, 5 Dec 2023 11:52:50 -0600
Subject: [PATCH 2/2] Call `getCommonBlockMember` from `instantiateCommon`
This avoids code duplication, and addresses an outstanding FIXME.
---
flang/include/flang/Lower/ConvertVariable.h | 1 +
flang/lib/Lower/ConvertVariable.cpp | 48 ++++++++-------------
flang/lib/Lower/OpenMP.cpp | 6 +--
3 files changed, 22 insertions(+), 33 deletions(-)
diff --git a/flang/include/flang/Lower/ConvertVariable.h b/flang/include/flang/Lower/ConvertVariable.h
index 970353b5fed93..0ff3ca9bdeac7 100644
--- a/flang/include/flang/Lower/ConvertVariable.h
+++ b/flang/include/flang/Lower/ConvertVariable.h
@@ -76,6 +76,7 @@ void defineCommonBlocks(
/// of the COMMON block. As the offset from the symbol \p sym, generate the
/// COMMON block member value (commonValue + offset) for the symbol.
mlir::Value genCommonBlockMember(AbstractConverter &converter,
+ mlir::Location loc,
const Fortran::semantics::Symbol &sym,
mlir::Value commonValue);
diff --git a/flang/lib/Lower/ConvertVariable.cpp b/flang/lib/Lower/ConvertVariable.cpp
index b36a8a0caa1f0..676fecdb52a8b 100644
--- a/flang/lib/Lower/ConvertVariable.cpp
+++ b/flang/lib/Lower/ConvertVariable.cpp
@@ -1331,25 +1331,26 @@ void Fortran::lower::defineCommonBlocks(
finalizeCommonBlockDefinition(loc, converter, global, cmnBlkMems);
}
-/// FIXME: Share the code with `instantiateCommon` in ConvertVariable.cpp.
mlir::Value Fortran::lower::genCommonBlockMember(
- Fortran::lower::AbstractConverter &converter,
+ Fortran::lower::AbstractConverter &converter, mlir::Location loc,
const Fortran::semantics::Symbol &sym, mlir::Value commonValue) {
- fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
- mlir::Location currentLocation = converter.getCurrentLocation();
- mlir::IntegerType i8Ty = firOpBuilder.getIntegerType(8);
- mlir::Type i8Ptr = firOpBuilder.getRefType(i8Ty);
- mlir::Type seqTy = firOpBuilder.getRefType(firOpBuilder.getVarLenSeqTy(i8Ty));
- mlir::Value base =
- firOpBuilder.createConvert(currentLocation, seqTy, commonValue);
+ fir::FirOpBuilder &builder = converter.getFirOpBuilder();
+
std::size_t byteOffset = sym.GetUltimate().offset();
- mlir::Value offs = firOpBuilder.createIntegerConstant(
- currentLocation, firOpBuilder.getIndexType(), byteOffset);
- mlir::Value varAddr = firOpBuilder.create<fir::CoordinateOp>(
- currentLocation, i8Ptr, base, mlir::ValueRange{offs});
+ mlir::IntegerType i8Ty = builder.getIntegerType(8);
+ mlir::Type i8Ptr = builder.getRefType(i8Ty);
+ mlir::Type seqTy = builder.getRefType(builder.getVarLenSeqTy(i8Ty));
+ mlir::Value base = builder.createConvert(loc, seqTy, commonValue);
+
+ mlir::Value offs =
+ builder.createIntegerConstant(loc, builder.getIndexType(), byteOffset);
+ mlir::Value varAddr = builder.create<fir::CoordinateOp>(
+ loc, i8Ptr, base, mlir::ValueRange{offs});
mlir::Type symType = converter.genType(sym);
- return firOpBuilder.createConvert(currentLocation,
- firOpBuilder.getRefType(symType), varAddr);
+
+ return Fortran::semantics::FindEquivalenceSet(sym) != nullptr
+ ? castAliasToPointer(builder, loc, symType, varAddr)
+ : builder.createConvert(loc, builder.getRefType(symType), varAddr);
}
/// The COMMON block is a global structure. `var` will be at some offset
@@ -1374,21 +1375,8 @@ static void instantiateCommon(Fortran::lower::AbstractConverter &converter,
symMap.addSymbol(common, commonAddr);
}
- std::size_t byteOffset = varSym.GetUltimate().offset();
- mlir::IntegerType i8Ty = builder.getIntegerType(8);
- mlir::Type i8Ptr = builder.getRefType(i8Ty);
- mlir::Type seqTy = builder.getRefType(builder.getVarLenSeqTy(i8Ty));
- mlir::Value base = builder.createConvert(loc, seqTy, commonAddr);
- mlir::Value offs =
- builder.createIntegerConstant(loc, builder.getIndexType(), byteOffset);
- auto varAddr = builder.create<fir::CoordinateOp>(loc, i8Ptr, base,
- mlir::ValueRange{offs});
- mlir::Type symType = converter.genType(var.getSymbol());
- mlir::Value local;
- if (Fortran::semantics::FindEquivalenceSet(var.getSymbol()) != nullptr)
- local = castAliasToPointer(builder, loc, symType, varAddr);
- else
- local = builder.createConvert(loc, builder.getRefType(symType), varAddr);
+
+ mlir::Value local = genCommonBlockMember(converter, loc, varSym, commonAddr);
Fortran::lower::StatementContext stmtCtx;
mapSymbolAttributes(converter, var, symMap, stmtCtx, local);
}
diff --git a/flang/lib/Lower/OpenMP.cpp b/flang/lib/Lower/OpenMP.cpp
index 9906bcff548e8..0fa1ac76d57ed 100644
--- a/flang/lib/Lower/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP.cpp
@@ -2025,7 +2025,7 @@ static void threadPrivatizeVars(Fortran::lower::AbstractConverter &converter,
commonSyms.insert(common);
}
symThreadprivateValue = Fortran::lower::genCommonBlockMember(
- converter, *sym, commonThreadprivateValue);
+ converter, currentLocation, *sym, commonThreadprivateValue);
} else {
symThreadprivateValue = genThreadprivateOp(*sym);
}
@@ -3529,8 +3529,8 @@ void Fortran::lower::genThreadprivateOp(
currentLocation, commonValue.getType(), commonValue);
converter.bindSymbol(*common, commonThreadprivateValue);
// Generate the threadprivate value for the common block member.
- symThreadprivateValue =
- genCommonBlockMember(converter, sym, commonThreadprivateValue);
+ symThreadprivateValue = genCommonBlockMember(converter, currentLocation,
+ sym, commonThreadprivateValue);
} else if (!var.isGlobal()) {
// Non-global variable which can be in threadprivate directive must be one
// variable in main program, and it has implicit SAVE attribute. Take it as
More information about the flang-commits
mailing list