[llvm] r257316 - [ORC] Move ORC RPC helper classes that rely on partial specialization into a

Lang Hames via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 15 11:41:12 PST 2016


I'm totally fine with a void cast as a workaround. I've added one in
r257927.

- Lang.

On Fri, Jan 15, 2016 at 11:14 AM, Aaron Ballman <aaron at aaronballman.com>
wrote:

> On Fri, Jan 15, 2016 at 2:07 PM, David Blaikie <dblaikie at gmail.com> wrote:
> >
> >
> > On Fri, Jan 15, 2016 at 8:36 AM, Aaron Ballman via llvm-commits
> > <llvm-commits at lists.llvm.org> wrote:
> >>
> >> On Mon, Jan 11, 2016 at 12:44 AM, Lang Hames via llvm-commits
> >> <llvm-commits at lists.llvm.org> wrote:
> >> > Author: lhames
> >> > Date: Sun Jan 10 23:44:39 2016
> >> > New Revision: 257316
> >> >
> >> > URL: http://llvm.org/viewvc/llvm-project?rev=257316&view=rev
> >> > Log:
> >> > [ORC] Move ORC RPC helper classes that rely on partial specialization
> >> > into a
> >> > non-template base class.
> >> >
> >> > Hopefully this should fix the issues with the windows bots arrising
> from
> >> > r257305.
> >> >
> >> > Modified:
> >> >     llvm/trunk/include/llvm/ExecutionEngine/Orc/RPCUtils.h
> >> >
> >> > Modified: llvm/trunk/include/llvm/ExecutionEngine/Orc/RPCUtils.h
> >> > URL:
> >> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/Orc/RPCUtils.h?rev=257316&r1=257315&r2=257316&view=diff
> >> >
> >> >
> ==============================================================================
> >> > --- llvm/trunk/include/llvm/ExecutionEngine/Orc/RPCUtils.h (original)
> >> > +++ llvm/trunk/include/llvm/ExecutionEngine/Orc/RPCUtils.h Sun Jan 10
> >> > 23:44:39 2016
> >> > @@ -20,6 +20,76 @@ namespace llvm {
> >> >  namespace orc {
> >> >  namespace remote {
> >> >
> >> > +// Base class containing utilities that require partial
> specialization.
> >> > +// These cannot be included in RPC, as template class members cannot
> be
> >> > +// partially specialized.
> >> > +class RPCBase {
> >> > +protected:
> >> > +  template <typename ProcedureIdT, ProcedureIdT ProcId, typename...
> Ts>
> >> > +  class ProcedureHelper {
> >> > +  public:
> >> > +    static const ProcedureIdT Id = ProcId;
> >> > +  };
> >> > +
> >> > +  template <typename ChannelT, typename Proc> class CallHelper;
> >> > +
> >> > +  template <typename ChannelT, typename ProcedureIdT, ProcedureIdT
> >> > ProcId,
> >> > +            typename... ArgTs>
> >> > +  class CallHelper<ChannelT, ProcedureHelper<ProcedureIdT, ProcId,
> >> > ArgTs...>> {
> >> > +  public:
> >> > +    static std::error_code call(ChannelT &C, const ArgTs &... Args) {
> >> > +      if (auto EC = serialize(C, ProcId))
> >> > +        return EC;
> >> > +      // If you see a compile-error on this line you're probably
> >> > calling a
> >> > +      // function with the wrong signature.
> >> > +      return serialize_seq(C, Args...);
> >> > +    }
> >> > +  };
> >> > +
> >> > +  template <typename ChannelT, typename Proc> class HandlerHelper;
> >> > +
> >> > +  template <typename ChannelT, typename ProcedureIdT, ProcedureIdT
> >> > ProcId,
> >> > +            typename... ArgTs>
> >> > +  class HandlerHelper<ChannelT,
> >> > +                      ProcedureHelper<ProcedureIdT, ProcId,
> ArgTs...>>
> >> > {
> >> > +  public:
> >> > +    template <typename HandlerT>
> >> > +    static std::error_code handle(ChannelT &C, HandlerT Handler) {
> >> > +      return readAndHandle(C, Handler,
> >> > llvm::index_sequence_for<ArgTs...>());
> >> > +    }
> >> > +
> >> > +  private:
> >> > +    template <typename HandlerT, size_t... Is>
> >> > +    static std::error_code readAndHandle(ChannelT &C, HandlerT
> Handler,
> >> > +                                         llvm::index_sequence<Is...>
> _)
> >> > {
> >> > +      std::tuple<ArgTs...> RPCArgs;
> >> > +      if (auto EC = deserialize_seq(C, std::get<Is>(RPCArgs)...))
> >> > +        return EC;
> >> > +      return Handler(std::get<Is>(RPCArgs)...);
> >>
> >> This is causing several instances of the following diagnostic:
> >>
> >>
> >>
> /opt/llvm/build-llvm/src/llvm/include/llvm/ExecutionEngine/Orc/RPCUtils.h:65:28:
> >> warning: variable ‘RPCArgs’ set but not used
> >> [-Wunused-but-set-variable]
> >>
> /opt/llvm/build-llvm/src/llvm/include/llvm/ExecutionEngine/Orc/RPCUtils.h:
> >> In instantiation of ‘static std::error_code
> >> llvm::orc::remote::RPCBase::HandlerHelper<ChannelT,
> >> llvm::orc::remote::RPCBase::ProcedureHelper<ProcedureIdT, ProcId,
> >> ArgTs ...> >::readAndHandle(ChannelT&, HandlerT,
> >> llvm::index_sequence<Is ...>) [with HandlerT =
> >>
> >>
> llvm::orc::remote::RPCBase::MemberFnWrapper<llvm::orc::remote::OrcRemoteTargetServer<FDRPCChannel,
> >> llvm::orc::OrcX86_64> >; long unsigned int ...Is = {}; ChannelT =
> >> llvm::orc::remote::RPCChannel; ProcedureIdT = unsigned int;
> >> ProcedureIdT ProcId = 15u; ArgTs = {}]’:
> >>
> >>
> /opt/llvm/build-llvm/src/llvm/include/llvm/ExecutionEngine/Orc/RPCUtils.h:58:76:
> >>   required from ‘static std::error_code
> >> llvm::orc::remote::RPCBase::HandlerHelper<ChannelT,
> >> llvm::orc::remote::RPCBase::ProcedureHelper<ProcedureIdT, ProcId,
> >> ArgTs ...> >::handle(ChannelT&, HandlerT) [with HandlerT =
> >>
> >>
> llvm::orc::remote::RPCBase::MemberFnWrapper<llvm::orc::remote::OrcRemoteTargetServer<FDRPCChannel,
> >> llvm::orc::OrcX86_64> >; ChannelT = llvm::orc::remote::RPCChannel;
> >> ProcedureIdT = unsigned int; ProcedureIdT ProcId = 15u; ArgTs = {}]’
> >>
> >>
> /opt/llvm/build-llvm/src/llvm/include/llvm/ExecutionEngine/Orc/RPCUtils.h:207:60:
> >>   required from ‘static std::error_code
> >> llvm::orc::remote::RPC<ChannelT, ProcedureIdT>::handle(ChannelT&,
> >> HandlerT) [with Proc =
> >> llvm::orc::remote::RPCBase::ProcedureHelper<unsigned int, 15u>;
> >> HandlerT =
> >>
> llvm::orc::remote::RPCBase::MemberFnWrapper<llvm::orc::remote::OrcRemoteTargetServer<FDRPCChannel,
> >> llvm::orc::OrcX86_64> >; ChannelT = llvm::orc::remote::RPCChannel;
> >> ProcedureIdT = unsigned int]’
> >>
> >>
> /opt/llvm/build-llvm/src/llvm/include/llvm/ExecutionEngine/Orc/RPCUtils.h:216:70:
> >>   required from ‘static std::error_code
> >> llvm::orc::remote::RPC<ChannelT, ProcedureIdT>::handle(ChannelT&,
> >> ClassT&, std::error_code (ClassT::*)(ArgTs ...)) [with Proc =
> >> llvm::orc::remote::RPCBase::ProcedureHelper<unsigned int, 15u>; ClassT
> >> = llvm::orc::remote::OrcRemoteTargetServer<FDRPCChannel,
> >> llvm::orc::OrcX86_64>; ArgTs = {}; ChannelT =
> >> llvm::orc::remote::RPCChannel; ProcedureIdT = unsigned int]’
> >>
> >>
> /opt/llvm/build-llvm/src/llvm/include/llvm/ExecutionEngine/Orc/OrcRemoteTargetServer.h:87:75:
> >>   required from ‘std::error_code
> >> llvm::orc::remote::OrcRemoteTargetServer<ChannelT,
> >>
> >>
> TargetT>::handleKnownProcedure(llvm::orc::remote::OrcRemoteTargetRPCAPI::JITProcId)
> >> [with ChannelT = FDRPCChannel; TargetT = llvm::orc::OrcX86_64]’
> >>
> /opt/llvm/build-llvm/src/llvm/tools/lli/ChildTarget/ChildTarget.cpp:66:51:
> >>   required from here
> >>
> >> I'm not comfortable trying to address this myself, but if you could
> >> fix it, I would appreciate it.
> >
> >
> > My best guess is that this is a GCC diagnostic bug. It is warning about
> the
> > local variable being unused when its use in a parameter pack expansion
> has
> > no elements.
> >
> > We could add a (void) cast to workaround it, but I'm not sure how high
> of a
> > priority warning cleanliness with GCC 4.7 is. Actually, this reproduces
> in
> > 4.8 too - not sure beyond that (don't have 4.9/5.0 on hand).
>
> I generally keep this bot warning-free, and I would like to keep it
> that way (IIRC, this bot has found some bugs, but I don't recall
> whether other bots would have found the same bugs or not). I don't
> think that it's a good idea to turn off this warning since it's
> generally useful, even if false positive in this case, and so I'd
> personally prefer to add the (void) cast to work around it if
> possible.
>
> ~Aaron
>
> >
> > Yep, here's a reproduction of the GCC bug:
> >
> > blaikie at blaikie-linux:/tmp/dbginfo$ g++-4.7 expand.cpp -std=c++11
> > -Wunused-but-set-variable
> > blaikie at blaikie-linux:/tmp/dbginfo$ g++-4.7 expand.cpp -std=c++11
> > -Wunused-but-set-variable -DFAIL
> > expand.cpp: In instantiation of ‘static void foo<Args>::func(Args ...)
> [with
> > long unsigned int ...Is = {}; Args = {}]’:
> > expand.cpp:17:17:   required from here
> > expand.cpp:9:23: warning: variable ‘t’ set but not used
> > [-Wunused-but-set-variable]
> > blaikie at blaikie-linux:/tmp/dbginfo$ cat expand.cpp
> > #include <cstdlib>
> > #include <tuple>
> > void f(int) { }
> > void f() { }
> > template<typename... Args>
> > struct foo {
> >   template<size_t... Is>
> >   static void func(Args... a) {
> >     std::tuple<Args...> t;
> >     f(std::get<Is>(t)...);
> >   }
> > };
> >
> > int main() {
> >   foo<int>::func<0>(0);
> > #ifdef FAIL
> >   foo<>::func<>();
> > #endif
> > }
> >
> > - Dave
> >
> >>
> >>
> >> Thanks!
> >>
> >> ~Aaron
> >>
> >> > +    }
> >> > +  };
> >> > +
> >> > +  template <typename... ArgTs> class ReadArgs {
> >> > +  public:
> >> > +    std::error_code operator()() { return std::error_code(); }
> >> > +  };
> >> > +
> >> > +  template <typename ArgT, typename... ArgTs>
> >> > +  class ReadArgs<ArgT, ArgTs...> : public ReadArgs<ArgTs...> {
> >> > +  public:
> >> > +    ReadArgs(ArgT &Arg, ArgTs &... Args)
> >> > +        : ReadArgs<ArgTs...>(Args...), Arg(Arg) {}
> >> > +
> >> > +    std::error_code operator()(ArgT &ArgVal, ArgTs &... ArgVals) {
> >> > +      this->Arg = std::move(ArgVal);
> >> > +      return ReadArgs<ArgTs...>::operator()(ArgVals...);
> >> > +    }
> >> > +
> >> > +  private:
> >> > +    ArgT &Arg;
> >> > +  };
> >> > +};
> >> > +
> >> >  /// Contains primitive utilities for defining, calling and handling
> >> > calls to
> >> >  /// remote procedures. ChannelT is a bidirectional stream conforming
> to
> >> > the
> >> >  /// RPCChannel interface (see RPCChannel.h), and ProcedureIdT is a
> >> > procedure
> >> > @@ -62,7 +132,8 @@ namespace remote {
> >> >  /// read yet. Expect will deserialize the id and assert that it
> matches
> >> > Proc's
> >> >  /// id. If it does not, and unexpected RPC call error is returned.
> >> >
> >> > -template <typename ChannelT, typename ProcedureIdT = uint32_t> class
> >> > RPC {
> >> > +template <typename ChannelT, typename ProcedureIdT = uint32_t>
> >> > +class RPC : public RPCBase {
> >> >  public:
> >> >    /// Utility class for defining/referring to RPC procedures.
> >> >    ///
> >> > @@ -89,75 +160,16 @@ public:
> >> >    ///         })
> >> >    ///     /* handle EC */;
> >> >    ///
> >> > -  template <ProcedureIdT ProcId, typename... Ts> class Procedure {
> >> > -  public:
> >> > -    static const ProcedureIdT Id = ProcId;
> >> > -  };
> >> > -
> >> > -private:
> >> > -  template <typename Proc> class CallHelper;
> >> > -
> >> > -  template <ProcedureIdT ProcId, typename... ArgTs>
> >> > -  class CallHelper<Procedure<ProcId, ArgTs...>> {
> >> > -  public:
> >> > -    static std::error_code call(ChannelT &C, const ArgTs &... Args) {
> >> > -      if (auto EC = serialize(C, ProcId))
> >> > -        return EC;
> >> > -      // If you see a compile-error on this line you're probably
> >> > calling a
> >> > -      // function with the wrong signature.
> >> > -      return serialize_seq(C, Args...);
> >> > -    }
> >> > -  };
> >> > +  template <ProcedureIdT ProcId, typename... Ts>
> >> > +  using Procedure = ProcedureHelper<ProcedureIdT, ProcId, Ts...>;
> >> >
> >> > -  template <typename Proc> class HandlerHelper;
> >> > -
> >> > -  template <ProcedureIdT ProcId, typename... ArgTs>
> >> > -  class HandlerHelper<Procedure<ProcId, ArgTs...>> {
> >> > -  public:
> >> > -    template <typename HandlerT>
> >> > -    static std::error_code handle(ChannelT &C, HandlerT Handler) {
> >> > -      return readAndHandle(C, Handler,
> >> > llvm::index_sequence_for<ArgTs...>());
> >> > -    }
> >> > -
> >> > -  private:
> >> > -    template <typename HandlerT, size_t... Is>
> >> > -    static std::error_code readAndHandle(ChannelT &C, HandlerT
> Handler,
> >> > -                                         llvm::index_sequence<Is...>
> _)
> >> > {
> >> > -      std::tuple<ArgTs...> RPCArgs;
> >> > -      if (auto EC = deserialize_seq(C, std::get<Is>(RPCArgs)...))
> >> > -        return EC;
> >> > -      return Handler(std::get<Is>(RPCArgs)...);
> >> > -    }
> >> > -  };
> >> > -
> >> > -  template <typename... ArgTs> class ReadArgs {
> >> > -  public:
> >> > -    std::error_code operator()() { return std::error_code(); }
> >> > -  };
> >> > -
> >> > -  template <typename ArgT, typename... ArgTs>
> >> > -  class ReadArgs<ArgT, ArgTs...> : public ReadArgs<ArgTs...> {
> >> > -  public:
> >> > -    ReadArgs(ArgT &Arg, ArgTs &... Args)
> >> > -        : ReadArgs<ArgTs...>(Args...), Arg(Arg) {}
> >> > -
> >> > -    std::error_code operator()(ArgT &ArgVal, ArgTs &... ArgVals) {
> >> > -      this->Arg = std::move(ArgVal);
> >> > -      return ReadArgs<ArgTs...>::operator()(ArgVals...);
> >> > -    }
> >> > -
> >> > -  private:
> >> > -    ArgT &Arg;
> >> > -  };
> >> > -
> >> > -public:
> >> >    /// Serialize Args... to channel C, but do not call C.send().
> >> >    ///
> >> >    /// For buffered channels, this can be used to queue up several
> calls
> >> > before
> >> >    /// flushing the channel.
> >> >    template <typename Proc, typename... ArgTs>
> >> >    static std::error_code appendCall(ChannelT &C, const ArgTs &...
> Args)
> >> > {
> >> > -    return CallHelper<Proc>::call(C, Args...);
> >> > +    return CallHelper<ChannelT, Proc>::call(C, Args...);
> >> >    }
> >> >
> >> >    /// Serialize Args... to channel C and call C.send().
> >> > @@ -178,7 +190,7 @@ public:
> >> >    /// the arguments used in the Proc typedef.
> >> >    template <typename Proc, typename HandlerT>
> >> >    static std::error_code handle(ChannelT &C, HandlerT Handler) {
> >> > -    return HandlerHelper<Proc>::handle(C, Handler);
> >> > +    return HandlerHelper<ChannelT, Proc>::handle(C, Handler);
> >> >    }
> >> >
> >> >    /// Deserialize a ProcedureIdT from C and verify it matches the id
> >> > for Proc.
> >> >
> >> >
> >> > _______________________________________________
> >> > llvm-commits mailing list
> >> > llvm-commits at lists.llvm.org
> >> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160115/3268cd60/attachment.html>


More information about the llvm-commits mailing list