From adda9a50221ddfb84cf1c3326a6dcf3ba41e017a Mon Sep 17 00:00:00 2001 From: Todd Nowacki Date: Mon, 24 Jun 2013 17:45:59 -0700 Subject: [PATCH] HPHP Changes for Clang More changes for HPHP to help make it clang friendly ~~~hphp/compiler/expression/constant_expression.h ~~~hphp/compiler/expression/function_call.h rfind returns a size_t/unsigned int ~~~hphp/runtime/base/server/http_protocol.cpp Switched to std::to_string. Assuming [] was not intended here ~~~hphp/runtime/base/ref_data.h These fields were accessed in a public manner, assuming public was intended instead of private ~~~hphp/runtime/base/variable_serializer.cpp Switched to using [] and & to make clang happy. Assuming this was to either take or drop the first char. ~~~hphp/runtime/ext/asio/asio_external_thread_event_queue.h ~~~hphp/runtime/ext/asio/asio_external_thread_event_queue.cpp Cast which performs the conversions of a reinterpret_cast is not allowed in a constant expression. This is been moved to a macro as a temporary fix. +++hphp/runtime/ext/ext_misc.cpp Added std::atomic to supress warnings ~~~hphp/runtime/vm/jit/simplifier.cpp Chosen constructor is explicit in copy-initialization ~~~hphp/runtime/vm/jit/translator-asm-helpers.S Ambiguous instructions require an explicit suffix Changed cmp to cmpl ~~~hphp/runtime/vm/jit/translator-x64-helpers.cpp Clang does not support global register variables +++hphp/runtime/vm/unwind.cpp describeFault was only used when DEBUG or USE_TRACE was defined ~~~hphp/runtime/vm/verifier/check_unit.cpp Made fmt pointer const to avoid string format issues/warnings ~~~hphp/util/stack_trace.cpp Clang does not support variable-length arrays. Uniqe_ptr is used instead to take advantage runtime-sized arrays, a restriced form of variable-length arrays ~~~hphp/util/thread_local.h Clang seems to be supporting the __thread attribute, or at the very least it is not complaining about it. ~~~hphp/util/tiny_vector.h Clang does not like the flexible array here, since T is not always POD. I have reimplemented the array here by just sticking one value in the struct and calculating the offset from its address manually. Alterinatively, we could change the the non-POD types to be pointers, or we could edit their implemenations. +++hphp/util/util.h Created a template for the union, A function declared with the constexpr specifier cannot contain type declarations that do not define classes or enumerations +++hphp/runtime/vm/jit/x64-util.h Added a TODO The way hphp/runtime/vm/jit/x64-util.h is currently implemented, it only works if USE_GCC_FAST_TLS is defined --- hphp/compiler/expression/constant_expression.h | 2 +- hphp/compiler/expression/function_call.h | 2 +- hphp/runtime/base/ref_data.h | 2 +- hphp/runtime/base/server/http_protocol.cpp | 2 +- hphp/runtime/base/variable_serializer.cpp | 2 +- .../ext/asio/asio_external_thread_event_queue.cpp | 12 ++++++------ .../ext/asio/asio_external_thread_event_queue.h | 8 ++++++-- hphp/runtime/vm/jit/simplifier.cpp | 2 +- hphp/runtime/vm/jit/translator-asm-helpers.S | 4 ++-- hphp/runtime/vm/jit/translator-x64-helpers.cpp | 2 +- hphp/runtime/vm/jit/x64-util.h | 1 + hphp/runtime/vm/unwind.cpp | 3 ++- hphp/runtime/vm/verifier/check_unit.cpp | 2 +- hphp/util/stack_trace.cpp | 6 +++--- hphp/util/thread_local.h | 7 +++++-- hphp/util/tiny_vector.h | 3 +-- hphp/util/util.h | 12 +++++++----- 17 files changed, 41 insertions(+), 31 deletions(-) diff --git a/hphp/compiler/expression/constant_expression.h b/hphp/compiler/expression/constant_expression.h index b46972c78..023093fcb 100644 --- a/hphp/compiler/expression/constant_expression.h +++ b/hphp/compiler/expression/constant_expression.h @@ -53,7 +53,7 @@ public: const std::string &getName() const { return m_name;} const std::string &getOriginalName() const { return m_origName;} const std::string getNonNSOriginalName() const { - int nsPos = m_origName.rfind('\\'); + auto nsPos = m_origName.rfind('\\'); if (nsPos == string::npos) { return m_origName; } diff --git a/hphp/compiler/expression/function_call.h b/hphp/compiler/expression/function_call.h index 6d956e77a..40ba2bd7b 100644 --- a/hphp/compiler/expression/function_call.h +++ b/hphp/compiler/expression/function_call.h @@ -48,7 +48,7 @@ public: const std::string &getName() const { return m_name; } const std::string &getOriginalName() const { return m_origName; } const std::string getNonNSOriginalName() const { - int nsPos = m_origName.rfind('\\'); + auto nsPos = m_origName.rfind('\\'); if (nsPos == string::npos) { return m_origName; } diff --git a/hphp/runtime/base/ref_data.h b/hphp/runtime/base/ref_data.h index 76cd5aad4..4a7a90822 100644 --- a/hphp/runtime/base/ref_data.h +++ b/hphp/runtime/base/ref_data.h @@ -104,7 +104,7 @@ private: TypedValue m_tv; }; #else // overlap TypedValue with count -private: +public: union { struct { void* _ptr; diff --git a/hphp/runtime/base/server/http_protocol.cpp b/hphp/runtime/base/server/http_protocol.cpp index b2f19fa88..c08e43d29 100644 --- a/hphp/runtime/base/server/http_protocol.cpp +++ b/hphp/runtime/base/server/http_protocol.cpp @@ -337,7 +337,7 @@ void HttpProtocol::PrepareSystemVariables(Transport *transport, // Need to append port if (!transport->isSSL() && RuntimeOption::ServerPort != 80) { - port_suffix = ":" + RuntimeOption::ServerPort; + port_suffix = folly::format(":{}", RuntimeOption::ServerPort).str(); } server.set(s_SCRIPT_URI, String(prefix + (hostHeader.empty() ? hostName + port_suffix : diff --git a/hphp/runtime/base/variable_serializer.cpp b/hphp/runtime/base/variable_serializer.cpp index 6cf6aa260..572543fa8 100644 --- a/hphp/runtime/base/variable_serializer.cpp +++ b/hphp/runtime/base/variable_serializer.cpp @@ -657,7 +657,7 @@ void VariableSerializer::writePropertyKey(CStrRef prop) { int l = strlen(cls); m_buf->append(cls + l + 1, kl - l - 2); int o = m_type == PrintR ? 1 : 0; - m_buf->append("\":\"" + o, 3 - 2*o); + m_buf->append(&"\":\""[o], 3 - 2*o); m_buf->append(cls, l); const char priv[] = "\":private"; m_buf->append(priv + o, sizeof(priv) - 1 - o); diff --git a/hphp/runtime/ext/asio/asio_external_thread_event_queue.cpp b/hphp/runtime/ext/asio/asio_external_thread_event_queue.cpp index 3c14ba7ca..5c8b94634 100644 --- a/hphp/runtime/ext/asio/asio_external_thread_event_queue.cpp +++ b/hphp/runtime/ext/asio/asio_external_thread_event_queue.cpp @@ -66,7 +66,7 @@ bool AsioExternalThreadEventQueue::abandonAllReceived(c_ExternalThreadEventWaitH bool AsioExternalThreadEventQueue::tryReceiveSome() { assert(!m_received); m_received = m_queue.exchange(nullptr); - assert(m_received != k_consumerWaiting); + assert(m_received != K_CONSUMER_WAITING); return m_received; } @@ -79,7 +79,7 @@ void AsioExternalThreadEventQueue::receiveSome() { // try receive external thread events without grabbing lock m_received = m_queue.exchange(nullptr); if (m_received) { - assert(m_received != k_consumerWaiting); + assert(m_received != K_CONSUMER_WAITING); return; } @@ -87,18 +87,18 @@ void AsioExternalThreadEventQueue::receiveSome() { std::unique_lock lock(m_queueMutex); // transition from empty to WAITING - if (m_queue.compare_exchange_strong(m_received, k_consumerWaiting)) { + if (m_queue.compare_exchange_strong(m_received, K_CONSUMER_WAITING)) { // wait for transition from WAITING to non-empty do { m_queueCondition.wait(lock); - } while (m_queue.load() == k_consumerWaiting); + } while (m_queue.load() == K_CONSUMER_WAITING); } else { // external thread transitioned from empty to non-empty while grabbing lock } m_received = m_queue.exchange(nullptr); assert(m_received); - assert(m_received != k_consumerWaiting); + assert(m_received != K_CONSUMER_WAITING); } /** @@ -107,7 +107,7 @@ void AsioExternalThreadEventQueue::receiveSome() { void AsioExternalThreadEventQueue::send(c_ExternalThreadEventWaitHandle* wait_handle) { auto next = m_queue.load(); while (true) { - while (next != k_consumerWaiting) { + while (next != K_CONSUMER_WAITING) { wait_handle->setNextToProcess(next); if (m_queue.compare_exchange_weak(next, wait_handle)) { return; diff --git a/hphp/runtime/ext/asio/asio_external_thread_event_queue.h b/hphp/runtime/ext/asio/asio_external_thread_event_queue.h index 4aac6ff6f..e498515d7 100644 --- a/hphp/runtime/ext/asio/asio_external_thread_event_queue.h +++ b/hphp/runtime/ext/asio/asio_external_thread_event_queue.h @@ -28,6 +28,12 @@ namespace HPHP { FORWARD_DECLARE_CLASS_BUILTIN(ExternalThreadEventWaitHandle); +/* This is not an optimal solution + * This value is in principle a constexp, but the integer-to-pointer cast would + * require a reinterpret cast, which is not allowed in a constexpr + */ +#define K_CONSUMER_WAITING (static_cast((void*)1L)) + class AsioExternalThreadEventQueue { public: AsioExternalThreadEventQueue(); @@ -41,8 +47,6 @@ class AsioExternalThreadEventQueue { void send(c_ExternalThreadEventWaitHandle* wait_handle); private: - static constexpr auto k_consumerWaiting = static_cast((void*)1L); - c_ExternalThreadEventWaitHandle* m_received; std::atomic m_queue; std::mutex m_queueMutex; diff --git a/hphp/runtime/vm/jit/simplifier.cpp b/hphp/runtime/vm/jit/simplifier.cpp index 6cc30419c..473112a4b 100644 --- a/hphp/runtime/vm/jit/simplifier.cpp +++ b/hphp/runtime/vm/jit/simplifier.cpp @@ -37,7 +37,7 @@ StackValueInfo getStackValue(SSATmp* sp, uint32_t index) { switch (inst->op()) { case DefSP: - return {}; + return StackValueInfo(); case ReDefGeneratorSP: case StashGeneratorSP: diff --git a/hphp/runtime/vm/jit/translator-asm-helpers.S b/hphp/runtime/vm/jit/translator-asm-helpers.S index fad169080..1583d6f8f 100644 --- a/hphp/runtime/vm/jit/translator-asm-helpers.S +++ b/hphp/runtime/vm/jit/translator-asm-helpers.S @@ -52,7 +52,7 @@ enterTCHelper: * in the ActRec pointed to by r15. The 0x1 in the cmp instruction * must be kept in sync with REQ_BIND_CALL in abi-x64.h. */ - cmp $0x1, 0x0(%rcx) + cmpl $0x1, 0x0(%rcx) jne .LenterTCHelper$jumpToTC lea .LenterTCHelper$serviceReqLabel(%rip), %rax push %rax @@ -104,4 +104,4 @@ enterTCHelper: enterTCHelper: brk 0 -#endif \ No newline at end of file +#endif diff --git a/hphp/runtime/vm/jit/translator-x64-helpers.cpp b/hphp/runtime/vm/jit/translator-x64-helpers.cpp index b3bc8511a..f19edcc09 100644 --- a/hphp/runtime/vm/jit/translator-x64-helpers.cpp +++ b/hphp/runtime/vm/jit/translator-x64-helpers.cpp @@ -38,7 +38,7 @@ checkEval(HPHP::Eval::PhpFile* efile) { * like any other local. So I put this function in its own * file to avoid impacting the rest of translator-x64.cpp */ -#ifdef __x86_64__ +#if defined(__x86_64__) && !defined(__clang__) register Cell* sp asm("rbx"); #else // TODO(#2056140): we have to decide regalloc conventions for ARM diff --git a/hphp/runtime/vm/jit/x64-util.h b/hphp/runtime/vm/jit/x64-util.h index 1e4b3e2cf..816ad03e3 100644 --- a/hphp/runtime/vm/jit/x64-util.h +++ b/hphp/runtime/vm/jit/x64-util.h @@ -56,6 +56,7 @@ emitTLSLoad(X64Assembler& a, const void* datum, RegNumber reg) { a. load_disp32_reg64(virtualAddress, reg); } +//TODO(2525714) Will not work without USE_GCC_FAST_TLS being defined template static inline void emitTLSLoad(X64Assembler& a, const ThreadLocalNoCheck& datum, diff --git a/hphp/runtime/vm/unwind.cpp b/hphp/runtime/vm/unwind.cpp index ca0deeea9..7977b0509 100644 --- a/hphp/runtime/vm/unwind.cpp +++ b/hphp/runtime/vm/unwind.cpp @@ -35,7 +35,7 @@ using boost::implicit_cast; namespace { ////////////////////////////////////////////////////////////////////// - +#if (defined(DEBUG) || defined(USE_TRACE)) std::string describeFault(const Fault& f) { switch (f.m_faultType) { case Fault::UserException: @@ -47,6 +47,7 @@ std::string describeFault(const Fault& f) { } not_reached(); } +#endif void discardStackTemps(const ActRec* const fp, Stack& stack, diff --git a/hphp/runtime/vm/verifier/check_unit.cpp b/hphp/runtime/vm/verifier/check_unit.cpp index 2ab181663..093c04d6b 100644 --- a/hphp/runtime/vm/verifier/check_unit.cpp +++ b/hphp/runtime/vm/verifier/check_unit.cpp @@ -42,7 +42,7 @@ class UnitChecker { private: template - void error(const char* fmt, Args&&... args) { + void error(const char* const fmt, Args&&... args) { verify_error(m_unit, nullptr, fmt, std::forward(args)...); } diff --git a/hphp/util/stack_trace.cpp b/hphp/util/stack_trace.cpp index 9a75667ad..b26e6aa07 100644 --- a/hphp/util/stack_trace.cpp +++ b/hphp/util/stack_trace.cpp @@ -175,10 +175,10 @@ void StackTraceNoHeap::printStackTrace(int fd) const { // m_btpointers_cnt must be an upper bound on the number of filenames // then *2 for tolerable hash table behavior unsigned int bfds_size = m_btpointers_cnt * 2; - NamedBfd bfds[bfds_size]; - for (unsigned int i = 0; i < bfds_size; i++) bfds[i].key[0]='\0'; + const std::unique_ptr bfds (new NamedBfd[bfds_size]); + for (unsigned int i = 0; i < bfds_size; i++) bfds.get()[i].key[0]='\0'; for (unsigned int i = 0; i < m_btpointers_cnt; i++) { - if (Translate(fd, m_btpointers[i], frame, bfds, bfds_size)) { + if (Translate(fd, m_btpointers[i], frame, bfds.get(), bfds_size)) { frame++; } } diff --git a/hphp/util/thread_local.h b/hphp/util/thread_local.h index fc12606d8..6a59d7af4 100644 --- a/hphp/util/thread_local.h +++ b/hphp/util/thread_local.h @@ -26,14 +26,17 @@ namespace HPHP { /////////////////////////////////////////////////////////////////////////////// -// Only gcc >= 4.3.0 supports the '__thread' keyword for thread locals +// gcc >= 4.3.0 supports the '__thread' keyword for thread locals +// +// Clang seems to have added this feature, or at the very least it is ignoring +// __thread keyword and compiling anyway // // icc 13.0.0 appears to support it as well but we end up with // assembler warnings of unknown importance about incorrect section // types #if !defined(NO_TLS) && \ - ((__llvm__ && !__clang__) || \ + ((__llvm__ && __clang__) || \ __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ > 3) || \ __INTEL_COMPILER) #define USE_GCC_FAST_TLS diff --git a/hphp/util/tiny_vector.h b/hphp/util/tiny_vector.h index c3fe8a7af..8a29ac7ae 100644 --- a/hphp/util/tiny_vector.h +++ b/hphp/util/tiny_vector.h @@ -158,7 +158,7 @@ struct TinyVector : private boost::noncopyable { private: struct HeapData { uint32_t capacity; // numbers of vals---excludes this capacity field - T vals[]; + T vals[0]; }; T* location(size_t index) { @@ -215,4 +215,3 @@ private: } #endif - diff --git a/hphp/util/util.h b/hphp/util/util.h index 9f451698c..dbd2e4c09 100644 --- a/hphp/util/util.h +++ b/hphp/util/util.h @@ -334,13 +334,15 @@ int64_t getVTableOffset(MethodPtr meth) { return u.offset - 1; } +template +union MethodPtrU { + MethodPtr meth; + void* ptr; +}; + template constexpr void* getMethodPtr(MethodPtr meth) { - union U { - MethodPtr meth; - void* ptr; - }; - return ((U*)(&meth))->ptr; + return ((MethodPtrU*)(&meth))->ptr; } /**