llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-flang-driver
Author: Eugene Epshteyn (eugeneepshteyn)
<details>
<summary>Changes</summary>
Drop the user-facing options that selected the legacy non-HLFIR lowering
path, the always-true `LowerToHighLevelFIR` lowering option, and the
descriptor-discretization debug switch:
- `-flang-experimental-hlfir` and `-flang-deprecated-no-hlfir` (flang
driver and `-fc1`)
- `-hlfir` / `--hlfir` (bbc)
- `--use-desc-for-alloc` (bbc, debug)
Remove every `if (lowerToHighLevelFIR()) { ... } else { ... }` branch in
`lib/Lower/`, keeping the HLFIR side. Delete the now-unused legacy
helpers in `Bridge.cpp` (`copyVarFIR`, both `genNoHLFIRPointerAssignment`
overloads, the legacy block of `genAssignment`) and the always-empty
`createMutableProperties` together with its four dead helpers in
`Allocatable.cpp`. Drop the corresponding `alwaysUseBox` parameter from
`createMutableBox`.
Note: tests were modified in https://github.com/llvm/llvm-project/pull/196137
Assisted-by: AI
---
Patch is 76.46 KiB, truncated to 20.00 KiB below, full version:
https://github.com/llvm/llvm-project/pull/196205.diff
15 Files Affected:
- (modified) clang/include/clang/Options/FlangOptions.td (-12)
- (modified) clang/lib/Driver/ToolChains/Flang.cpp (-2)
- (modified) flang/include/flang/Lower/Allocatable.h (+6-5)
- (modified) flang/include/flang/Lower/LoweringOptions.def (-3)
- (modified) flang/lib/Frontend/CompilerInvocation.cpp (-19)
- (modified) flang/lib/Frontend/FrontendActions.cpp (+1-7)
- (modified) flang/lib/Lower/Allocatable.cpp (+5-139)
- (modified) flang/lib/Lower/Bridge.cpp (+100-564)
- (modified) flang/lib/Lower/CallInterface.cpp (+1-10)
- (modified) flang/lib/Lower/ConvertCall.cpp (+7-13)
- (modified) flang/lib/Lower/ConvertConstant.cpp (-12)
- (modified) flang/lib/Lower/ConvertType.cpp (+1-29)
- (modified) flang/lib/Lower/ConvertVariable.cpp (+28-80)
- (modified) flang/lib/Lower/HostAssociations.cpp (+2-5)
- (modified) flang/tools/bbc/bbc.cpp (+1-6)
``````````diff
diff --git a/clang/include/clang/Options/FlangOptions.td
b/clang/include/clang/Options/FlangOptions.td
index 1ab83b6ffbbad..50e4642358b71 100644
--- a/clang/include/clang/Options/FlangOptions.td
+++ b/clang/include/clang/Options/FlangOptions.td
@@ -114,18 +114,6 @@ def static_libflangrt : Flag<["-"], "static-libflangrt">,
HelpText<"Link the flang-rt static library">, Group<Link_Group>,
Visibility<[FlangOption]>, Flags<[NoArgumentUnused]>;
-//===----------------------------------------------------------------------===//
-// FlangOption + NoXarchOption
-//===----------------------------------------------------------------------===//
-
-def flang_experimental_hlfir : Flag<["-"], "flang-experimental-hlfir">,
- Flags<[HelpHidden]>, Visibility<[FlangOption, FC1Option]>,
- HelpText<"Use HLFIR lowering (experimental)">;
-
-def flang_deprecated_no_hlfir : Flag<["-"], "flang-deprecated-no-hlfir">,
- Flags<[HelpHidden]>, Visibility<[FlangOption, FC1Option]>,
- HelpText<"Do not use HLFIR lowering (deprecated)">;
-
//===----------------------------------------------------------------------===//
// FlangOption + CoreOption + NoXarchOption
//===----------------------------------------------------------------------===//
diff --git a/clang/lib/Driver/ToolChains/Flang.cpp
b/clang/lib/Driver/ToolChains/Flang.cpp
index ce503b74295e4..082df7beb4b85 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -252,8 +252,6 @@ void Flang::addCodegenOptions(const ArgList &Args,
Args.addAllArgs(
CmdArgs,
{options::OPT_fdo_concurrent_to_openmp_EQ,
- options::OPT_flang_experimental_hlfir,
- options::OPT_flang_deprecated_no_hlfir,
options::OPT_fno_ppc_native_vec_elem_order,
options::OPT_fppc_native_vec_elem_order, options::OPT_finit_global_zero,
options::OPT_fno_init_global_zero, options::OPT_frepack_arrays,
diff --git a/flang/include/flang/Lower/Allocatable.h
b/flang/include/flang/Lower/Allocatable.h
index 0e89af94af40f..515fd20b2bcb5 100644
--- a/flang/include/flang/Lower/Allocatable.h
+++ b/flang/include/flang/Lower/Allocatable.h
@@ -68,11 +68,12 @@ void genDeallocateIfAllocated(AbstractConverter &converter,
/// Create a MutableBoxValue for an allocatable or pointer entity.
/// If the variables is a local variable that is not a dummy, it will be
/// initialized to unallocated/diassociated status.
-fir::MutableBoxValue
-createMutableBox(AbstractConverter &converter, mlir::Location loc,
- const pft::Variable &var, mlir::Value boxAddr,
- mlir::ValueRange nonDeferredParams, bool alwaysUseBox,
- unsigned allocator = kDefaultAllocator);
+fir::MutableBoxValue createMutableBox(AbstractConverter &converter,
+ mlir::Location loc,
+ const pft::Variable &var,
+ mlir::Value boxAddr,
+ mlir::ValueRange nonDeferredParams,
+ unsigned allocator = kDefaultAllocator);
/// Assign a boxed value to a boxed variable, \p box (known as a
/// MutableBoxValue). Expression \p source will be lowered to build the
diff --git a/flang/include/flang/Lower/LoweringOptions.def
b/flang/include/flang/Lower/LoweringOptions.def
index 0b829bf3e08af..e89ad75704609 100644
--- a/flang/include/flang/Lower/LoweringOptions.def
+++ b/flang/include/flang/Lower/LoweringOptions.def
@@ -24,9 +24,6 @@ LOWERINGOPT(Name, Bits, Default)
/// If true, lower transpose without a runtime call.
ENUM_LOWERINGOPT(OptimizeTranspose, unsigned, 1, 1)
-/// If true, lower to High level FIR before lowering to FIR. On by default.
-ENUM_LOWERINGOPT(LowerToHighLevelFIR, unsigned, 1, 1)
-
/// If true, reverse PowerPC native vector element order.
ENUM_LOWERINGOPT(NoPPCNativeVecElemOrder, unsigned, 1, 0)
diff --git a/flang/lib/Frontend/CompilerInvocation.cpp
b/flang/lib/Frontend/CompilerInvocation.cpp
index e7f4762e167fb..7205eb4548968 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -1629,25 +1629,6 @@ bool CompilerInvocation::createFromArgs(
success = false;
}
- // -flang-experimental-hlfir
- if (args.hasArg(clang::options::OPT_flang_experimental_hlfir) ||
- args.hasArg(clang::options::OPT_emit_hlfir)) {
- invoc.loweringOpts.setLowerToHighLevelFIR(true);
- }
-
- // -flang-deprecated-no-hlfir
- if (args.hasArg(clang::options::OPT_flang_deprecated_no_hlfir) &&
- !args.hasArg(clang::options::OPT_emit_hlfir)) {
- if (args.hasArg(clang::options::OPT_flang_experimental_hlfir)) {
- const unsigned diagID = diags.getCustomDiagID(
- clang::DiagnosticsEngine::Error,
- "Options '-flang-experimental-hlfir' and "
- "'-flang-deprecated-no-hlfir' cannot be both specified");
- diags.Report(diagID);
- }
- invoc.loweringOpts.setLowerToHighLevelFIR(false);
- }
-
// -fno-ppc-native-vector-element-order
if (args.hasArg(clang::options::OPT_fno_ppc_native_vec_elem_order)) {
invoc.loweringOpts.setNoPPCNativeVecElemOrder(true);
diff --git a/flang/lib/Frontend/FrontendActions.cpp
b/flang/lib/Frontend/FrontendActions.cpp
index 4e058786a9a72..9ad1da0011ef4 100644
--- a/flang/lib/Frontend/FrontendActions.cpp
+++ b/flang/lib/Frontend/FrontendActions.cpp
@@ -1345,8 +1345,6 @@ void CodeGenAction::executeAction() {
clang::DiagnosticsEngine &diags = ci.getDiagnostics();
const CodeGenOptions &codeGenOpts = ci.getInvocation().getCodeGenOpts();
const TargetOptions &targetOpts = ci.getInvocation().getTargetOpts();
- Fortran::lower::LoweringOptions &loweringOpts =
- ci.getInvocation().getLoweringOpts();
mlir::DefaultTimingManager &timingMgr = ci.getTimingManager();
mlir::TimingScope &timingScopeRoot = ci.getTimingScopeRoot();
@@ -1375,16 +1373,12 @@ void CodeGenAction::executeAction() {
}
if (action == BackendActionTy::Backend_EmitFIR) {
- if (loweringOpts.getLowerToHighLevelFIR()) {
- lowerHLFIRToFIR();
- }
+ lowerHLFIRToFIR();
mlirModule->print(ci.isOutputStreamNull() ? *os : ci.getOutputStream());
return;
}
if (action == BackendActionTy::Backend_EmitHLFIR) {
- assert(loweringOpts.getLowerToHighLevelFIR() &&
- "Lowering must have been configured to emit HLFIR");
mlirModule->print(ci.isOutputStreamNull() ? *os : ci.getOutputStream());
return;
}
diff --git a/flang/lib/Lower/Allocatable.cpp b/flang/lib/Lower/Allocatable.cpp
index 5cbfba23cffdf..8ca861105ce23 100644
--- a/flang/lib/Lower/Allocatable.cpp
+++ b/flang/lib/Lower/Allocatable.cpp
@@ -48,16 +48,6 @@ static llvm::cl::opt<bool> useAllocateRuntime(
"use-alloc-runtime",
llvm::cl::desc("Lower allocations to fortran runtime calls"),
llvm::cl::init(false));
-/// Switch to force lowering of allocatable and pointers to descriptors in all
-/// cases. This is now turned on by default since that is what will happen with
-/// HLFIR lowering, so this allows getting early feedback of the impact.
-/// If this turns out to cause performance regressions, a dedicated fir.box
-/// "discretization pass" would make more sense to cover all the fir.box usage
-/// (taking advantage of any future inlining for instance).
-static llvm::cl::opt<bool> useDescForMutableBox(
- "use-desc-for-alloc",
- llvm::cl::desc("Always use descriptors for POINTER and ALLOCATABLE"),
- llvm::cl::init(true));
//===----------------------------------------------------------------------===//
// Error management
@@ -1017,123 +1007,12 @@ void Fortran::lower::genDeallocateStmt(
// MutableBoxValue creation implementation
//===----------------------------------------------------------------------===//
-/// Is this symbol a pointer to a pointer array that does not have the
-/// CONTIGUOUS attribute ?
-static inline bool
-isNonContiguousArrayPointer(const Fortran::semantics::Symbol &sym) {
- return Fortran::semantics::IsPointer(sym) && sym.Rank() != 0 &&
- !sym.attrs().test(Fortran::semantics::Attr::CONTIGUOUS);
-}
-
-/// Is this symbol a polymorphic pointer?
-static inline bool isPolymorphicPointer(const Fortran::semantics::Symbol &sym)
{
- return Fortran::semantics::IsPointer(sym) &&
- Fortran::semantics::IsPolymorphic(sym);
-}
-
-/// Is this symbol a polymorphic allocatable?
-static inline bool
-isPolymorphicAllocatable(const Fortran::semantics::Symbol &sym) {
- return Fortran::semantics::IsAllocatable(sym) &&
- Fortran::semantics::IsPolymorphic(sym);
-}
-
-/// Is this a local procedure symbol in a procedure that contains internal
-/// procedures ?
-static bool mayBeCapturedInInternalProc(const Fortran::semantics::Symbol &sym)
{
- const Fortran::semantics::Scope &owner = sym.owner();
- Fortran::semantics::Scope::Kind kind = owner.kind();
- // Test if this is a procedure scope that contains a subprogram scope that is
- // not an interface.
- if (kind == Fortran::semantics::Scope::Kind::Subprogram ||
- kind == Fortran::semantics::Scope::Kind::MainProgram)
- for (const Fortran::semantics::Scope &childScope : owner.children())
- if (childScope.kind() == Fortran::semantics::Scope::Kind::Subprogram)
- if (const Fortran::semantics::Symbol *childSym = childScope.symbol())
- if (const auto *details =
- childSym->detailsIf<Fortran::semantics::SubprogramDetails>())
- if (!details->isInterface())
- return true;
- return false;
-}
-
-/// In case it is safe to track the properties in variables outside a
-/// descriptor, create the variables to hold the mutable properties of the
-/// entity var. The variables are not initialized here.
-static fir::MutableProperties
-createMutableProperties(Fortran::lower::AbstractConverter &converter,
- mlir::Location loc,
- const Fortran::lower::pft::Variable &var,
- mlir::ValueRange nonDeferredParams, bool alwaysUseBox)
{
- fir::FirOpBuilder &builder = converter.getFirOpBuilder();
- const Fortran::semantics::Symbol &sym = var.getSymbol();
- // Globals and dummies may be associated, creating local variables would
- // require keeping the values and descriptor before and after every single
- // impure calls in the current scope (not only the ones taking the variable
as
- // arguments. All.) Volatile means the variable may change in ways not
defined
- // per Fortran, so lowering can most likely not keep the descriptor and
values
- // in sync as needed.
- // Pointers to non contiguous arrays need to be represented with a fir.box to
- // account for the discontiguity.
- // Pointer/Allocatable in internal procedure are descriptors in the host
link,
- // and it would increase complexity to sync this descriptor with the local
- // values every time the host link is escaping.
- if (alwaysUseBox || var.isGlobal() || Fortran::semantics::IsDummy(sym) ||
- Fortran::semantics::IsFunctionResult(sym) ||
- sym.attrs().test(Fortran::semantics::Attr::VOLATILE) ||
- isNonContiguousArrayPointer(sym) || useAllocateRuntime ||
- useDescForMutableBox || mayBeCapturedInInternalProc(sym) ||
- isPolymorphicPointer(sym) || isPolymorphicAllocatable(sym))
- return {};
- fir::MutableProperties mutableProperties;
- std::string name = converter.mangleName(sym);
- mlir::Type baseAddrTy = converter.genType(sym);
- if (auto boxType = mlir::dyn_cast<fir::BaseBoxType>(baseAddrTy))
- baseAddrTy = boxType.getEleTy();
- // Allocate and set a variable to hold the address.
- // It will be set to null in setUnallocatedStatus.
- mutableProperties.addr =
- builder.allocateLocal(loc, baseAddrTy, name + ".addr", "",
- /*shape=*/{}, /*typeparams=*/{});
- // Allocate variables to hold lower bounds and extents.
- int rank = sym.Rank();
- mlir::Type idxTy = builder.getIndexType();
- for (decltype(rank) i = 0; i < rank; ++i) {
- mlir::Value lboundVar =
- builder.allocateLocal(loc, idxTy, name + ".lb" + std::to_string(i), "",
- /*shape=*/{}, /*typeparams=*/{});
- mlir::Value extentVar =
- builder.allocateLocal(loc, idxTy, name + ".ext" + std::to_string(i),
"",
- /*shape=*/{}, /*typeparams=*/{});
- mutableProperties.lbounds.emplace_back(lboundVar);
- mutableProperties.extents.emplace_back(extentVar);
- }
-
- // Allocate variable to hold deferred length parameters.
- mlir::Type eleTy = baseAddrTy;
- if (auto newTy = fir::dyn_cast_ptrEleTy(eleTy))
- eleTy = newTy;
- if (auto seqTy = mlir::dyn_cast<fir::SequenceType>(eleTy))
- eleTy = seqTy.getEleTy();
- if (auto record = mlir::dyn_cast<fir::RecordType>(eleTy))
- if (record.getNumLenParams() != 0)
- TODO(loc, "deferred length type parameters.");
- if (fir::isa_char(eleTy) && nonDeferredParams.empty()) {
- mlir::Value lenVar = builder.allocateLocal(
- loc, builder.getCharacterLengthType(), name + ".len", "", /*shape=*/{},
- /*typeparams=*/{});
- mutableProperties.deferredParams.emplace_back(lenVar);
- }
- return mutableProperties;
-}
-
fir::MutableBoxValue Fortran::lower::createMutableBox(
Fortran::lower::AbstractConverter &converter, mlir::Location loc,
const Fortran::lower::pft::Variable &var, mlir::Value boxAddr,
- mlir::ValueRange nonDeferredParams, bool alwaysUseBox, unsigned allocator)
{
- fir::MutableProperties mutableProperties = createMutableProperties(
- converter, loc, var, nonDeferredParams, alwaysUseBox);
- fir::MutableBoxValue box(boxAddr, nonDeferredParams, mutableProperties);
+ mlir::ValueRange nonDeferredParams, unsigned allocator) {
+ fir::MutableBoxValue box(boxAddr, nonDeferredParams,
+ /*mutableProperties=*/{});
fir::FirOpBuilder &builder = converter.getFirOpBuilder();
if (!var.isGlobal() && !Fortran::semantics::IsDummy(var.getSymbol()))
fir::factory::disassociateMutableBox(builder, loc, box,
@@ -1163,22 +1042,9 @@ void Fortran::lower::associateMutableBox(
cuf::genPointerSync(box.getAddr(), builder);
return;
}
- if (converter.getLoweringOptions().getLowerToHighLevelFIR()) {
- fir::ExtendedValue rhs = converter.genExprAddr(loc, source, stmtCtx);
- fir::factory::associateMutableBox(builder, loc, box, rhs, lbounds);
- cuf::genPointerSync(box.getAddr(), builder);
- return;
- }
- // The right hand side is not be evaluated into a temp. Array sections can
- // typically be represented as a value of type `!fir.box`. However, an
- // expression that uses vector subscripts cannot be emboxed. In that case,
- // generate a reference to avoid having to later use a fir.rebox to implement
- // the pointer association.
- fir::ExtendedValue rhs = isArraySectionWithoutVectorSubscript(source)
- ? converter.genExprBox(loc, source, stmtCtx)
- : converter.genExprAddr(loc, source, stmtCtx);
-
+ fir::ExtendedValue rhs = converter.genExprAddr(loc, source, stmtCtx);
fir::factory::associateMutableBox(builder, loc, box, rhs, lbounds);
+ cuf::genPointerSync(box.getAddr(), builder);
}
bool Fortran::lower::isWholeAllocatable(const Fortran::lower::SomeExpr &expr) {
diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index c5709e1cd94d4..091f267b43c17 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -808,11 +808,8 @@ class FirConverter : public
Fortran::lower::AbstractConverter {
Fortran::lower::StatementContext &context,
mlir::Location *locPtr = nullptr) override final {
mlir::Location loc = locPtr ? *locPtr : toLocation();
- if (lowerToHighLevelFIR())
- return Fortran::lower::convertExprToAddress(loc, *this, expr,
- localSymbols, context);
- return Fortran::lower::createSomeExtendedAddress(loc, *this, expr,
- localSymbols, context);
+ return Fortran::lower::convertExprToAddress(loc, *this, expr, localSymbols,
+ context);
}
fir::ExtendedValue
@@ -820,21 +817,15 @@ class FirConverter : public
Fortran::lower::AbstractConverter {
Fortran::lower::StatementContext &context,
mlir::Location *locPtr = nullptr) override final {
mlir::Location loc = locPtr ? *locPtr : toLocation();
- if (lowerToHighLevelFIR())
- return Fortran::lower::convertExprToValue(loc, *this, expr, localSymbols,
- context);
- return Fortran::lower::createSomeExtendedExpression(loc, *this, expr,
- localSymbols, context);
+ return Fortran::lower::convertExprToValue(loc, *this, expr, localSymbols,
+ context);
}
fir::ExtendedValue
genExprBox(mlir::Location loc, const Fortran::lower::SomeExpr &expr,
Fortran::lower::StatementContext &stmtCtx) override final {
- if (lowerToHighLevelFIR())
- return Fortran::lower::convertExprToBox(loc, *this, expr, localSymbols,
- stmtCtx);
- return Fortran::lower::createBoxValue(loc, *this, expr, localSymbols,
- stmtCtx);
+ return Fortran::lower::convertExprToBox(loc, *this, expr, localSymbols,
+ stmtCtx);
}
Fortran::evaluate::FoldingContext &getFoldingContext() override final {
@@ -1378,56 +1369,51 @@ class FirConverter : public
Fortran::lower::AbstractConverter {
Fortran::lower::SymMap *symMap = nullptr,
bool forceHlfirBase = false) {
symMap = symMap ? symMap : &localSymbols;
- if (lowerToHighLevelFIR()) {
- if (std::optional<fir::FortranVariableOpInterface> var =
- symMap->lookupVariableDefinition(sym)) {
- auto exv = hlfir::translateToExtendedValue(toLocation(), *builder,
*var,
- forceHlfirBase);
- return exv.match(
- [](mlir::Value x) -> Fortran::lower::SymbolBox {
- return Fortran::lower::SymbolBox::Intrinsic{x};
- },
- [](auto x) -> Fortran::lower::SymbolBox { return x; });
- }
-
- // Entry character result represented as an argument pair
- // needs to be represented in the symbol table even before
- // we can create DeclareOp for it. The temporary mapping
- // is EmboxCharOp that conveys the address and length information.
- // After mapSymbolAttributes is done, the mapping is replaced
- // with the new DeclareOp, and the following table lookups
- // do not reach here.
- if (sym.IsFuncResult())
- if (const Fortran::semantics::DeclTypeSpec *declTy = sym.GetType())
- if (declTy->category() ==
- Fortran::semantics::DeclTypeSpec::Category::Character)
- return symMap->lookupSymbol(sym);
-
- // Procedure dummies are not mapped with an hlfir.declare because
- // they are not "variable" (cannot be assigned to), and it would
- // make hlfir.declare more complex than it needs to to allow this.
- // Do a regular lookup.
- if (Fortran::semantics::IsProcedure(sym))
- return symMap->lookupSymbol(sym);
-
- // Commonblock names are not variables, but in some lowerings (like
- // OpenMP) it is useful to maintain the address of the commonblock in an
- // MLIR value and query it. hlfir.declare need not be created for these.
- if (sym.detailsIf<Fortran::semantics::CommonBlockDetails>())
- return symMap->lookupSymbol(sym);
-
- // For symbols to be privatized in OMP, the symbol is mapped to an
- // instance of `SymbolBox::Intrinsic` (i.e. a direct mapping to an MLIR
- // SSA value). This MLIR SSA value is the block argument to the
- // `omp.private`'s `alloc` block. If this is the case, we return this
- // `SymbolBox::Intrinsic` value.
- if (Fortran::lower::SymbolBox v = symMap->lookupSymbol(sym))
- return v;
-
- return {};
- }
+ if (std::optional<fir::FortranVariableOpInterface> var =
+ symMap->lookupVariableDefinition...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/196205
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
