[llvm] r238305 - [PM] Use a simpler technique to drop optional analysis manager arguments

Chandler Carruth chandlerc at gmail.com
Wed May 27 02:42:00 PDT 2015


FYI Dinesh, this didn't work. =[

One example failure from the MSVC bots:
http://lab.llvm.org:8011/builders/lld-x86_64-win7/builds/21015/steps/build_Lld/logs/stdio

Lots of others looked like that. No idea why it can't handle it. I've
backed out the commit to get the bots green, but I'd love to understand why
they rejected this code.

On Wed, May 27, 2015 at 2:10 AM Chandler Carruth <chandlerc at gmail.com>
wrote:

> Author: chandlerc
> Date: Wed May 27 04:02:51 2015
> New Revision: 238305
>
> URL: http://llvm.org/viewvc/llvm-project?rev=238305&view=rev
> Log:
> [PM] Use a simpler technique to drop optional analysis manager arguments
> when invoking run methods.
>
> This technique was suggested by Dinesh Dwivedi who also wrote the
> original patch. During the code review, they explained to me that this
> isn't a fully general technique as we need to know the signatures of the
> method candidates. Since this is really a narrower utility, I switched
> the names and structure to be more clearly a specialized run method
> invoke helper and commented it accordingly. I still think this is
> a pretty big win.
>
> Very sorry to Dinesh for the extreme delay in landing this patch. I've
> been far to busy poking at other things.
>
> Original review: http://reviews.llvm.org/D3543
>
> Modified:
>     llvm/trunk/include/llvm/IR/PassManagerInternal.h
>
> Modified: llvm/trunk/include/llvm/IR/PassManagerInternal.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/PassManagerInternal.h?rev=238305&r1=238304&r2=238305&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/IR/PassManagerInternal.h (original)
> +++ llvm/trunk/include/llvm/IR/PassManagerInternal.h Wed May 27 04:02:51
> 2015
> @@ -29,41 +29,56 @@ class PreservedAnalyses;
>  /// \brief Implementation details of the pass manager interfaces.
>  namespace detail {
>
> +/// \brief Helper template to run a pass *with* an optional analysis
> manager.
> +///
> +/// This accepts the address of the run method as an argument to detect
> that it
> +/// accepts the analysis manager and pass it through. This isn't (quite)
> +/// a generic optional argument invocation tool, and couldn't be without
> making
> +/// the calling syntax significantly more verbose.
> +template <typename ResultT, typename IRUnitT, typename PassT>
> +ResultT invokeRunMethod(PassT &Pass,
> +                        ResultT (PassT::*run)(IRUnitT &,
> +                                              AnalysisManager<IRUnitT> *),
> +                        IRUnitT &IR, AnalysisManager<IRUnitT> *AM) {
> +  return (Pass.*run)(IR, AM);
> +}
> +
> +/// \brief Helper template to run a pass *without* an optional analysis
> manager.
> +///
> +/// This accepts the address of the run method as an argument to detect
> that it
> +/// does not accept an analysis manager and drop it. This isn't (quite)
> +/// a generic optional argument invocation tool, and couldn't be without
> making
> +/// the calling syntax significantly more verbose.
> +template <typename ResultT, typename IRUnitT, typename PassT>
> +ResultT invokeRunMethod(
> +    PassT &Pass,
> +    ResultT (PassT::*run)(IRUnitT &),
> +    IRUnitT &IR, AnalysisManager<IRUnitT> * /*AM*/) {
> +  return (Pass.*run)(IR);
> +}
> +
>  /// \brief Template for the abstract base class used to dispatch
>  /// polymorphically over pass objects.
> -template <typename IRUnitT> struct PassConcept {
> -  // Boiler plate necessary for the container of derived classes.
> -  virtual ~PassConcept() {}
> +template <typename ResultT, typename IRUnitT> struct PassConceptBase {
> +  virtual ~PassConceptBase() {}
>
>    /// \brief The polymorphic API which runs the pass over a given IR
> entity.
>    ///
>    /// Note that actual pass object can omit the analysis manager argument
> if
>    /// desired. Also that the analysis manager may be null if there is no
>    /// analysis manager in the pass pipeline.
> -  virtual PreservedAnalyses run(IRUnitT &IR, AnalysisManager<IRUnitT>
> *AM) = 0;
> +  virtual ResultT run(IRUnitT &IR, AnalysisManager<IRUnitT> *AM) = 0;
>
>    /// \brief Polymorphic method to access the name of a pass.
>    virtual StringRef name() = 0;
>  };
>
> -/// \brief SFINAE metafunction for computing whether \c PassT has a run
> method
> -/// accepting an \c AnalysisManager<IRUnitT>.
> -template <typename IRUnitT, typename PassT, typename ResultT>
> -class PassRunAcceptsAnalysisManager {
> -  typedef char SmallType;
> -  struct BigType {
> -    char a, b;
> -  };
> -
> -  template <typename T, ResultT (T::*)(IRUnitT &,
> AnalysisManager<IRUnitT> *)>
> -  struct Checker;
> -
> -  template <typename T> static SmallType f(Checker<T, &T::run> *);
> -  template <typename T> static BigType f(...);
> -
> -public:
> -  enum { Value = sizeof(f<PassT>(nullptr)) == sizeof(SmallType) };
> -};
> +/// \brief Abstract concept of an transform pass.
> +///
> +/// This concept is parameterized over the IR unit that it can run over
> and
> +/// returns status of preserved analyses.
> +template <typename IRUnitT>
> +using PassConcept = PassConceptBase<PreservedAnalyses, IRUnitT>;
>
>  /// \brief A template wrapper used to implement the polymorphic API.
>  ///
> @@ -72,42 +87,8 @@ public:
>  /// \c run method also accepts an \c AnalysisManager<IRUnitT>*, we pass it
>  /// along.
>  template <typename IRUnitT, typename PassT,
> -          typename PreservedAnalysesT = PreservedAnalyses,
> -          bool AcceptsAnalysisManager = PassRunAcceptsAnalysisManager<
> -              IRUnitT, PassT, PreservedAnalysesT>::Value>
> -struct PassModel;
> -
> -/// \brief Specialization of \c PassModel for passes that accept an
> analyis
> -/// manager.
> -template <typename IRUnitT, typename PassT, typename PreservedAnalysesT>
> -struct PassModel<IRUnitT, PassT, PreservedAnalysesT, true>
> -    : PassConcept<IRUnitT> {
> -  explicit PassModel(PassT Pass) : Pass(std::move(Pass)) {}
> -  // We have to explicitly define all the special member functions
> because MSVC
> -  // refuses to generate them.
> -  PassModel(const PassModel &Arg) : Pass(Arg.Pass) {}
> -  PassModel(PassModel &&Arg) : Pass(std::move(Arg.Pass)) {}
> -  friend void swap(PassModel &LHS, PassModel &RHS) {
> -    using std::swap;
> -    swap(LHS.Pass, RHS.Pass);
> -  }
> -  PassModel &operator=(PassModel RHS) {
> -    swap(*this, RHS);
> -    return *this;
> -  }
> -
> -  PreservedAnalysesT run(IRUnitT &IR, AnalysisManager<IRUnitT> *AM)
> override {
> -    return Pass.run(IR, AM);
> -  }
> -  StringRef name() override { return PassT::name(); }
> -  PassT Pass;
> -};
> -
> -/// \brief Specialization of \c PassModel for passes that accept an
> analyis
> -/// manager.
> -template <typename IRUnitT, typename PassT, typename PreservedAnalysesT>
> -struct PassModel<IRUnitT, PassT, PreservedAnalysesT, false>
> -    : PassConcept<IRUnitT> {
> +          typename PreservedAnalysesT = PreservedAnalyses>
> +struct PassModel : PassConcept<IRUnitT> {
>    explicit PassModel(PassT Pass) : Pass(std::move(Pass)) {}
>    // We have to explicitly define all the special member functions
> because MSVC
>    // refuses to generate them.
> @@ -122,8 +103,9 @@ struct PassModel<IRUnitT, PassT, Preserv
>      return *this;
>    }
>
> +  /// \brief The model delegates to the \c PassT::run method.
>    PreservedAnalysesT run(IRUnitT &IR, AnalysisManager<IRUnitT> *AM)
> override {
> -    return Pass.run(IR);
> +    return invokeRunMethod<PreservedAnalysesT>(Pass, &PassT::run, IR, AM);
>    }
>    StringRef name() override { return PassT::name(); }
>    PassT Pass;
> @@ -245,33 +227,17 @@ struct AnalysisResultModel<IRUnitT, Pass
>  ///
>  /// This concept is parameterized over the IR unit that it can run over
> and
>  /// produce an analysis result.
> -template <typename IRUnitT> struct AnalysisPassConcept {
> -  virtual ~AnalysisPassConcept() {}
> -
> -  /// \brief Method to run this analysis over a unit of IR.
> -  /// \returns A unique_ptr to the analysis result object to be queried by
> -  /// users.
> -  virtual std::unique_ptr<AnalysisResultConcept<IRUnitT>>
> -  run(IRUnitT &IR, AnalysisManager<IRUnitT> *AM) = 0;
> -
> -  /// \brief Polymorphic method to access the name of a pass.
> -  virtual StringRef name() = 0;
> -};
> +template <typename IRUnitT>
> +using AnalysisPassConcept =
> +    PassConceptBase<std::unique_ptr<AnalysisResultConcept<IRUnitT>>,
> IRUnitT>;
>
>  /// \brief Wrapper to model the analysis pass concept.
>  ///
>  /// Can wrap any type which implements a suitable \c run method. The
> method
>  /// must accept the IRUnitT as an argument and produce an object which
> can be
>  /// wrapped in a \c AnalysisResultModel.
> -template <typename IRUnitT, typename PassT,
> -          bool AcceptsAnalysisManager = PassRunAcceptsAnalysisManager<
> -              IRUnitT, PassT, typename PassT::Result>::Value>
> -struct AnalysisPassModel;
> -
> -/// \brief Specialization of \c AnalysisPassModel which passes an
> -/// \c AnalysisManager to PassT's run method.
>  template <typename IRUnitT, typename PassT>
> -struct AnalysisPassModel<IRUnitT, PassT, true> :
> AnalysisPassConcept<IRUnitT> {
> +struct AnalysisPassModel : AnalysisPassConcept<IRUnitT> {
>    explicit AnalysisPassModel(PassT Pass) : Pass(std::move(Pass)) {}
>    // We have to explicitly define all the special member functions
> because MSVC
>    // refuses to generate them.
> @@ -287,53 +253,16 @@ struct AnalysisPassModel<IRUnitT, PassT,
>    }
>
>    // FIXME: Replace PassT::Result with type traits when we use C++11.
> -  typedef AnalysisResultModel<IRUnitT, PassT, typename PassT::Result>
> -      ResultModelT;
> +  using ResultT = typename PassT::Result;
> +  using ResultModelT = AnalysisResultModel<IRUnitT, PassT, ResultT>;
>
>    /// \brief The model delegates to the \c PassT::run method.
>    ///
>    /// The return is wrapped in an \c AnalysisResultModel.
>    std::unique_ptr<AnalysisResultConcept<IRUnitT>>
>    run(IRUnitT &IR, AnalysisManager<IRUnitT> *AM) override {
> -    return make_unique<ResultModelT>(Pass.run(IR, AM));
> -  }
> -
> -  /// \brief The model delegates to a static \c PassT::name method.
> -  ///
> -  /// The returned string ref must point to constant immutable data!
> -  StringRef name() override { return PassT::name(); }
> -
> -  PassT Pass;
> -};
> -
> -/// \brief Specialization of \c AnalysisPassModel which does not pass an
> -/// \c AnalysisManager to PassT's run method.
> -template <typename IRUnitT, typename PassT>
> -struct AnalysisPassModel<IRUnitT, PassT, false> :
> AnalysisPassConcept<IRUnitT> {
> -  explicit AnalysisPassModel(PassT Pass) : Pass(std::move(Pass)) {}
> -  // We have to explicitly define all the special member functions
> because MSVC
> -  // refuses to generate them.
> -  AnalysisPassModel(const AnalysisPassModel &Arg) : Pass(Arg.Pass) {}
> -  AnalysisPassModel(AnalysisPassModel &&Arg) : Pass(std::move(Arg.Pass))
> {}
> -  friend void swap(AnalysisPassModel &LHS, AnalysisPassModel &RHS) {
> -    using std::swap;
> -    swap(LHS.Pass, RHS.Pass);
> -  }
> -  AnalysisPassModel &operator=(AnalysisPassModel RHS) {
> -    swap(*this, RHS);
> -    return *this;
> -  }
> -
> -  // FIXME: Replace PassT::Result with type traits when we use C++11.
> -  typedef AnalysisResultModel<IRUnitT, PassT, typename PassT::Result>
> -      ResultModelT;
> -
> -  /// \brief The model delegates to the \c PassT::run method.
> -  ///
> -  /// The return is wrapped in an \c AnalysisResultModel.
> -  std::unique_ptr<AnalysisResultConcept<IRUnitT>>
> -  run(IRUnitT &IR, AnalysisManager<IRUnitT> *) override {
> -    return make_unique<ResultModelT>(Pass.run(IR));
> +    return make_unique<ResultModelT>(
> +        invokeRunMethod<ResultT>(Pass, &PassT::run, IR, AM));
>    }
>
>    /// \brief The model delegates to a static \c PassT::name method.
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150527/4fef4cf2/attachment.html>


More information about the llvm-commits mailing list