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

Aaron Ballman via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 15 11:14:14 PST 2016


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
>
>


More information about the llvm-commits mailing list