<div dir="ltr">Hi Lang,<div><br></div><div>I'm getting a lot of build errors that look to be related to this patch. Building with clang 4.0 on Linux x86:</div><div><br></div><div>In file included from unittests/ExecutionEngine/Orc/QueueChannel.cpp:9:<br>unittests/ExecutionEngine/Orc/QueueChannel.h:154:23: error: implicit instantiation of undefined template 'std::atomic<unsigned long>'<br> std::atomic<size_t> InFlightIncomingMessages{0};<br> ^<br>/usr/bin/../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/atomic_base.h:126:12: note: template is declared here<br> struct atomic;<br> ^<br>In file included from unittests/ExecutionEngine/Orc/QueueChannel.cpp:9:<br>unittests/ExecutionEngine/Orc/QueueChannel.h:155:23: error: implicit instantiation of undefined template 'std::atomic<unsigned long>'<br> std::atomic<size_t> CompletedIncomingMessages{0};<br> ^<br>/usr/bin/../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/atomic_base.h:126:12: note: template is declared here<br> struct atomic;<br> ^<br>In file included from unittests/ExecutionEngine/Orc/QueueChannel.cpp:9:<br>unittests/ExecutionEngine/Orc/QueueChannel.h:156:23: error: implicit instantiation of undefined template 'std::atomic<unsigned long>'<br> std::atomic<size_t> InFlightOutgoingMessages{0};<br> ^<br>/usr/bin/../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/atomic_base.h:126:12: note: template is declared here<br> struct atomic;<br> ^<br>In file included from unittests/ExecutionEngine/Orc/QueueChannel.cpp:9:<br>unittests/ExecutionEngine/Orc/QueueChannel.h:157:23: error: implicit instantiation of undefined template 'std::atomic<unsigned long>'<br> std::atomic<size_t> CompletedOutgoingMessages{0};<br> ^<br>/usr/bin/../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/atomic_base.h:126:12: note: template is declared here<br> struct atomic;<br> ^<br>In file included from unittests/ExecutionEngine/Orc/QueueChannel.cpp:9:<br>unittests/ExecutionEngine/Orc/QueueChannel.h:158:23: error: implicit instantiation of undefined template 'std::atomic<unsigned long>'<br> std::atomic<size_t> SendCalls{0};<br> ^<br>/usr/bin/../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/atomic_base.h:126:12: note: template is declared here<br> struct atomic;<br> ^<br>In file included from unittests/ExecutionEngine/Orc/QueueChannel.cpp:9:<br>unittests/ExecutionEngine/Orc/QueueChannel.h:72:7: warning: 'llvm::QueueChannel' has virtual functions but non-virtual destructor [-Wnon-virtual-dtor]<br>class QueueChannel : public orc::rpc::RawByteChannel {<br> ^<br>unittests/ExecutionEngine/Orc/QueueChannel.h:92:38: error: cannot initialize object parameter of type 'llvm::orc::rpc::RawByteChannel' with an expression of type 'llvm::QueueChannel'<br> return orc::rpc::RawByteChannel::endSendMessage();<br> ^~~~~~~~~~~~~~<br>unittests/ExecutionEngine/Orc/QueueChannel.h:104:38: error: cannot initialize object parameter of type 'llvm::orc::rpc::RawByteChannel' with an expression of type 'llvm::QueueChannel'<br> return orc::rpc::RawByteChannel::endReceiveMessage();<br> ^~~~~~~~~~~~~~~~~<br>1 warning and 7 errors generated.<br></div><div><br></div><div><br></div><div>Teresa</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Sep 6, 2019 at 12:20 PM Lang Hames via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Author: lhames<br>
Date: Fri Sep 6 12:21:59 2019<br>
New Revision: 371245<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=371245&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=371245&view=rev</a><br>
Log:<br>
[ORC] Make sure RPC channel-send is called in blocking calls and responses.<br>
<br>
ORC-RPC batches calls by default, and the channel's send method must be called<br>
to transfer any buffered calls to the remote. The call to send was missing on<br>
responses and blocking calls in the SingleThreadedRPCEndpoint. This patch adds<br>
the necessary calls and modifies the RPC unit test to check for them.<br>
<br>
Modified:<br>
llvm/trunk/include/llvm/ExecutionEngine/Orc/RPCUtils.h<br>
llvm/trunk/unittests/ExecutionEngine/Orc/QueueChannel.h<br>
llvm/trunk/unittests/ExecutionEngine/Orc/RPCUtilsTest.cpp<br>
<br>
Modified: llvm/trunk/include/llvm/ExecutionEngine/Orc/RPCUtils.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/Orc/RPCUtils.h?rev=371245&r1=371244&r2=371245&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/Orc/RPCUtils.h?rev=371245&r1=371244&r2=371245&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/ExecutionEngine/Orc/RPCUtils.h (original)<br>
+++ llvm/trunk/include/llvm/ExecutionEngine/Orc/RPCUtils.h Fri Sep 6 12:21:59 2019<br>
@@ -338,7 +338,9 @@ public:<br>
return Err;<br>
<br>
// Close the response message.<br>
- return C.endSendMessage();<br>
+ if (auto Err = C.endSendMessage())<br>
+ return Err;<br>
+ return C.send();<br>
}<br>
<br>
template <typename ChannelT, typename FunctionIdT, typename SequenceNumberT><br>
@@ -350,7 +352,9 @@ public:<br>
return Err2;<br>
if (auto Err2 = serializeSeq(C, std::move(Err)))<br>
return Err2;<br>
- return C.endSendMessage();<br>
+ if (auto Err2 = C.endSendMessage())<br>
+ return Err2;<br>
+ return C.send();<br>
}<br>
<br>
};<br>
@@ -378,8 +382,11 @@ public:<br>
C, *ResultOrErr))<br>
return Err;<br>
<br>
- // Close the response message.<br>
- return C.endSendMessage();<br>
+ // End the response message.<br>
+ if (auto Err = C.endSendMessage())<br>
+ return Err;<br>
+<br>
+ return C.send();<br>
}<br>
<br>
template <typename ChannelT, typename FunctionIdT, typename SequenceNumberT><br>
@@ -389,7 +396,9 @@ public:<br>
return Err;<br>
if (auto Err2 = C.startSendMessage(ResponseId, SeqNo))<br>
return Err2;<br>
- return C.endSendMessage();<br>
+ if (auto Err2 = C.endSendMessage())<br>
+ return Err2;<br>
+ return C.send();<br>
}<br>
<br>
};<br>
@@ -1518,6 +1527,12 @@ public:<br>
detail::ResultTraits<typename Func::ReturnType>::consumeAbandoned(<br>
std::move(Result));<br>
return std::move(Err);<br>
+ }<br>
+<br>
+ if (auto Err = this->C.send()) {<br>
+ detail::ResultTraits<typename Func::ReturnType>::consumeAbandoned(<br>
+ std::move(Result));<br>
+ return std::move(Err);<br>
}<br>
<br>
while (!ReceivedResponse) {<br>
<br>
Modified: llvm/trunk/unittests/ExecutionEngine/Orc/QueueChannel.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/QueueChannel.h?rev=371245&r1=371244&r2=371245&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/QueueChannel.h?rev=371245&r1=371244&r2=371245&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/unittests/ExecutionEngine/Orc/QueueChannel.h (original)<br>
+++ llvm/trunk/unittests/ExecutionEngine/Orc/QueueChannel.h Fri Sep 6 12:21:59 2019<br>
@@ -80,6 +80,30 @@ public:<br>
QueueChannel(QueueChannel&&) = delete;<br>
QueueChannel& operator=(QueueChannel&&) = delete;<br>
<br>
+ template <typename FunctionIdT, typename SequenceIdT><br>
+ Error startSendMessage(const FunctionIdT &FnId, const SequenceIdT &SeqNo) {<br>
+ ++InFlightOutgoingMessages;<br>
+ return orc::rpc::RawByteChannel::startSendMessage(FnId, SeqNo);<br>
+ }<br>
+<br>
+ Error endSendMessage() {<br>
+ --InFlightOutgoingMessages;<br>
+ ++CompletedOutgoingMessages;<br>
+ return orc::rpc::RawByteChannel::endSendMessage();<br>
+ }<br>
+<br>
+ template <typename FunctionIdT, typename SequenceNumberT><br>
+ Error startReceiveMessage(FunctionIdT &FnId, SequenceNumberT &SeqNo) {<br>
+ ++InFlightIncomingMessages;<br>
+ return orc::rpc::RawByteChannel::startReceiveMessage(FnId, SeqNo);<br>
+ }<br>
+<br>
+ Error endReceiveMessage() {<br>
+ --InFlightIncomingMessages;<br>
+ ++CompletedIncomingMessages;<br>
+ return orc::rpc::RawByteChannel::endReceiveMessage();<br>
+ }<br>
+<br>
Error readBytes(char *Dst, unsigned Size) override {<br>
std::unique_lock<std::mutex> Lock(InQueue->getMutex());<br>
while (Size) {<br>
@@ -112,7 +136,10 @@ public:<br>
return Error::success();<br>
}<br>
<br>
- Error send() override { return Error::success(); }<br>
+ Error send() override {<br>
+ ++SendCalls;<br>
+ return Error::success();<br>
+ }<br>
<br>
void close() {<br>
auto ChannelClosed = []() { return make_error<QueueChannelClosedError>(); };<br>
@@ -124,6 +151,11 @@ public:<br>
<br>
uint64_t NumWritten = 0;<br>
uint64_t NumRead = 0;<br>
+ std::atomic<size_t> InFlightIncomingMessages{0};<br>
+ std::atomic<size_t> CompletedIncomingMessages{0};<br>
+ std::atomic<size_t> InFlightOutgoingMessages{0};<br>
+ std::atomic<size_t> CompletedOutgoingMessages{0};<br>
+ std::atomic<size_t> SendCalls{0};<br>
<br>
private:<br>
<br>
<br>
Modified: llvm/trunk/unittests/ExecutionEngine/Orc/RPCUtilsTest.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/RPCUtilsTest.cpp?rev=371245&r1=371244&r2=371245&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/RPCUtilsTest.cpp?rev=371245&r1=371244&r2=371245&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/unittests/ExecutionEngine/Orc/RPCUtilsTest.cpp (original)<br>
+++ llvm/trunk/unittests/ExecutionEngine/Orc/RPCUtilsTest.cpp Fri Sep 6 12:21:59 2019<br>
@@ -214,6 +214,17 @@ TEST(DummyRPC, TestCallAsyncVoidBool) {<br>
EXPECT_FALSE(!!Err) << "Client failed to handle response from void(bool)";<br>
}<br>
<br>
+ // The client should have made two calls to send: One implicit call to<br>
+ // negotiate the VoidBool function key, and a second to make the VoidBool<br>
+ // call.<br>
+ EXPECT_EQ(Channels.first->SendCalls, 2U)<br>
+ << "Expected one send call to have been made by client";<br>
+<br>
+ // The server should have made two calls to send: One to send the response to<br>
+ // the negotiate call, and another to send the response to the VoidBool call.<br>
+ EXPECT_EQ(Channels.second->SendCalls, 2U)<br>
+ << "Expected two send calls to have been made by server";<br>
+<br>
ServerThread.join();<br>
}<br>
<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr"><div dir="ltr"><div><span style="font-family:Times;font-size:medium"><table cellspacing="0" cellpadding="0"><tbody><tr style="color:rgb(85,85,85);font-family:sans-serif;font-size:small"><td nowrap style="border-top:2px solid rgb(213,15,37)">Teresa Johnson |</td><td nowrap style="border-top:2px solid rgb(51,105,232)"> Software Engineer |</td><td nowrap style="border-top:2px solid rgb(0,153,57)"> <a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a> |</td><td nowrap style="border-top:2px solid rgb(238,178,17)"><br></td></tr></tbody></table></span></div></div></div>