From 4ca111d4cb4c0b425268c86b54fb19c4be2e88dd Mon Sep 17 00:00:00 2001 From: Andrzej Warzynski Date: Mon, 28 Mar 2022 10:46:47 +0000 Subject: [PATCH] Revert "[flang] Add & use a better visit()" This reverts commit 2ab9990c9eb79682a4d4b183dfbc7612d3e55328. It has caused multiple build failures: * https://lab.llvm.org/buildbot/#/builders/177/builds/4346 * https://lab.llvm.org/buildbot/#/builders/180/builds/3803 * https://lab.llvm.org/buildbot/#/builders/175/builds/10419 * https://lab.llvm.org/buildbot/#/builders/191/builds/4318 * https://lab.llvm.org/buildbot/#/builders/173/builds/4274 * https://lab.llvm.org/buildbot/#/builders/181/builds/4297 All these bots failed with a time-out: ``` command timed out: 1200 seconds without output running [b'ninja', b'-j', b'32'], attempting to kill ``` I'm guessing that that's due to template instantiations failing at some point (https://reviews.llvm.org/D122441 introduced a custom implementation of std::visit). Everything seems fine when either: * building on X86 with GCC or Clang (tested with GCC 9.3 and Clang 12) * building on AArch64 with GCC (tested with GCC 11) --- flang/include/flang/Common/idioms.h | 5 +- flang/include/flang/Common/template.h | 4 +- flang/include/flang/Common/unwrap.h | 5 +- flang/include/flang/Common/visit.h | 94 --- flang/include/flang/Evaluate/expression.h | 2 +- .../include/flang/Evaluate/fold-designator.h | 8 +- flang/include/flang/Evaluate/initial-image.h | 2 +- flang/include/flang/Evaluate/shape.h | 2 +- flang/include/flang/Evaluate/tools.h | 43 +- flang/include/flang/Evaluate/traverse.h | 2 +- .../include/flang/Parser/parse-tree-visitor.h | 5 +- flang/include/flang/Parser/tools.h | 2 +- flang/include/flang/Semantics/expression.h | 7 +- flang/include/flang/Semantics/symbol.h | 43 +- flang/include/flang/Semantics/tools.h | 3 +- flang/lib/Evaluate/call.cpp | 17 +- flang/lib/Evaluate/characteristics.cpp | 69 +- flang/lib/Evaluate/check-expression.cpp | 22 +- flang/lib/Evaluate/expression.cpp | 14 +- flang/lib/Evaluate/fold-designator.cpp | 8 +- flang/lib/Evaluate/fold-implementation.h | 28 +- flang/lib/Evaluate/fold-integer.cpp | 40 +- flang/lib/Evaluate/fold-logical.cpp | 4 +- flang/lib/Evaluate/fold-real.cpp | 2 +- flang/lib/Evaluate/fold.cpp | 35 +- flang/lib/Evaluate/formatting.cpp | 52 +- flang/lib/Evaluate/shape.cpp | 4 +- flang/lib/Evaluate/tools.cpp | 110 ++- flang/lib/Evaluate/type.cpp | 2 +- flang/lib/Evaluate/variable.cpp | 156 ++-- flang/lib/Parser/message.cpp | 28 +- flang/lib/Parser/parse-tree.cpp | 10 +- flang/lib/Parser/provenance.cpp | 75 +- flang/lib/Parser/tools.cpp | 42 +- flang/lib/Parser/unparse.cpp | 684 +++++++++--------- flang/lib/Semantics/canonicalize-do.cpp | 2 +- flang/lib/Semantics/check-allocate.cpp | 4 +- flang/lib/Semantics/check-call.cpp | 2 +- flang/lib/Semantics/check-case.cpp | 41 +- flang/lib/Semantics/check-data.cpp | 27 +- flang/lib/Semantics/check-deallocate.cpp | 4 +- flang/lib/Semantics/check-declarations.cpp | 62 +- flang/lib/Semantics/check-do-forall.cpp | 77 +- flang/lib/Semantics/check-io.cpp | 2 +- flang/lib/Semantics/check-nullify.cpp | 2 +- flang/lib/Semantics/check-omp-structure.cpp | 119 ++- flang/lib/Semantics/check-select-rank.cpp | 4 +- flang/lib/Semantics/check-select-type.cpp | 14 +- flang/lib/Semantics/data-to-inits.cpp | 4 +- flang/lib/Semantics/expression.cpp | 97 ++- flang/lib/Semantics/mod-file.cpp | 179 +++-- flang/lib/Semantics/pointer-assignment.cpp | 6 +- flang/lib/Semantics/program-tree.cpp | 30 +- flang/lib/Semantics/resolve-directives.cpp | 36 +- flang/lib/Semantics/resolve-names-utils.cpp | 31 +- flang/lib/Semantics/resolve-names.cpp | 198 +++-- flang/lib/Semantics/rewrite-parse-tree.cpp | 4 +- flang/lib/Semantics/runtime-type-info.cpp | 50 +- flang/lib/Semantics/symbol.cpp | 32 +- flang/lib/Semantics/tools.cpp | 14 +- flang/runtime/io-stmt.cpp | 51 +- flang/runtime/io-stmt.h | 3 +- 62 files changed, 1297 insertions(+), 1427 deletions(-) delete mode 100644 flang/include/flang/Common/visit.h diff --git a/flang/include/flang/Common/idioms.h b/flang/include/flang/Common/idioms.h index 1a086162f1e2a..84a8fd5be4cba 100644 --- a/flang/include/flang/Common/idioms.h +++ b/flang/include/flang/Common/idioms.h @@ -23,7 +23,6 @@ #error g++ >= 7.2 is required #endif -#include "visit.h" #include "llvm/Support/Compiler.h" #include #include @@ -50,8 +49,8 @@ using namespace std::literals::string_literals; namespace Fortran::common { // Helper templates for combining a list of lambdas into an anonymous -// struct for use with common::visit() on a std::variant<> sum type. -// E.g.: common::visit(visitors{ +// struct for use with std::visit() on a std::variant<> sum type. +// E.g.: std::visit(visitors{ // [&](const firstType &x) { ... }, // [&](const secondType &x) { ... }, // ... diff --git a/flang/include/flang/Common/template.h b/flang/include/flang/Common/template.h index 2a9958f74db38..f31b0afa97fb7 100644 --- a/flang/include/flang/Common/template.h +++ b/flang/include/flang/Common/template.h @@ -118,14 +118,14 @@ template const A *GetPtrFromOptional(const std::optional &x) { // Copy a value from one variant type to another. The types allowed in the // source variant must all be allowed in the destination variant type. template TOV CopyVariant(const FROMV &u) { - return common::visit([](const auto &x) -> TOV { return {x}; }, u); + return std::visit([](const auto &x) -> TOV { return {x}; }, u); } // Move a value from one variant type to another. The types allowed in the // source variant must all be allowed in the destination variant type. template common::IfNoLvalue MoveVariant(FROMV &&u) { - return common::visit( + return std::visit( [](auto &&x) -> TOV { return {std::move(x)}; }, std::move(u)); } diff --git a/flang/include/flang/Common/unwrap.h b/flang/include/flang/Common/unwrap.h index edb343d77b537..b6ea4a1546096 100644 --- a/flang/include/flang/Common/unwrap.h +++ b/flang/include/flang/Common/unwrap.h @@ -12,7 +12,6 @@ #include "indirection.h" #include "reference-counted.h" #include "reference.h" -#include "visit.h" #include #include #include @@ -104,7 +103,7 @@ struct UnwrapperHelper { template static A *Unwrap(std::variant &u) { - return common::visit( + return std::visit( [](auto &x) -> A * { using Ty = std::decay_t(x))>; if constexpr (!std::is_const_v> || @@ -118,7 +117,7 @@ struct UnwrapperHelper { template static auto Unwrap(const std::variant &u) -> std::add_const_t * { - return common::visit( + return std::visit( [](const auto &x) -> std::add_const_t * { return Unwrap(x); }, u); } diff --git a/flang/include/flang/Common/visit.h b/flang/include/flang/Common/visit.h deleted file mode 100644 index db68fdbbf1099..0000000000000 --- a/flang/include/flang/Common/visit.h +++ /dev/null @@ -1,94 +0,0 @@ -//===-- include/flang/Common/visit.h ----------------------------*- C++ -*-===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - -// common::visit() is a drop-in replacement for std::visit() that reduces both -// compiler build time and compiler execution time modestly, and reduces -// compiler build memory requirements significantly (overall & maximum). -// It does not require redefinition of std::variant<>. -// -// The C++ standard mandates that std::visit be O(1), but most variants are -// small and O(logN) is faster in practice to compile and execute, avoiding -// the need to build a dispatch table. -// -// Define FLANG_USE_STD_VISIT to avoid this code and make common::visit() an -// alias for ::std::visit(). -// -// -// With GCC 9.3.0 on a Haswell x86 Ubuntu system, doing out-of-tree builds: -// Before: -// build: -// 6948.53user 212.48system 27:32.92elapsed 433%CPU -// (0avgtext+0avgdata 6429568maxresident)k -// 36181912inputs+8943720outputs (3613684major+97908699minor)pagefaults 0swaps -// execution of tests: -// 205.99user 26.05system 1:08.87elapsed 336%CPU -// (0avgtext+0avgdata 2671452maxresident)k -// 244432inputs+355464outputs (422major+8746468minor)pagefaults 0swaps -// After: -// build: -// 6651.91user 182.57system 25:15.73elapsed 450%CPU -// (0avgtext+0avgdata 6209296maxresident)k -// 17413480inputs+6376360outputs (1567210major+93068230minor)pagefaults 0swaps -// execution of tests: -// 201.42user 25.91system 1:04.68elapsed 351%CPU -// (0avgtext+0avgdata 2661424maxresident)k -// 238840inputs+295912outputs (428major+8489300minor)pagefaults 0swaps - -#ifndef FORTRAN_COMMON_VISIT_H_ -#define FORTRAN_COMMON_VISIT_H_ - -#include -#include - -namespace Fortran::common { -namespace log2visit { - -template -inline RESULT Log2VisitHelper( - VISITOR &&visitor, std::size_t which, VARIANT &&...u) { - if constexpr (LOW == HIGH) { - return visitor(std::get(std::forward(u))...); - } else { - static constexpr std::size_t mid{(HIGH + LOW) / 2}; - if (which <= mid) { - return Log2VisitHelper( - std::forward(visitor), which, std::forward(u)...); - } else { - return Log2VisitHelper<(mid + 1), HIGH, RESULT>( - std::forward(visitor), which, std::forward(u)...); - } - } -} - -template -inline auto visit(VISITOR &&visitor, VARIANT &&...u) - -> decltype(visitor(std::get<0>(std::forward(u))...)) { - using Result = decltype(visitor(std::get<0>(std::forward(u))...)); - if constexpr (sizeof...(u) == 1) { - static constexpr std::size_t high{ - (std::variant_size_v> * ...) - 1}; - return Log2VisitHelper<0, high, Result>(std::forward(visitor), - u.index()..., std::forward(u)...); - } else { - // TODO: figure out how to do multiple variant arguments - return ::std::visit( - std::forward(visitor), std::forward(u)...); - } -} - -} // namespace log2visit - -#ifdef FLANG_USE_STD_VISIT -using ::std::visit; -#else -using Fortran::common::log2visit::visit; -#endif - -} // namespace Fortran::common -#endif // FORTRAN_COMMON_VISIT_H_ diff --git a/flang/include/flang/Evaluate/expression.h b/flang/include/flang/Evaluate/expression.h index 90309affbe257..f24750e4f0944 100644 --- a/flang/include/flang/Evaluate/expression.h +++ b/flang/include/flang/Evaluate/expression.h @@ -646,7 +646,7 @@ template <> class Relational { EVALUATE_UNION_CLASS_BOILERPLATE(Relational) static constexpr DynamicType GetType() { return Result::GetType(); } int Rank() const { - return common::visit([](const auto &x) { return x.Rank(); }, u); + return std::visit([](const auto &x) { return x.Rank(); }, u); } llvm::raw_ostream &AsFortran(llvm::raw_ostream &o) const; common::MapTemplate u; diff --git a/flang/include/flang/Evaluate/fold-designator.h b/flang/include/flang/Evaluate/fold-designator.h index f246bd12020e0..457e86d4fdad9 100644 --- a/flang/include/flang/Evaluate/fold-designator.h +++ b/flang/include/flang/Evaluate/fold-designator.h @@ -67,7 +67,7 @@ class DesignatorFolder { template std::optional FoldDesignator(const Expr &expr) { - return common::visit( + return std::visit( [&](const auto &x) { return FoldDesignator(x, elementNumber_++); }, expr.u); } @@ -98,7 +98,7 @@ class DesignatorFolder { template std::optional FoldDesignator( const Expr &expr, ConstantSubscript which) { - return common::visit( + return std::visit( [&](const auto &x) { return FoldDesignator(x, which); }, expr.u); } @@ -110,14 +110,14 @@ class DesignatorFolder { template std::optional FoldDesignator( const Designator &designator, ConstantSubscript which) { - return common::visit( + return std::visit( [&](const auto &x) { return FoldDesignator(x, which); }, designator.u); } template std::optional FoldDesignator( const Designator> &designator, ConstantSubscript which) { - return common::visit( + return std::visit( common::visitors{ [&](const Substring &ss) { if (const auto *dataRef{ss.GetParentIf()}) { diff --git a/flang/include/flang/Evaluate/initial-image.h b/flang/include/flang/Evaluate/initial-image.h index fcc18835418a8..596b1f77d1790 100644 --- a/flang/include/flang/Evaluate/initial-image.h +++ b/flang/include/flang/Evaluate/initial-image.h @@ -88,7 +88,7 @@ class InitialImage { template Result Add(ConstantSubscript offset, std::size_t bytes, const Expr &x, FoldingContext &c) { - return common::visit( + return std::visit( [&](const auto &y) { return Add(offset, bytes, y, c); }, x.u); } diff --git a/flang/include/flang/Evaluate/shape.h b/flang/include/flang/Evaluate/shape.h index 378c0d6734f40..246f346dcc327 100644 --- a/flang/include/flang/Evaluate/shape.h +++ b/flang/include/flang/Evaluate/shape.h @@ -167,7 +167,7 @@ class GetShapeHelper template MaybeExtentExpr GetArrayConstructorValueExtent( const ArrayConstructorValue &value) const { - return common::visit( + return std::visit( common::visitors{ [&](const Expr &x) -> MaybeExtentExpr { if (auto xShape{ diff --git a/flang/include/flang/Evaluate/tools.h b/flang/include/flang/Evaluate/tools.h index 5523b1bf035b2..ae6772a871070 100644 --- a/flang/include/flang/Evaluate/tools.h +++ b/flang/include/flang/Evaluate/tools.h @@ -83,7 +83,7 @@ template bool IsAssumedRank(const Designator &designator) { } } template bool IsAssumedRank(const Expr &expr) { - return common::visit([](const auto &x) { return IsAssumedRank(x); }, expr.u); + return std::visit([](const auto &x) { return IsAssumedRank(x); }, expr.u); } template bool IsAssumedRank(const std::optional &x) { return x && IsAssumedRank(*x); @@ -100,7 +100,7 @@ template bool IsCoarray(const Designator &designator) { return false; } template bool IsCoarray(const Expr &expr) { - return common::visit([](const auto &x) { return IsCoarray(x); }, expr.u); + return std::visit([](const auto &x) { return IsCoarray(x); }, expr.u); } template bool IsCoarray(const std::optional &x) { return x && IsCoarray(*x); @@ -177,11 +177,11 @@ auto UnwrapExpr(B &x) -> common::Constify * { return UnwrapExpr(*expr); } } else if constexpr (std::is_same_v>) { - return common::visit([](auto &x) { return UnwrapExpr(x); }, x.u); + return std::visit([](auto &x) { return UnwrapExpr(x); }, x.u); } else if constexpr (!common::HasMember) { if constexpr (std::is_same_v>> || std::is_same_v::category>>>) { - return common::visit([](auto &x) { return UnwrapExpr(x); }, x.u); + return std::visit([](auto &x) { return UnwrapExpr(x); }, x.u); } } return nullptr; @@ -217,17 +217,15 @@ auto UnwrapConvertedExpr(B &x) -> common::Constify * { return UnwrapConvertedExpr(*expr); } } else if constexpr (std::is_same_v>) { - return common::visit( - [](auto &x) { return UnwrapConvertedExpr(x); }, x.u); + return std::visit([](auto &x) { return UnwrapConvertedExpr(x); }, x.u); } else if constexpr (!common::HasMember) { using Result = ResultType; if constexpr (std::is_same_v> || std::is_same_v>>) { - return common::visit( - [](auto &x) { return UnwrapConvertedExpr(x); }, x.u); + return std::visit([](auto &x) { return UnwrapConvertedExpr(x); }, x.u); } else if constexpr (std::is_same_v> || std::is_same_v>) { - return common::visit( + return std::visit( [](auto &x) { return UnwrapConvertedExpr(x); }, x.left().u); } } @@ -264,7 +262,7 @@ common::IfNoLvalue, A> ExtractDataRef( template std::optional ExtractDataRef( const Designator &d, bool intoSubstring = false) { - return common::visit( + return std::visit( [=](const auto &x) -> std::optional { if constexpr (common::HasMember) { return DataRef{x}; @@ -281,7 +279,7 @@ std::optional ExtractDataRef( template std::optional ExtractDataRef( const Expr &expr, bool intoSubstring = false) { - return common::visit( + return std::visit( [=](const auto &x) { return ExtractDataRef(x, intoSubstring); }, expr.u); } template @@ -330,7 +328,7 @@ bool IsArrayElement(const Expr &expr, bool intoSubstring = true, template std::optional ExtractNamedEntity(const A &x) { if (auto dataRef{ExtractDataRef(x, true)}) { - return common::visit( + return std::visit( common::visitors{ [](SymbolRef &&symbol) -> std::optional { return NamedEntity{symbol}; @@ -356,10 +354,10 @@ struct ExtractCoindexedObjectHelper { std::optional operator()(const CoarrayRef &x) const { return x; } template std::optional operator()(const Expr &expr) const { - return common::visit(*this, expr.u); + return std::visit(*this, expr.u); } std::optional operator()(const DataRef &dataRef) const { - return common::visit(*this, dataRef.u); + return std::visit(*this, dataRef.u); } std::optional operator()(const NamedEntity &named) const { if (const Component * component{named.UnwrapComponent()}) { @@ -451,7 +449,7 @@ Expr ConvertToType(Expr> &&x) { ConvertToType(std::move(x)), Expr{Constant{zero}}}}; } else if constexpr (FROMCAT == TypeCategory::Complex) { // Extract and convert the real component of a complex value - return common::visit( + return std::visit( [&](auto &&z) { using ZType = ResultType; using Part = typename ZType::Part; @@ -505,7 +503,7 @@ common::IfNoLvalue>, FROM> ConvertTo( template common::IfNoLvalue>, FROM> ConvertTo( const Expr> &to, FROM &&from) { - return common::visit( + return std::visit( [&](const auto &toKindExpr) { using KindExpr = std::decay_t; return AsCategoryExpr( @@ -517,7 +515,7 @@ common::IfNoLvalue>, FROM> ConvertTo( template common::IfNoLvalue, FROM> ConvertTo( const Expr &to, FROM &&from) { - return common::visit( + return std::visit( [&](const auto &toCatExpr) { return AsGenericExpr(ConvertTo(toCatExpr, std::move(from))); }, @@ -567,7 +565,7 @@ using SameKindExprs = template SameKindExprs AsSameKindExprs( Expr> &&x, Expr> &&y) { - return common::visit( + return std::visit( [&](auto &&kx, auto &&ky) -> SameKindExprs { using XTy = ResultType; using YTy = ResultType; @@ -628,7 +626,7 @@ Expr Combine(Expr &&x, Expr &&y) { template