llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Alexis Engelke (aengelke) <details> <summary>Changes</summary> Currently, formatv erases types using a base class and calls the virtual function format() to format the objects. To avoid these vtables, refactor formatv to instead store function_refs to a functor (struct with overloaded operator()). This saves ~5kiB in vtables. Additionally, add a static assertion that a formatter is present instead of relying on errors due to missing templates. This requires a little change to MLIR's tgfmt to use a different name -- previously, the substitution failure on ArrayRef<> would cause the variadic function to be skipped, but a static assertion failure is not a substitution error. Also, simplify the code in FormatVariadicDetails to use if constexpr instead of template overloads with enable_if, making the code shorter and easier to read. --- Patch is 28.75 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/207516.diff 15 Files Affected: - (modified) lldb/source/DataFormatters/FormatterBytecode.cpp (+8-8) - (modified) llvm/include/llvm/DebugInfo/CodeView/Formatters.h (+1-1) - (modified) llvm/include/llvm/DebugInfo/PDB/Native/FormatUtil.h (+1-1) - (modified) llvm/include/llvm/Support/FormatAdapters.h (+11-14) - (modified) llvm/include/llvm/Support/FormatCommon.h (+4-4) - (modified) llvm/include/llvm/Support/FormatProviders.h (+4-18) - (modified) llvm/include/llvm/Support/FormatVariadic.h (+9-11) - (modified) llvm/include/llvm/Support/FormatVariadicDetails.h (+41-117) - (modified) llvm/lib/Support/FormatVariadic.cpp (-2) - (modified) llvm/tools/llvm-xray/xray-stacks.cpp (+1-1) - (modified) llvm/unittests/ADT/TwineTest.cpp (+1-1) - (modified) llvm/unittests/Support/FormatVariadicTest.cpp (+19-13) - (modified) mlir/include/mlir/TableGen/Format.h (+11-13) - (modified) mlir/lib/TableGen/Format.cpp (+5-7) - (modified) mlir/tools/mlir-tblgen/RewriterGen.cpp (+4-4) ``````````diff diff --git a/lldb/source/DataFormatters/FormatterBytecode.cpp b/lldb/source/DataFormatters/FormatterBytecode.cpp index 1936524d37dfc..786f9198a53d0 100644 --- a/lldb/source/DataFormatters/FormatterBytecode.cpp +++ b/lldb/source/DataFormatters/FormatterBytecode.cpp @@ -107,26 +107,26 @@ static llvm::Error FormatImpl(DataStack &data) { } using namespace llvm::support::detail; auto arg = data[data.size() - num_args + r.Index]; - auto format = [&](format_adapter &&adapter) { + auto format = [&](FormatFunctorRef &&adapter) { llvm::FmtAlign Align(adapter, r.Where, r.Width, r.Pad); Align.format(os, r.Options); }; if (auto s = std::get_if<std::string>(&arg)) - format(build_format_adapter(s->c_str())); + format(FormatFunctor<const char *>(s->c_str())); else if (auto u = std::get_if<uint64_t>(&arg)) - format(build_format_adapter(u)); + format(FormatFunctor<uint64_t *&>(u)); else if (auto i = std::get_if<int64_t>(&arg)) - format(build_format_adapter(i)); + format(FormatFunctor<int64_t *&>(i)); else if (auto valobj = std::get_if<ValueObjectSP>(&arg)) { if (!valobj->get()) - format(build_format_adapter("null object")); + format(FormatFunctor<const char *>("null object")); else - format(build_format_adapter(valobj->get()->GetValueAsCString())); + format(FormatFunctor<const char *>(valobj->get()->GetValueAsCString())); } else if (auto type = std::get_if<CompilerType>(&arg)) - format(build_format_adapter(type->GetDisplayTypeName())); + format(FormatFunctor<llvm::StringRef>(type->GetDisplayTypeName())); else if (auto sel = std::get_if<FormatterBytecode::Selectors>(&arg)) - format(build_format_adapter(toString(*sel))); + format(FormatFunctor<std::string>(toString(*sel))); } data.Push(s); return llvm::Error::success(); diff --git a/llvm/include/llvm/DebugInfo/CodeView/Formatters.h b/llvm/include/llvm/DebugInfo/CodeView/Formatters.h index bf0340be901e7..dd802d1b144b0 100644 --- a/llvm/include/llvm/DebugInfo/CodeView/Formatters.h +++ b/llvm/include/llvm/DebugInfo/CodeView/Formatters.h @@ -34,7 +34,7 @@ class LLVM_ABI GuidAdapter final : public FormatAdapter<ArrayRef<uint8_t>> { explicit GuidAdapter(ArrayRef<uint8_t> Guid); explicit GuidAdapter(StringRef Guid); - void format(raw_ostream &Stream, StringRef Style) override; + void format(raw_ostream &Stream, StringRef Style); }; } // end namespace detail diff --git a/llvm/include/llvm/DebugInfo/PDB/Native/FormatUtil.h b/llvm/include/llvm/DebugInfo/PDB/Native/FormatUtil.h index 7f70d4157c358..97fb3a12f1501 100644 --- a/llvm/include/llvm/DebugInfo/PDB/Native/FormatUtil.h +++ b/llvm/include/llvm/DebugInfo/PDB/Native/FormatUtil.h @@ -72,7 +72,7 @@ struct EndianAdapter final explicit EndianAdapter(EndianType &&Item) : FormatAdapter<EndianType>(std::move(Item)) {} - void format(llvm::raw_ostream &Stream, StringRef Style) override { + void format(llvm::raw_ostream &Stream, StringRef Style) { format_provider<T>::format(static_cast<T>(this->Item), Stream, Style); } }; diff --git a/llvm/include/llvm/Support/FormatAdapters.h b/llvm/include/llvm/Support/FormatAdapters.h index 91e9c41d8a395..5a22c0bbde8f4 100644 --- a/llvm/include/llvm/Support/FormatAdapters.h +++ b/llvm/include/llvm/Support/FormatAdapters.h @@ -16,8 +16,7 @@ #include "llvm/Support/raw_ostream.h" namespace llvm { -template <typename T> -class FormatAdapter : public support::detail::format_adapter { +template <typename T> class FormatAdapter { protected: explicit FormatAdapter(T &&Item) : Item(std::forward<T>(Item)) {} @@ -36,8 +35,8 @@ template <typename T> class AlignAdapter final : public FormatAdapter<T> { : FormatAdapter<T>(std::forward<T>(Item)), Where(Where), Amount(Amount), Fill(Fill) {} - void format(llvm::raw_ostream &Stream, StringRef Style) override { - auto Adapter = detail::build_format_adapter(std::forward<T>(this->Item)); + void format(llvm::raw_ostream &Stream, StringRef Style) { + auto Adapter = detail::FormatFunctor<T>(std::forward<T>(this->Item)); FmtAlign(Adapter, Where, Amount, Fill).format(Stream, Style); } }; @@ -50,10 +49,10 @@ template <typename T> class PadAdapter final : public FormatAdapter<T> { PadAdapter(T &&Item, size_t Left, size_t Right) : FormatAdapter<T>(std::forward<T>(Item)), Left(Left), Right(Right) {} - void format(llvm::raw_ostream &Stream, StringRef Style) override { - auto Adapter = detail::build_format_adapter(std::forward<T>(this->Item)); + void format(llvm::raw_ostream &Stream, StringRef Style) { + auto Adapter = detail::FormatFunctor<T>(std::forward<T>(this->Item)); Stream.indent(Left); - Adapter.format(Stream, Style); + Adapter(Stream, Style); Stream.indent(Right); } }; @@ -65,10 +64,10 @@ template <typename T> class RepeatAdapter final : public FormatAdapter<T> { RepeatAdapter(T &&Item, size_t Count) : FormatAdapter<T>(std::forward<T>(Item)), Count(Count) {} - void format(llvm::raw_ostream &Stream, StringRef Style) override { - auto Adapter = detail::build_format_adapter(std::forward<T>(this->Item)); + void format(llvm::raw_ostream &Stream, StringRef Style) { + auto Adapter = detail::FormatFunctor<T>(std::forward<T>(this->Item)); for (size_t I = 0; I < Count; ++I) { - Adapter.format(Stream, Style); + Adapter(Stream, Style); } } }; @@ -77,10 +76,8 @@ class ErrorAdapter : public FormatAdapter<Error> { public: ErrorAdapter(Error &&Item) : FormatAdapter(std::move(Item)) {} ErrorAdapter(ErrorAdapter &&) = default; - ~ErrorAdapter() override { consumeError(std::move(Item)); } - void format(llvm::raw_ostream &Stream, StringRef Style) override { - Stream << Item; - } + ~ErrorAdapter() { consumeError(std::move(Item)); } + void format(llvm::raw_ostream &Stream, StringRef Style) { Stream << Item; } }; } // namespace detail } // namespace support diff --git a/llvm/include/llvm/Support/FormatCommon.h b/llvm/include/llvm/Support/FormatCommon.h index eaf291b864c5e..112cf1fd68478 100644 --- a/llvm/include/llvm/Support/FormatCommon.h +++ b/llvm/include/llvm/Support/FormatCommon.h @@ -19,12 +19,12 @@ enum class AlignStyle { Left, Center, Right }; /// Helper class to format to a \p Width wide field, with alignment \p Where /// within that field. struct FmtAlign { - support::detail::format_adapter &Adapter; + support::detail::FormatFunctorRef Adapter; AlignStyle Where; unsigned Width; char Fill; - FmtAlign(support::detail::format_adapter &Adapter, AlignStyle Where, + FmtAlign(support::detail::FormatFunctorRef Adapter, AlignStyle Where, unsigned Width, char Fill = ' ') : Adapter(Adapter), Where(Where), Width(Width), Fill(Fill) {} @@ -35,13 +35,13 @@ struct FmtAlign { // TODO: Make the format method return the number of bytes written, that // way we can also skip the intermediate stream for left-aligned output. if (Width == 0) { - Adapter.format(S, Options); + Adapter(S, Options); return; } SmallString<64> Item; raw_svector_ostream Stream(Item); - Adapter.format(Stream, Options); + Adapter(Stream, Options); if (Width <= Item.size()) { S << Item; return; diff --git a/llvm/include/llvm/Support/FormatProviders.h b/llvm/include/llvm/Support/FormatProviders.h index 78eeec76cf79b..38a203482d594 100644 --- a/llvm/include/llvm/Support/FormatProviders.h +++ b/llvm/include/llvm/Support/FormatProviders.h @@ -325,18 +325,6 @@ struct format_provider< } }; -namespace support { -namespace detail { -template <typename IterT> -using IterValue = typename std::iterator_traits<IterT>::value_type; - -template <typename IterT> -struct range_item_has_provider - : public std::bool_constant< - !support::detail::uses_missing_provider<IterValue<IterT>>::value> {}; -} // namespace detail -} // namespace support - /// Implementation of format_provider<T> for ranges. /// /// This will print an arbitrary range as a delimited sequence of items. @@ -399,8 +387,6 @@ template <typename IterT> class format_provider<llvm::iterator_range<IterT>> { } public: - static_assert(support::detail::range_item_has_provider<IterT>::value, - "Range value_type does not have a format provider!"); static void format(const llvm::iterator_range<IterT> &V, llvm::raw_ostream &Stream, StringRef Style) { StringRef Sep; @@ -409,14 +395,14 @@ template <typename IterT> class format_provider<llvm::iterator_range<IterT>> { auto Begin = V.begin(); auto End = V.end(); if (Begin != End) { - auto Adapter = support::detail::build_format_adapter(*Begin); - Adapter.format(Stream, ArgStyle); + auto Adapter = support::detail::FormatFunctor<decltype(*Begin)>(*Begin); + Adapter(Stream, ArgStyle); ++Begin; } while (Begin != End) { Stream << Sep; - auto Adapter = support::detail::build_format_adapter(*Begin); - Adapter.format(Stream, ArgStyle); + auto Adapter = support::detail::FormatFunctor<decltype(*Begin)>(*Begin); + Adapter(Stream, ArgStyle); ++Begin; } } diff --git a/llvm/include/llvm/Support/FormatVariadic.h b/llvm/include/llvm/Support/FormatVariadic.h index 145fbb47c91b4..322e6e9d8105a 100644 --- a/llvm/include/llvm/Support/FormatVariadic.h +++ b/llvm/include/llvm/Support/FormatVariadic.h @@ -65,11 +65,11 @@ struct ReplacementItem { class formatv_object_base { protected: StringRef Fmt; - ArrayRef<support::detail::format_adapter *> Adapters; + ArrayRef<support::detail::FormatFunctorRef> Adapters; bool Validate; formatv_object_base(StringRef Fmt, - ArrayRef<support::detail::format_adapter *> Adapters, + ArrayRef<support::detail::FormatFunctorRef> Adapters, bool Validate) : Fmt(Fmt), Adapters(Adapters), Validate(Validate) {} @@ -89,9 +89,9 @@ class formatv_object_base { continue; } - auto *W = Adapters[R.Index]; + auto &W = Adapters[R.Index]; - FmtAlign Align(*W, R.Where, R.Width, R.Pad); + FmtAlign Align(W, R.Where, R.Width, R.Pad); Align.format(S, R.Options); } } @@ -122,21 +122,19 @@ template <typename Tuple> class formatv_object : public formatv_object_base { // of the parameters, we have to own the storage for the parameters here, and // have the base class store type-erased pointers into this tuple. Tuple Parameters; - std::array<support::detail::format_adapter *, std::tuple_size<Tuple>::value> + std::array<support::detail::FormatFunctorRef, std::tuple_size<Tuple>::value> ParameterPointers; // The parameters are stored in a std::tuple, which does not provide runtime // indexing capabilities. In order to enable runtime indexing, we use this // structure to put the parameters into a std::array. Since the parameters // are not all the same type, we use some type-erasure by wrapping the - // parameters in a template class that derives from a non-template superclass. - // Essentially, we are converting a std::tuple<Derived<Ts...>> to a - // std::array<Base*>. + // parameters in a template class and refer to them using function_refs. struct create_adapters { template <typename... Ts> - std::array<support::detail::format_adapter *, std::tuple_size<Tuple>::value> + std::array<support::detail::FormatFunctorRef, std::tuple_size<Tuple>::value> operator()(Ts &...Items) { - return {{&Items...}}; + return {{Items...}}; } }; @@ -248,7 +246,7 @@ template <typename Tuple> class formatv_object : public formatv_object_base { template <typename... Ts> inline auto formatv(bool Validate, const char *Fmt, Ts &&...Vals) { auto Params = std::make_tuple( - support::detail::build_format_adapter(std::forward<Ts>(Vals))...); + support::detail::FormatFunctor<Ts>(std::forward<Ts>(Vals))...); return formatv_object<decltype(Params)>(Fmt, std::move(Params), Validate); } diff --git a/llvm/include/llvm/Support/FormatVariadicDetails.h b/llvm/include/llvm/Support/FormatVariadicDetails.h index c0b245e297a58..9fab9b84a3757 100644 --- a/llvm/include/llvm/Support/FormatVariadicDetails.h +++ b/llvm/include/llvm/Support/FormatVariadicDetails.h @@ -10,6 +10,7 @@ #define LLVM_SUPPORT_FORMATVARIADICDETAILS_H #include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/STLForwardCompat.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Compiler.h" #include "llvm/Support/raw_ostream.h" @@ -22,134 +23,57 @@ class Error; namespace support { namespace detail { -class LLVM_ABI format_adapter { - virtual void anchor(); -protected: - virtual ~format_adapter() = default; +using FormatFunctorRef = function_ref<void(llvm::raw_ostream &, StringRef)>; -public: - virtual void format(raw_ostream &S, StringRef Options) = 0; -}; - -template <typename T> class provider_format_adapter : public format_adapter { - T Item; - -public: - explicit provider_format_adapter(T &&Item) : Item(std::forward<T>(Item)) {} - - void format(llvm::raw_ostream &S, StringRef Options) override { - format_provider<std::decay_t<T>>::format(Item, S, Options); - } -}; +template <typename T> class FormatFunctor { + // If the caller passed an Error by value, then stream_operator_format_adapter + // would be responsible for consuming it. + // Make the caller opt into this by calling fmt_consume(). + static_assert( + !std::is_same_v<llvm::Error, std::remove_cv_t<T>>, + "llvm::Error-by-value must be wrapped in fmt_consume() for formatv"); -template <typename T> -class stream_operator_format_adapter : public format_adapter { T Item; -public: - explicit stream_operator_format_adapter(T &&Item) - : Item(std::forward<T>(Item)) {} - - void format(llvm::raw_ostream &S, StringRef) override { S << Item; } -}; - -template <typename T> class missing_format_adapter; - -// Test if format_provider<T> is defined on T and contains a member function -// with the signature: -// static void format(const T&, raw_stream &, StringRef); -// -template <class T> class has_FormatProvider { -public: - using Decayed = std::decay_t<T>; - using Signature_format = void (*)(const Decayed &, llvm::raw_ostream &, + using DecayedT = std::decay_t<T>; + using Signature_format = void (*)(const DecayedT &, llvm::raw_ostream &, StringRef); - template <typename U> using check = SameType<Signature_format, &U::format>; - - static constexpr bool value = - llvm::is_detected<check, llvm::format_provider<Decayed>>::value; -}; - -// Test if raw_ostream& << T -> raw_ostream& is findable via ADL. -template <class T> class has_StreamOperator { -public: - using ConstRefT = const std::decay_t<T> &; - template <typename U> - static auto test(int) - -> std::is_same<decltype(std::declval<llvm::raw_ostream &>() - << std::declval<U>()), - llvm::raw_ostream &>; - - template <typename U> static auto test(...) -> std::false_type; + using MemberFormatCheck = decltype(std::declval<U>().format( + std::declval<llvm::raw_ostream &>(), std::declval<llvm::StringRef>())); + template <typename U> + using StaticFormatCheck = SameType<Signature_format, &U::format>; + template <typename U> + using StreamCheck = std::is_same<decltype(std::declval<llvm::raw_ostream &>() + << std::declval<U>()), + llvm::raw_ostream &>; - static constexpr bool value = decltype(test<ConstRefT>(0))::value; +public: + static constexpr bool HasMemberProvider = + llvm::is_detected<MemberFormatCheck, DecayedT>::value; + static constexpr bool HasFormatProvider = + llvm::is_detected<StaticFormatCheck, + llvm::format_provider<DecayedT>>::value; + static constexpr bool HasStreamProvider = + llvm::is_detected<StreamCheck, DecayedT>::value; + + static_assert(HasMemberProvider || HasFormatProvider || HasStreamProvider, + "type has no format provider"); + + explicit FormatFunctor(T &&Item) : Item(std::forward<T>(Item)) {} + + void operator()(llvm::raw_ostream &S, StringRef Options) { + if constexpr (HasMemberProvider) + Item.format(S, Options); + else if constexpr (HasFormatProvider) + format_provider<DecayedT>::format(Item, S, Options); + else if constexpr (HasStreamProvider) + S << Item; + } }; -// Simple template that decides whether a type T should use the member-function -// based format() invocation. -template <typename T> -struct uses_format_member - : public std::is_base_of<format_adapter, std::remove_reference_t<T>> {}; - -// Simple template that decides whether a type T should use the format_provider -// based format() invocation. The member function takes priority, so this test -// will only be true if there is not ALSO a format member. -template <typename T> -struct uses_format_provider - : public std::bool_constant<!uses_format_member<T>::value && - has_FormatProvider<T>::value> {}; - -// Simple template that decides whether a type T should use the operator<< -// based format() invocation. This takes last priority. -template <typename T> -struct uses_stream_operator - : public std::bool_constant<!uses_format_member<T>::value && - !uses_format_provider<T>::value && - has_StreamOperator<T>::value> {}; - -// Simple template that decides whether a type T has neither a member-function -// nor format_provider based implementation that it can use. Mostly used so -// that the compiler spits out a nice diagnostic when a type with no format -// implementation can be located. -template <typename T> -struct uses_missing_provider - : public std::bool_constant<!uses_format_member<T>::value && - !uses_format_provider<T>::value && - !uses_stream_operator<T>::value> {}; - -template <typename T> -std::enable_if_t<uses_format_member<T>::value, T> -build_format_adapter(T &&Item) { - return std::forward<T>(Item); -} - -template <typename T> -std::enable_if_t<uses_format_provider<T>::value, provider_format_adapter<T>> -build_format_adapter(T &&Item) { - return provider_format_adapter<T>(std::forward<T>(Item)); -} - -template <typename T> -std::enable_if_t<uses_stream_operator<T>::value, - stream_operator_format_adapter<T>> -build_format_adapter(T &&Item) { - // If the caller passed an Error by value, then stream_operator_format_adapter - // would be responsible for consuming it. - // Make the caller opt into this by calling fmt_consume(). - static_assert( - !std::is_same_v<llvm::Error, std::remove_cv_t<T>>, - "llvm::Error-by-value must be wrapped in fmt_consume() for formatv"); - return stream_operator_format_adapter<T>(std::forward<T>(Item)); -} - -template <typename T> -std::enable_if_t<uses_missing_provider<T>::value, missing_format_adapter<T>> -build_format_adapter(T &&) { - return missing_format_adapter<T>(); -} } // namespace detail } // namespace support } // namespace llvm diff --git a/llvm/lib/Support/FormatVariadic.cpp b/llvm/lib/Support/FormatVariadic.cpp index f3e8d0a7fe6f3..a853bd357aac3 100644 --- a/llvm/lib/Support/FormatVariadic.cpp +++ b/llvm/lib/Support/FormatVariadic.cpp @@ -222,5 +222,3 @@ formatv_object_base::parseFormatString(StringRef Fmt, size_t NumArgs, #endif // ENABLE_VALIDATION return Replacements; } - -void support::detail::format_adapter::anchor() {} diff --git a/llvm/tools/llvm-xray/xray-stacks.cpp b/llvm/tools/llvm-xray/xray-stacks.cpp index 8101cad28ca1c..28e567cf81c87 100644 --- a/llvm/tools/llvm-xray/xray-stacks.cpp +++ b/llvm/tools/llvm-xray/xray-stacks.cpp @@ -115,7 +115,7 @@ struct format_xray_record : public FormatAdapter<XRayRecord> { explicit format_xray_record(XRayRecord record, const FuncIdConversionHelper &conv) : FormatAdapter<XRayRecord>(std::move(record)), Converter(&conv) {} - void format(raw_ostream &Stream, StringRef Style) override { + void format(raw_ostream &Stream, StringRef Style) { Stream << formatv( "{FuncId: \"{0}\", ThreadId: \"{1}\"... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/207516 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
