[Lldb-commits] [PATCH] D32823: Remove an expensive lock from Timer
labath added a comment.
Don't forget to update the usages in unit tests (and make sure the
check-lldb-unit target passes).
Seems reasonable, however: I am not sure who actually uses these timers. I'd be
tempted to just remove the timers that are causing the contention.
Comment at: include/lldb/Core/Timer.h:26
+class TimerCategory {
+public:
I would put this inside the Timer class, so that we can refer to it as
Timer::Category. I guess tastes might differ.
Comment at: source/Core/Timer.cpp:147
- const size_t count = sorted_iterators.size();
+ const size_t count = sorted.size();
for (size_t i = 0; i < count; ++i) {
This could be a range-based loop.
Comment at: source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:5421
char *buf = (char *) malloc (ident_command.cmdsize);
-if (buf != nullptr
+if (buf != nullptr
&& m_data.CopyData (offset, ident_command.cmdsize, buf) ==
ident_command.cmdsize) {
I am not sure how you format your changes, but you should make sure you format
only the lines you've touched, and not the whole files. git-clang-format
will do that for you -- when you set it up, you just run `git clang-format
HEAD^` and it will format your last patch).
Repository:
rL LLVM
https://reviews.llvm.org/D32823
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32421: Fix segfault resulting from empty print prompt
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Perfect, thank you. Do you have commit access? https://reviews.llvm.org/D32421 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32832: Make ConstString creation and access lockfree
labath added a comment. I have feeling you gave up on the llvm change too quickly. My interpretation of that thread was that there was general support for the hash function switch, and people only wanted some confirmation it will not regress. However, I do believe that this can be made faster than any solution based on a generic string map (but I don't like hardcoding bucket numbers either). For a change of this depth, I also think it would be fair to ask for some ConstString unit tests, so we have something we can run thread sanitizer on, instead of tracking down obscure debugger failures. Comment at: source/Utility/ConstString.cpp:58 + typedef std::atomic StringPool; + typedef Entry StringPoolEntryType; s/StringPoolEntryType/Pool::Entry/ instead of the typedef? Repository: rL LLVM https://reviews.llvm.org/D32832 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r302133 - MainLoop: Add unit tests
Author: labath
Date: Thu May 4 05:11:33 2017
New Revision: 302133
URL: http://llvm.org/viewvc/llvm-project?rev=302133&view=rev
Log:
MainLoop: Add unit tests
Summary:
This adds a couple of unit tests to the MainLoop class. To get the
kqueue based version of the signal handling passing, I needed to
modify the implementation a bit to make the queue object persistent.
Otherwise, only the signals which are send during the Run call would get
processed, which did not match the ppoll behaviour.
I also took the opportunity to remove the ForEach template functions and
replace them with something more reasonable.
Reviewers: beanz, eugene
Subscribers: lldb-commits, mgorny
Differential Revision: https://reviews.llvm.org/D32753
Added:
lldb/trunk/unittests/Host/MainLoopTest.cpp
Modified:
lldb/trunk/include/lldb/Host/MainLoop.h
lldb/trunk/source/Host/common/MainLoop.cpp
lldb/trunk/unittests/Host/CMakeLists.txt
Modified: lldb/trunk/include/lldb/Host/MainLoop.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/MainLoop.h?rev=302133&r1=302132&r2=302133&view=diff
==
--- lldb/trunk/include/lldb/Host/MainLoop.h (original)
+++ lldb/trunk/include/lldb/Host/MainLoop.h Thu May 4 05:11:33 2017
@@ -42,6 +42,7 @@ private:
public:
typedef std::unique_ptr SignalHandleUP;
+ MainLoop();
~MainLoop() override;
ReadHandleUP RegisterReadObject(const lldb::IOObjectSP &object_sp,
@@ -71,6 +72,9 @@ protected:
void UnregisterSignal(int signo);
private:
+ void ProcessReadObject(IOObject::WaitableHandle handle);
+ void ProcessSignal(int signo);
+
class SignalHandle {
public:
~SignalHandle() { m_mainloop.UnregisterSignal(m_signo); }
@@ -97,6 +101,9 @@ private:
llvm::DenseMap m_read_fds;
llvm::DenseMap m_signals;
+#if HAVE_SYS_EVENT_H
+ int m_kqueue;
+#endif
bool m_terminate_request : 1;
};
Modified: lldb/trunk/source/Host/common/MainLoop.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/MainLoop.cpp?rev=302133&r1=302132&r2=302133&view=diff
==
--- lldb/trunk/source/Host/common/MainLoop.cpp (original)
+++ lldb/trunk/source/Host/common/MainLoop.cpp Thu May 4 05:11:33 2017
@@ -18,6 +18,11 @@
#include
#include
+// Multiplexing is implemented using kqueue on systems that support it (BSD
+// variants including OSX). On linux we use ppoll, while android uses pselect
+// (ppoll is present but not implemented properly). On windows we use WSApoll
+// (which does not support signals).
+
#if HAVE_SYS_EVENT_H
#include
#elif defined(LLVM_ON_WIN32)
@@ -65,92 +70,72 @@ static void SignalHandler(int signo, sig
class MainLoop::RunImpl {
public:
- // TODO: Use llvm::Expected
- static std::unique_ptr Create(MainLoop &loop, Error &error);
- ~RunImpl();
+ RunImpl(MainLoop &loop);
+ ~RunImpl() = default;
Error Poll();
-
- template void ForEachReadFD(F &&f);
- template void ForEachSignal(F &&f);
+ void ProcessEvents();
private:
MainLoop &loop;
#if HAVE_SYS_EVENT_H
- int queue_id;
std::vector in_events;
struct kevent out_events[4];
int num_events = -1;
- RunImpl(MainLoop &loop, int queue_id) : loop(loop), queue_id(queue_id) {
-in_events.reserve(loop.m_read_fds.size() + loop.m_signals.size());
- }
#else
- std::vector signals;
#ifdef FORCE_PSELECT
fd_set read_fd_set;
#else
std::vector read_fds;
#endif
- RunImpl(MainLoop &loop) : loop(loop) {
-signals.reserve(loop.m_signals.size());
- }
-
sigset_t get_sigmask();
#endif
};
#if HAVE_SYS_EVENT_H
-MainLoop::RunImpl::~RunImpl() {
- int r = close(queue_id);
- assert(r == 0);
- (void)r;
-}
-std::unique_ptr MainLoop::RunImpl::Create(MainLoop &loop,
Error &error)
-{
- error.Clear();
- int queue_id = kqueue();
- if(queue_id < 0) {
-error = Error(errno, eErrorTypePOSIX);
-return nullptr;
- }
- return std::unique_ptr(new RunImpl(loop, queue_id));
+MainLoop::RunImpl::RunImpl(MainLoop &loop) : loop(loop) {
+ in_events.reserve(loop.m_read_fds.size());
}
Error MainLoop::RunImpl::Poll() {
- in_events.resize(loop.m_read_fds.size() + loop.m_signals.size());
+ in_events.resize(loop.m_read_fds.size());
unsigned i = 0;
for (auto &fd : loop.m_read_fds)
EV_SET(&in_events[i++], fd.first, EVFILT_READ, EV_ADD, 0, 0, 0);
- for (const auto &sig : loop.m_signals)
-EV_SET(&in_events[i++], sig.first, EVFILT_SIGNAL, EV_ADD, 0, 0, 0);
-
- num_events = kevent(queue_id, in_events.data(), in_events.size(), out_events,
- llvm::array_lengthof(out_events), nullptr);
+ num_events = kevent(loop.m_kqueue, in_events.data(), in_events.size(),
+ out_events, llvm::array_lengthof(out_events), nullptr);
if (num_events < 0)
return Error("kevent() failed with error %d\n", num_events);
return Error();
}
-template void MainLoo
[Lldb-commits] [PATCH] D32753: MainLoop: Add unit tests
This revision was automatically updated to reflect the committed changes.
Closed by commit rL302133: MainLoop: Add unit tests (authored by labath).
Changed prior to commit:
https://reviews.llvm.org/D32753?vs=97464&id=97802#toc
Repository:
rL LLVM
https://reviews.llvm.org/D32753
Files:
lldb/trunk/include/lldb/Host/MainLoop.h
lldb/trunk/source/Host/common/MainLoop.cpp
lldb/trunk/unittests/Host/CMakeLists.txt
lldb/trunk/unittests/Host/MainLoopTest.cpp
Index: lldb/trunk/unittests/Host/MainLoopTest.cpp
===
--- lldb/trunk/unittests/Host/MainLoopTest.cpp
+++ lldb/trunk/unittests/Host/MainLoopTest.cpp
@@ -0,0 +1,120 @@
+//===-- MainLoopTest.cpp *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "lldb/Host/MainLoop.h"
+#include "lldb/Host/common/TCPSocket.h"
+#include "gtest/gtest.h"
+#include
+
+using namespace lldb_private;
+
+namespace {
+class MainLoopTest : public testing::Test {
+public:
+ static void SetUpTestCase() {
+#ifdef _MSC_VER
+WSADATA data;
+ASSERT_EQ(0, WSAStartup(MAKEWORD(2, 2), &data));
+#endif
+ }
+
+ static void TearDownTestCase() {
+#ifdef _MSC_VER
+ASSERT_EQ(0, WSACleanup());
+#endif
+ }
+
+ void SetUp() override {
+bool child_processes_inherit = false;
+Error error;
+std::unique_ptr listen_socket_up(
+new TCPSocket(true, child_processes_inherit));
+ASSERT_TRUE(error.Success());
+error = listen_socket_up->Listen("localhost:0", 5);
+ASSERT_TRUE(error.Success());
+
+Socket *accept_socket;
+std::future accept_error = std::async(std::launch::async, [&] {
+ return listen_socket_up->Accept(accept_socket);
+});
+
+std::unique_ptr connect_socket_up(
+new TCPSocket(true, child_processes_inherit));
+error = connect_socket_up->Connect(
+llvm::formatv("localhost:{0}", listen_socket_up->GetLocalPortNumber())
+.str());
+ASSERT_TRUE(error.Success());
+ASSERT_TRUE(accept_error.get().Success());
+
+callback_count = 0;
+socketpair[0] = std::move(connect_socket_up);
+socketpair[1].reset(accept_socket);
+ }
+
+ void TearDown() override {
+socketpair[0].reset();
+socketpair[1].reset();
+ }
+
+protected:
+ MainLoop::Callback make_callback() {
+return [&](MainLoopBase &loop) {
+ ++callback_count;
+ loop.RequestTermination();
+};
+ }
+ std::shared_ptr socketpair[2];
+ unsigned callback_count;
+};
+} // namespace
+
+TEST_F(MainLoopTest, ReadObject) {
+ char X = 'X';
+ size_t len = sizeof(X);
+ ASSERT_TRUE(socketpair[0]->Write(&X, len).Success());
+
+ MainLoop loop;
+
+ Error error;
+ auto handle = loop.RegisterReadObject(socketpair[1], make_callback(), error);
+ ASSERT_TRUE(error.Success());
+ ASSERT_TRUE(handle);
+ ASSERT_TRUE(loop.Run().Success());
+ ASSERT_EQ(1u, callback_count);
+}
+
+TEST_F(MainLoopTest, TerminatesImmediately) {
+ char X = 'X';
+ size_t len = sizeof(X);
+ ASSERT_TRUE(socketpair[0]->Write(&X, len).Success());
+ ASSERT_TRUE(socketpair[1]->Write(&X, len).Success());
+
+ MainLoop loop;
+ Error error;
+ auto handle0 = loop.RegisterReadObject(socketpair[0], make_callback(), error);
+ ASSERT_TRUE(error.Success());
+ auto handle1 = loop.RegisterReadObject(socketpair[1], make_callback(), error);
+ ASSERT_TRUE(error.Success());
+
+ ASSERT_TRUE(loop.Run().Success());
+ ASSERT_EQ(1u, callback_count);
+}
+
+#ifdef LLVM_ON_UNIX
+TEST_F(MainLoopTest, Signal) {
+ MainLoop loop;
+ Error error;
+
+ auto handle = loop.RegisterSignal(SIGUSR1, make_callback(), error);
+ ASSERT_TRUE(error.Success());
+ kill(getpid(), SIGUSR1);
+ ASSERT_TRUE(loop.Run().Success());
+ ASSERT_EQ(1u, callback_count);
+}
+#endif
Index: lldb/trunk/unittests/Host/CMakeLists.txt
===
--- lldb/trunk/unittests/Host/CMakeLists.txt
+++ lldb/trunk/unittests/Host/CMakeLists.txt
@@ -1,6 +1,7 @@
set (FILES
FileSpecTest.cpp
FileSystemTest.cpp
+ MainLoopTest.cpp
SocketAddressTest.cpp
SocketTest.cpp
SymbolsTest.cpp
Index: lldb/trunk/source/Host/common/MainLoop.cpp
===
--- lldb/trunk/source/Host/common/MainLoop.cpp
+++ lldb/trunk/source/Host/common/MainLoop.cpp
@@ -18,6 +18,11 @@
#include
#include
+// Multiplexing is implemented using kqueue on systems that support it (BSD
+// variants including OSX). On linux we use ppoll, while android uses pselect
+// (ppoll is present but not implemented properly). On windows we use WSApoll
+// (which does not support signals).
+
#if HAVE_SYS_EVENT_H
#include
#elif defined(LLVM_ON_WIN32)
@@ -65,92 +70,72 @@
class MainLoop::
[Lldb-commits] [lldb] r302139 - [LLDB][MIPS] Fix TestStepOverBreakpoint.py failure.
Author: nitesh.jain Date: Thu May 4 06:34:42 2017 New Revision: 302139 URL: http://llvm.org/viewvc/llvm-project?rev=302139&view=rev Log: [LLDB][MIPS] Fix TestStepOverBreakpoint.py failure. Reviewers: jingham, labath Subscribers: jaydeep, bhushan, lldb-commits, slthakur Differential Revision: https://reviews.llvm.org/D32168 Modified: lldb/trunk/include/lldb/API/SBAddress.h lldb/trunk/include/lldb/API/SBInstruction.h lldb/trunk/include/lldb/API/SBInstructionList.h lldb/trunk/include/lldb/Core/Disassembler.h lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/step_over_breakpoint/TestStepOverBreakpoint.py lldb/trunk/scripts/interface/SBInstruction.i lldb/trunk/scripts/interface/SBInstructionList.i lldb/trunk/source/API/SBAddress.cpp lldb/trunk/source/API/SBInstruction.cpp lldb/trunk/source/API/SBInstructionList.cpp lldb/trunk/source/Core/Disassembler.cpp Modified: lldb/trunk/include/lldb/API/SBAddress.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBAddress.h?rev=302139&r1=302138&r2=302139&view=diff == --- lldb/trunk/include/lldb/API/SBAddress.h (original) +++ lldb/trunk/include/lldb/API/SBAddress.h Thu May 4 06:34:42 2017 @@ -103,6 +103,8 @@ protected: const lldb_private::Address *operator->() const; + friend bool operator==(const SBAddress &lhs, const SBAddress &rhs); + lldb_private::Address *get(); lldb_private::Address &ref(); @@ -117,6 +119,8 @@ private: std::unique_ptr m_opaque_ap; }; +bool operator==(const SBAddress &lhs, const SBAddress &rhs); + } // namespace lldb #endif // LLDB_SBAddress_h_ Modified: lldb/trunk/include/lldb/API/SBInstruction.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBInstruction.h?rev=302139&r1=302138&r2=302139&view=diff == --- lldb/trunk/include/lldb/API/SBInstruction.h (original) +++ lldb/trunk/include/lldb/API/SBInstruction.h Thu May 4 06:34:42 2017 @@ -53,6 +53,8 @@ public: bool HasDelaySlot(); + bool CanSetBreakpoint(); + void Print(FILE *out); bool GetDescription(lldb::SBStream &description); Modified: lldb/trunk/include/lldb/API/SBInstructionList.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBInstructionList.h?rev=302139&r1=302138&r2=302139&view=diff == --- lldb/trunk/include/lldb/API/SBInstructionList.h (original) +++ lldb/trunk/include/lldb/API/SBInstructionList.h Thu May 4 06:34:42 2017 @@ -32,6 +32,15 @@ public: lldb::SBInstruction GetInstructionAtIndex(uint32_t idx); + // -- + // Returns the number of instructions between the start and end address. + // If canSetBreakpoint is true then the count will be the number of + // instructions on which a breakpoint can be set. + // -- + size_t GetInstructionsCount(const SBAddress &start, + const SBAddress &end, + bool canSetBreakpoint = false); + void Clear(); void AppendInstruction(lldb::SBInstruction inst); Modified: lldb/trunk/include/lldb/Core/Disassembler.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Disassembler.h?rev=302139&r1=302138&r2=302139&view=diff == --- lldb/trunk/include/lldb/Core/Disassembler.h (original) +++ lldb/trunk/include/lldb/Core/Disassembler.h Thu May 4 06:34:42 2017 @@ -173,6 +173,8 @@ public: virtual bool HasDelaySlot(); + bool CanSetBreakpoint (); + virtual size_t Decode(const Disassembler &disassembler, const DataExtractor &data, lldb::offset_t data_offset) = 0; Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/step_over_breakpoint/TestStepOverBreakpoint.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/step_over_breakpoint/TestStepOverBreakpoint.py?rev=302139&r1=302138&r2=302139&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/step_over_breakpoint/TestStepOverBreakpoint.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/step_over_breakpoint/TestStepOverBreakpoint.py Thu May 4 06:34:42 2017 @@ -62,12 +62,11 @@ class StepOverBreakpointsTestCase(TestBa instructions = function.GetInstructions(self.target) addr_1 = self.breakpoint1.GetLocationAtIndex(0).GetAddress() addr_4 = self.breakpoint4.GetLocationAtI
[Lldb-commits] [PATCH] D32168: [LLDB][MIPS] Fix TestStepOverBreakpoint.py failure
This revision was automatically updated to reflect the committed changes.
Closed by commit rL302139: [LLDB][MIPS] Fix TestStepOverBreakpoint.py failure.
(authored by nitesh.jain).
Changed prior to commit:
https://reviews.llvm.org/D32168?vs=97415&id=97811#toc
Repository:
rL LLVM
https://reviews.llvm.org/D32168
Files:
lldb/trunk/include/lldb/API/SBAddress.h
lldb/trunk/include/lldb/API/SBInstruction.h
lldb/trunk/include/lldb/API/SBInstructionList.h
lldb/trunk/include/lldb/Core/Disassembler.h
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/step_over_breakpoint/TestStepOverBreakpoint.py
lldb/trunk/scripts/interface/SBInstruction.i
lldb/trunk/scripts/interface/SBInstructionList.i
lldb/trunk/source/API/SBAddress.cpp
lldb/trunk/source/API/SBInstruction.cpp
lldb/trunk/source/API/SBInstructionList.cpp
lldb/trunk/source/Core/Disassembler.cpp
Index: lldb/trunk/source/API/SBInstructionList.cpp
===
--- lldb/trunk/source/API/SBInstructionList.cpp
+++ lldb/trunk/source/API/SBInstructionList.cpp
@@ -9,6 +9,7 @@
#include "lldb/API/SBInstructionList.h"
#include "lldb/API/SBInstruction.h"
+#include "lldb/API/SBAddress.h"
#include "lldb/API/SBStream.h"
#include "lldb/Core/Disassembler.h"
#include "lldb/Core/Module.h"
@@ -49,6 +50,31 @@
return inst;
}
+size_t SBInstructionList::GetInstructionsCount(const SBAddress &start,
+ const SBAddress &end,
+ bool canSetBreakpoint) {
+ size_t num_instructions = GetSize();
+ size_t i = 0;
+ SBAddress addr;
+ size_t lower_index = 0;
+ size_t upper_index = 0;
+ size_t instructions_to_skip = 0;
+ for (i = 0; i < num_instructions; ++i) {
+addr = GetInstructionAtIndex(i).GetAddress();
+if (start == addr)
+ lower_index = i;
+if (end == addr)
+ upper_index = i;
+ }
+ if (canSetBreakpoint)
+for (i = lower_index; i <= upper_index; ++i) {
+ SBInstruction insn = GetInstructionAtIndex(i);
+ if (!insn.CanSetBreakpoint())
+++instructions_to_skip;
+}
+ return upper_index - lower_index - instructions_to_skip;
+}
+
void SBInstructionList::Clear() { m_opaque_sp.reset(); }
void SBInstructionList::AppendInstruction(SBInstruction insn) {}
Index: lldb/trunk/source/API/SBAddress.cpp
===
--- lldb/trunk/source/API/SBAddress.cpp
+++ lldb/trunk/source/API/SBAddress.cpp
@@ -55,6 +55,12 @@
return *this;
}
+bool lldb::operator==(const SBAddress &lhs, const SBAddress &rhs) {
+ if (lhs.IsValid() && rhs.IsValid())
+return lhs.ref() == rhs.ref();
+ return false;
+}
+
bool SBAddress::IsValid() const {
return m_opaque_ap.get() != NULL && m_opaque_ap->IsValid();
}
Index: lldb/trunk/source/API/SBInstruction.cpp
===
--- lldb/trunk/source/API/SBInstruction.cpp
+++ lldb/trunk/source/API/SBInstruction.cpp
@@ -176,6 +176,13 @@
return false;
}
+bool SBInstruction::CanSetBreakpoint () {
+ lldb::InstructionSP inst_sp(GetOpaque());
+ if (inst_sp)
+return inst_sp->CanSetBreakpoint();
+ return false;
+}
+
lldb::InstructionSP SBInstruction::GetOpaque() {
if (m_opaque_sp)
return m_opaque_sp->GetSP();
Index: lldb/trunk/source/Core/Disassembler.cpp
===
--- lldb/trunk/source/Core/Disassembler.cpp
+++ lldb/trunk/source/Core/Disassembler.cpp
@@ -759,6 +759,10 @@
return false;
}
+bool Instruction::CanSetBreakpoint () {
+ return !HasDelaySlot();
+}
+
bool Instruction::HasDelaySlot() {
// Default is false.
return false;
Index: lldb/trunk/scripts/interface/SBInstruction.i
===
--- lldb/trunk/scripts/interface/SBInstruction.i
+++ lldb/trunk/scripts/interface/SBInstruction.i
@@ -54,6 +54,9 @@
bool
HasDelaySlot ();
+bool
+CanSetBreakpoint ();
+
void
Print (FILE *out);
Index: lldb/trunk/scripts/interface/SBInstructionList.i
===
--- lldb/trunk/scripts/interface/SBInstructionList.i
+++ lldb/trunk/scripts/interface/SBInstructionList.i
@@ -44,6 +44,9 @@
lldb::SBInstruction
GetInstructionAtIndex (uint32_t idx);
+size_t GetInstructionsCount(const SBAddress &start, const SBAddress &end,
+bool canSetBreakpoint);
+
void
Clear ();
Index: lldb/trunk/include/lldb/Core/Disassembler.h
===
--- lldb/trunk/include/lldb/Core/Disassembler.h
+++ lldb/trunk/include/lldb/Core/Disassembler.h
@@ -173,6 +173,8 @@
virtual bool HasDelaySlot();
+ bool CanSetBreakpoint ();
+
virtual size_t Decode(const Disassembler &disassembler,
const DataExtractor
[Lldb-commits] [PATCH] D32813: ABISysV_arm64: compute return value for large vectors correctly
tberghammer added a comment. I am a bit confused by the correlation between your change and commit message. In the commit message you say that 32 byte structs are passed as x8 pointers but the implementation of LoadValueFromConsecutiveGPRRegisters seems to read it out from the v0-v8 registers for vectors of up to 8 elements independently of there size. Also based on that code I have the suspicion that the first branch (where byte_size <= 16) is not actually used or necessary and also I don't see anything in the ABI documentation indicating otherwise (it would be a pretty crazy ABI if they say that if you have 4 double then passed in a single 32 byte register while if you have 8 double then passed in 8 different 32 byte registers). Can you make sure that branch is necessary (e.g. removing it breaks at least 1 test)? Comment at: packages/Python/lldbsuite/test/functionalities/return-value/TestReturnValue.py:194 +exe = os.path.join(os.getcwd(), "a.out") +error = lldb.SBError() + (nit): Not needed https://reviews.llvm.org/D32813 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32813: ABISysV_arm64: compute return value for large vectors correctly
labath added a comment. In https://reviews.llvm.org/D32813#746012, @tberghammer wrote: > I am a bit confused by the correlation between your change and commit > message. In the commit message you say that 32 byte structs I mean 32-byte vectors. I.e. variables declared as `float foo __attribute__((__vector_size__(32)));` > are passed as x8 pointers but the implementation of > LoadValueFromConsecutiveGPRRegisters seems to read it out from the v0-v8 > registers for vectors of up to 8 elements independently of there size. LoadValueFromConsecutiveGPRRegisters does this for "homogeneous structs", which is a different concept than vector: """Note that for short-vector types the fundamental types are 64-bit vector and 128-bit vector; the type of the elements in the short vector does not form part of the test for homogeneity. """ So an 8-byte and 16-byte vector (and probably structures containing them) are passed in v0..v7 registers. However, a 32-byte vector is not a short-vector type, nor a homogeneous aggregate, so it is passed as a generic struct, via the v8 pointer. Also based on that code I have the suspicion that the first branch (where byte_size <= 16) is not actually used or necessary and also I don't see anything in the ABI documentation indicating otherwise (it would be a pretty crazy ABI if they say that if you have 4 double then passed in a single 32 byte register while if you have 8 double then passed in 8 different 32 byte registers). Can you make sure that branch is necessary (e.g. removing it breaks at least 1 test)? Removing the branch makes the test for 8 and 16-byte vectors fail. https://reviews.llvm.org/D32813 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32866: Fix -DLLVM_BUILD_TESTS=ON lldb build
jroelofs created this revision.
Herald added a subscriber: mgorny.
There are two issues here. The first:
liblldbHost.a(Editline.cpp.o):(.debug_addr+0x238): undefined reference to
`el_get'
when linking InterpreterTests, is caused by -ledit coming after liblldbHost.a
in the link line.
The second:
liblldbCore.a(IOHandler.cpp.o):(.debug_addr+0x2564): undefined reference to
`del_panel'
because libpanel.a and liblldbCore.a are out of order in the link line.
https://reviews.llvm.org/D32866
Files:
source/Core/CMakeLists.txt
source/Host/CMakeLists.txt
Index: source/Host/CMakeLists.txt
===
--- source/Host/CMakeLists.txt
+++ source/Host/CMakeLists.txt
@@ -48,6 +48,7 @@
add_host_subdirectory(common
common/Editline.cpp
)
+ list(APPEND EXTRA_LIBS edit)
endif()
add_host_subdirectory(posix
@@ -160,7 +161,7 @@
endif()
if (CMAKE_SYSTEM_NAME MATCHES "NetBSD")
- set(EXTRA_LIBS kvm)
+ list(APPEND EXTRA_LIBS kvm)
endif ()
add_lldb_library(lldbHost
Index: source/Core/CMakeLists.txt
===
--- source/Core/CMakeLists.txt
+++ source/Core/CMakeLists.txt
@@ -69,6 +69,7 @@
LINK_COMPONENTS
Support
Demangle
+${LLDB_SYSTEM_LIBS}
)
# Needed to properly resolve references in a debug build.
Index: source/Host/CMakeLists.txt
===
--- source/Host/CMakeLists.txt
+++ source/Host/CMakeLists.txt
@@ -48,6 +48,7 @@
add_host_subdirectory(common
common/Editline.cpp
)
+ list(APPEND EXTRA_LIBS edit)
endif()
add_host_subdirectory(posix
@@ -160,7 +161,7 @@
endif()
if (CMAKE_SYSTEM_NAME MATCHES "NetBSD")
- set(EXTRA_LIBS kvm)
+ list(APPEND EXTRA_LIBS kvm)
endif ()
add_lldb_library(lldbHost
Index: source/Core/CMakeLists.txt
===
--- source/Core/CMakeLists.txt
+++ source/Core/CMakeLists.txt
@@ -69,6 +69,7 @@
LINK_COMPONENTS
Support
Demangle
+${LLDB_SYSTEM_LIBS}
)
# Needed to properly resolve references in a debug build.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32866: Fix -DLLVM_BUILD_TESTS=ON lldb build
krytarowski added a comment. NetBSD change looks fine. https://reviews.llvm.org/D32866 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32866: Fix -DLLVM_BUILD_TESTS=ON lldb build
jroelofs planned changes to this revision. jroelofs added a comment. hmm. I'm getting more libpanel link issues in another build. https://reviews.llvm.org/D32866 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32866: Fix -DLLVM_BUILD_TESTS=ON lldb build
krytarowski added a comment. FYI, NetBSD has its distinct libpanel in its base system, API compatible with ncurses. https://reviews.llvm.org/D32866 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32866: Fix -DLLVM_BUILD_TESTS=ON lldb build
jroelofs updated this revision to Diff 97842.
https://reviews.llvm.org/D32866
Files:
source/Core/CMakeLists.txt
source/Host/CMakeLists.txt
Index: source/Host/CMakeLists.txt
===
--- source/Host/CMakeLists.txt
+++ source/Host/CMakeLists.txt
@@ -48,6 +48,7 @@
add_host_subdirectory(common
common/Editline.cpp
)
+ list(APPEND EXTRA_LIBS edit)
endif()
add_host_subdirectory(posix
@@ -160,12 +161,12 @@
endif()
if (CMAKE_SYSTEM_NAME MATCHES "NetBSD")
- set(EXTRA_LIBS kvm)
+ list(APPEND EXTRA_LIBS kvm)
endif ()
add_lldb_library(lldbHost
${HOST_SOURCES}
-
+
LINK_LIBS
lldbCore
lldbInterpreter
@@ -174,7 +175,8 @@
lldbUtility
${LLDB_PLUGINS}
${EXTRA_LIBS}
-
+
LINK_COMPONENTS
Support
)
+
Index: source/Core/CMakeLists.txt
===
--- source/Core/CMakeLists.txt
+++ source/Core/CMakeLists.txt
@@ -1,3 +1,5 @@
+include(${LLDB_PROJECT_ROOT}/cmake/LLDBDependencies.cmake)
+
add_lldb_library(lldbCore
Address.cpp
AddressRange.cpp
@@ -65,6 +67,7 @@
lldbPluginCPlusPlusLanguage
lldbPluginObjCLanguage
lldbPluginObjectFileJIT
+${LLDB_SYSTEM_LIBS}
LINK_COMPONENTS
Support
Index: source/Host/CMakeLists.txt
===
--- source/Host/CMakeLists.txt
+++ source/Host/CMakeLists.txt
@@ -48,6 +48,7 @@
add_host_subdirectory(common
common/Editline.cpp
)
+ list(APPEND EXTRA_LIBS edit)
endif()
add_host_subdirectory(posix
@@ -160,12 +161,12 @@
endif()
if (CMAKE_SYSTEM_NAME MATCHES "NetBSD")
- set(EXTRA_LIBS kvm)
+ list(APPEND EXTRA_LIBS kvm)
endif ()
add_lldb_library(lldbHost
${HOST_SOURCES}
-
+
LINK_LIBS
lldbCore
lldbInterpreter
@@ -174,7 +175,8 @@
lldbUtility
${LLDB_PLUGINS}
${EXTRA_LIBS}
-
+
LINK_COMPONENTS
Support
)
+
Index: source/Core/CMakeLists.txt
===
--- source/Core/CMakeLists.txt
+++ source/Core/CMakeLists.txt
@@ -1,3 +1,5 @@
+include(${LLDB_PROJECT_ROOT}/cmake/LLDBDependencies.cmake)
+
add_lldb_library(lldbCore
Address.cpp
AddressRange.cpp
@@ -65,6 +67,7 @@
lldbPluginCPlusPlusLanguage
lldbPluginObjCLanguage
lldbPluginObjectFileJIT
+${LLDB_SYSTEM_LIBS}
LINK_COMPONENTS
Support
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32421: Fix segfault resulting from empty print prompt
xiaobai added a comment. I do not have commit access. I would appreciate it if you could commit it for me. https://reviews.llvm.org/D32421 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32732: "target variable" not showing all global variable if we print any global variable before execution starts
jingham added a comment. Yes, I was printing a global variable that WASN'T defined in the source file I later stopped in. So it worked for me. The bug is really that looking up a variable by name goes through and adds only that variable to the CU that defines it, but the CU doesn't make note of the fact that it didn't actually fully populate the m_variables for the CU. So then later on when you ask for them, we think we got them all, which of course we haven't. The fix needs to be in CompileUnit. It shouldn't use m_variables.get to tell itself that it has finished getting variables for the CU. It has to keep another flag telling itself that it has gotten ALL the variables defined in the comp unit, and if that's false, it should keep adding. Repository: rL LLVM https://reviews.llvm.org/D32732 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32823: Remove an expensive lock from Timer
scott.smith marked 3 inline comments as done.
scott.smith added a comment.
In https://reviews.llvm.org/D32823#745799, @labath wrote:
> Seems reasonable, however: I am not sure who actually uses these timers. I'd
> be tempted to just remove the timers that are causing the contention.
IMO we should either keep the timers and make them fast, or get rid of all
timers. I don't like the idea of training developers to know that timers are
slow, and should be used sparingly. Especially since the fix is relatively
simple.
Though an atomic skip list would be another way to improve performance without
requiring changing each callsite; the downside is the lg-n lookup when tallying
the time.
Comment at: source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:5421
char *buf = (char *) malloc (ident_command.cmdsize);
-if (buf != nullptr
+if (buf != nullptr
&& m_data.CopyData (offset, ident_command.cmdsize, buf) ==
ident_command.cmdsize) {
labath wrote:
> I am not sure how you format your changes, but you should make sure you
> format only the lines you've touched, and not the whole files.
> git-clang-format
>
> will do that for you -- when you set it up, you just run `git clang-format
> HEAD^` and it will format your last patch).
Yeah that'll be helpful. I have default .emacs settings for work that don't
necessarily apply well to the llvm codebase, this tool will make it easier to
fix up.
Repository:
rL LLVM
https://reviews.llvm.org/D32823
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D32823: Remove an expensive lock from Timer
I'd vote for keeping the timers if possible. Their job is to tell you "of the
time spent in some operation, how much was spent in say DWARF parsing", etc.
That has been useful on occasion.
Jim
> On May 4, 2017, at 2:12 PM, Scott Smith via Phabricator via lldb-commits
> wrote:
>
> scott.smith marked 3 inline comments as done.
> scott.smith added a comment.
>
> In https://reviews.llvm.org/D32823#745799, @labath wrote:
>
>> Seems reasonable, however: I am not sure who actually uses these timers. I'd
>> be tempted to just remove the timers that are causing the contention.
>
>
> IMO we should either keep the timers and make them fast, or get rid of all
> timers. I don't like the idea of training developers to know that timers are
> slow, and should be used sparingly. Especially since the fix is relatively
> simple.
>
> Though an atomic skip list would be another way to improve performance
> without requiring changing each callsite; the downside is the lg-n lookup
> when tallying the time.
>
>
>
>
> Comment at: source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:5421
> char *buf = (char *) malloc (ident_command.cmdsize);
> -if (buf != nullptr
> +if (buf != nullptr
> && m_data.CopyData (offset, ident_command.cmdsize, buf) ==
> ident_command.cmdsize) {
>
> labath wrote:
>> I am not sure how you format your changes, but you should make sure you
>> format only the lines you've touched, and not the whole files.
>> git-clang-format
>>
>> will do that for you -- when you set it up, you just run `git clang-format
>> HEAD^` and it will format your last patch).
> Yeah that'll be helpful. I have default .emacs settings for work that don't
> necessarily apply well to the llvm codebase, this tool will make it easier to
> fix up.
>
>
> Repository:
> rL LLVM
>
> https://reviews.llvm.org/D32823
>
>
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32823: Remove an expensive lock from Timer
scott.smith updated this revision to Diff 97880.
scott.smith marked an inline comment as done.
scott.smith added a comment.
updated per review comments.
added a test to fix the Jurassic Park problem.
Repository:
rL LLVM
https://reviews.llvm.org/D32823
Files:
include/lldb/Core/Timer.h
source/API/SystemInitializerFull.cpp
source/Commands/CommandObjectTarget.cpp
source/Core/Disassembler.cpp
source/Core/Mangled.cpp
source/Core/Module.cpp
source/Core/Timer.cpp
source/Host/common/Symbols.cpp
source/Initialization/SystemInitializerCommon.cpp
source/Interpreter/CommandInterpreter.cpp
source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp
source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.cpp
source/Plugins/SymbolFile/DWARF/DWARFDebugLine.cpp
source/Plugins/SymbolFile/DWARF/DWARFDebugPubnames.cpp
source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
source/Symbol/DWARFCallFrameInfo.cpp
source/Symbol/ObjectFile.cpp
source/Symbol/Symtab.cpp
source/Target/Target.cpp
source/Target/TargetList.cpp
unittests/Core/TimerTest.cpp
Index: unittests/Core/TimerTest.cpp
===
--- unittests/Core/TimerTest.cpp
+++ unittests/Core/TimerTest.cpp
@@ -18,7 +18,8 @@
TEST(TimerTest, CategoryTimes) {
Timer::ResetCategoryTimes();
{
-Timer t("CAT1", "");
+static Timer::Category tcat("CAT1");
+Timer t(tcat, "");
std::this_thread::sleep_for(std::chrono::milliseconds(10));
}
StreamString ss;
@@ -32,25 +33,32 @@
TEST(TimerTest, CategoryTimesNested) {
Timer::ResetCategoryTimes();
{
-Timer t1("CAT1", "");
+static Timer::Category tcat1a("CAT1");
+Timer t1(tcat1a, "");
std::this_thread::sleep_for(std::chrono::milliseconds(10));
-Timer t2("CAT1", "");
+// Explicitly testing the same category as above.
+static Timer::Category tcat1b("CAT1");
+Timer t2(tcat1b, "");
std::this_thread::sleep_for(std::chrono::milliseconds(10));
}
StreamString ss;
Timer::DumpCategoryTimes(&ss);
double seconds;
+ // It should only appear once.
+ ASSERT_EQ(nullptr, strstr(strstr(ss.GetData(), "CAT1") + 1, "CAT1"));
ASSERT_EQ(1, sscanf(ss.GetData(), "%lf sec for CAT1", &seconds));
EXPECT_LT(0.002, seconds);
EXPECT_GT(0.2, seconds);
}
TEST(TimerTest, CategoryTimes2) {
Timer::ResetCategoryTimes();
{
-Timer t1("CAT1", "");
+static Timer::Category tcat1("CAT1");
+Timer t1(tcat1, "");
std::this_thread::sleep_for(std::chrono::milliseconds(100));
-Timer t2("CAT2", "");
+static Timer::Category tcat2("CAT2");
+Timer t2(tcat2, "");
std::this_thread::sleep_for(std::chrono::milliseconds(10));
}
StreamString ss;
Index: source/Target/TargetList.cpp
===
--- source/Target/TargetList.cpp
+++ source/Target/TargetList.cpp
@@ -325,10 +325,10 @@
lldb::PlatformSP &platform_sp,
lldb::TargetSP &target_sp,
bool is_dummy_target) {
- Timer scoped_timer(LLVM_PRETTY_FUNCTION,
- "TargetList::CreateTarget (file = '%s', arch = '%s')",
- user_exe_path.str().c_str(),
- specified_arch.GetArchitectureName());
+ static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
+ Timer scoped_timer(
+ func_cat, "TargetList::CreateTarget (file = '%s', arch = '%s')",
+ user_exe_path.str().c_str(), specified_arch.GetArchitectureName());
Error error;
ArchSpec arch(specified_arch);
Index: source/Target/Target.cpp
===
--- source/Target/Target.cpp
+++ source/Target/Target.cpp
@@ -1235,7 +1235,8 @@
ClearModules(false);
if (executable_sp) {
-Timer scoped_timer(LLVM_PRETTY_FUNCTION,
+static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
+Timer scoped_timer(func_cat,
"Target::SetExecutableModule (executable = '%s')",
executable_sp->GetFileSpec().GetPath().c_str());
Index: source/Symbol/Symtab.cpp
===
--- source/Symbol/Symtab.cpp
+++ source/Symbol/Symtab.cpp
@@ -220,7 +220,8 @@
// Protected function, no need to lock mutex...
if (!m_name_indexes_computed) {
m_name_indexes_computed = true;
-Timer scoped_timer(LLVM_PRETTY_FUNCTION, "%s", LLVM_PRETTY_FUNCTION);
+static Timer::Categ
[Lldb-commits] [PATCH] D32757: Add TaskMap for iterating a function over a set of integers
scott.smith added a comment. In https://reviews.llvm.org/D32757#743793, @zturner wrote: > Not to sound like a broken record, but please try to put this in LLVM instead > of LLVM. I suggested a convenient function signature earlier. @zturner ok to commit? TaskMapOverInt(x, y, fn) maps directly to parallel_for(0, x, fn). Rather than rebundle the change you have for lldb, only for it to be deleted once you get it into llvm, can we just commit this as a stopgap? It is a step in the right direction as it removes TaskRunner and puts us on an API more likely to end up in LLVM. Repository: rL LLVM https://reviews.llvm.org/D32757 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32757: Add TaskMap for iterating a function over a set of integers
scott.smith updated this revision to Diff 97907.
scott.smith added a comment.
1. Change the API to more closely match parallel_for (begin, end, fn) instead
of (end, batch, fn).
2. Fix unit test. I didn't realize I had to run check-lldb-unit separately
from dotest (oops).
3. Fix formatting via git-clang-format.
Repository:
rL LLVM
https://reviews.llvm.org/D32757
Files:
include/lldb/Utility/TaskPool.h
source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
source/Utility/TaskPool.cpp
unittests/Utility/TaskPoolTest.cpp
Index: unittests/Utility/TaskPoolTest.cpp
===
--- unittests/Utility/TaskPoolTest.cpp
+++ unittests/Utility/TaskPoolTest.cpp
@@ -30,25 +30,14 @@
ASSERT_EQ(17, r[3]);
}
-TEST(TaskPoolTest, TaskRunner) {
- auto fn = [](int x) { return std::make_pair(x, x * x); };
-
- TaskRunner> tr;
- tr.AddTask(fn, 1);
- tr.AddTask(fn, 2);
- tr.AddTask(fn, 3);
- tr.AddTask(fn, 4);
-
- int count = 0;
- while (true) {
-auto f = tr.WaitForNextCompletedTask();
-if (!f.valid())
- break;
-
-++count;
-std::pair v = f.get();
-ASSERT_EQ(v.first * v.first, v.second);
- }
-
- ASSERT_EQ(4, count);
+TEST(TaskPoolTest, TaskMap) {
+ int data[4];
+ auto fn = [&data](int x) { data[x] = x * x; };
+
+ TaskMapOverInt(0, 4, fn);
+
+ ASSERT_EQ(data[0], 0);
+ ASSERT_EQ(data[1], 1);
+ ASSERT_EQ(data[2], 4);
+ ASSERT_EQ(data[3], 9);
}
Index: source/Utility/TaskPool.cpp
===
--- source/Utility/TaskPool.cpp
+++ source/Utility/TaskPool.cpp
@@ -73,3 +73,26 @@
f();
}
}
+
+void TaskMapOverInt(size_t begin, size_t end,
+std::function const &func) {
+ std::atomic idx{begin};
+ size_t num_workers =
+ std::min(end, std::thread::hardware_concurrency());
+
+ auto wrapper = [&idx, end, &func]() {
+while (true) {
+ size_t i = idx.fetch_add(1);
+ if (i >= end)
+break;
+ func(i);
+}
+ };
+
+ std::vector> futures;
+ futures.reserve(num_workers);
+ for (size_t i = 0; i < num_workers; i++)
+futures.push_back(TaskPool::AddTask(wrapper));
+ for (size_t i = 0; i < num_workers; i++)
+futures[i].wait();
+}
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1946,7 +1946,9 @@
std::vector type_index(num_compile_units);
std::vector namespace_index(num_compile_units);
-std::vector clear_cu_dies(num_compile_units, false);
+// std::vector might be implemented using bit test-and-set, so use
+// uint8_t instead.
+std::vector clear_cu_dies(num_compile_units, false);
auto parser_fn = [debug_info, &function_basename_index,
&function_fullname_index, &function_method_index,
&function_selector_index, &objc_class_selectors_index,
@@ -1963,22 +1965,18 @@
return cu_idx;
};
-auto extract_fn = [debug_info](uint32_t cu_idx) {
+auto extract_fn = [debug_info, &clear_cu_dies](uint32_t cu_idx) {
DWARFCompileUnit *dwarf_cu = debug_info->GetCompileUnitAtIndex(cu_idx);
if (dwarf_cu) {
// dwarf_cu->ExtractDIEsIfNeeded(false) will return zero if the
// DIEs for a compile unit have already been parsed.
-return std::make_pair(cu_idx, dwarf_cu->ExtractDIEsIfNeeded(false) > 1);
+if (dwarf_cu->ExtractDIEsIfNeeded(false) > 1)
+ clear_cu_dies[cu_idx] = true;
}
- return std::make_pair(cu_idx, false);
};
// Create a task runner that extracts dies for each DWARF compile unit in a
// separate thread
-TaskRunner> task_runner_extract;
-for (uint32_t cu_idx = 0; cu_idx < num_compile_units; ++cu_idx)
- task_runner_extract.AddTask(extract_fn, cu_idx);
-
//--
// First figure out which compile units didn't have their DIEs already
// parsed and remember this. If no DIEs were parsed prior to this index
@@ -1988,48 +1986,37 @@
// a DIE in one compile unit refers to another and the indexes accesses
// those DIEs.
//--
-while (true) {
- auto f = task_runner_extract.WaitForNextCompletedTask();
- if (!f.valid())
-break;
- unsigned cu_idx;
- bool clear;
- std::tie(cu_idx, clear) = f.get();
- clear_cu_dies[cu_idx] = clear;
-}
+TaskMapOverInt(0, num_compile_units, extract_fn);
// Now create a task runner that can index each DWARF compile unit in a
// separate
// thread so we can index quickly.
-TaskRunner task_runner;
-for (uint32_t cu_idx = 0; cu_idx < num_compile_units; ++cu_idx)
- task_runner.AddTask(parser_fn, cu_idx);
[Lldb-commits] [PATCH] D32757: Add TaskMap for iterating a function over a set of integers
scott.smith added a comment. Last changes are cosmetic (though look big because I captured a different amount of context) so hopefully doesn't need a re-review. Can someone push them for me? Thank you! Repository: rL LLVM https://reviews.llvm.org/D32757 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32597: Initiate loading of shared libraries in parallel
scott.smith added a subscriber: ruiu.
scott.smith added inline comments.
Comment at:
source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp:530-560
+ struct loader {
+DYLDRendezvous::iterator I;
+ModuleSP m;
+std::shared_future f;
+ };
+ std::vector loaders(num_to_load);
+ llvm::ThreadPool load_pool(
zturner wrote:
> scott.smith wrote:
> > zturner wrote:
> > > zturner wrote:
> > > > I'm still unclear why we're still going down this route of adding a
> > > > very specialized instance of parallelism to this one place, that is
> > > > probably going to be less efficient than having a global thread pool
> > > > (as now you will have to spin up and destroy the entire thread pool
> > > > every time you go to load shared libraries).
> > > >
> > > > So, rather than keep repeating the same thing over and over, I went and
> > > > looked into this a little bit. It turns out LLD has a large library of
> > > > parallel algorithms in `lld/Include/lld/Parallel.h`. There is almost
> > > > nothing specific to LLD in any of these algorithms however, and they
> > > > should probably be moved to llvm support. Upon doing so, you will be
> > > > able to write this code as:
> > > >
> > > > ```
> > > > std::vector Modules(m_rendevous.size());
> > > > llvm::parallel_for_each(make_integer_range(0, m_rendevous.size()),
> > > > [this, &Modules](int I)
> > > > {
> > > > auto &R = m_rendevous[I];
> > > > Modules[I] = LoadModuleAtAddress(R.file_spec, R.link_addr,
> > > > R.base_addr, true);
> > > > });
> > > > for (const auto &M : Modules)
> > > >module_list.Append(M);
> > > > ```
> > > >
> > > > Please look into making this change.
> > > Note that this is, of course, exactly why I've been saying all along that
> > > we should not even be discussing this here. Had the discussion happened
> > > over on the LLVM side the first time I asked about this, someone could
> > > have mentioned this without me having to spend time looking into it.
> > As long as those routines allow you to recursively call those routines,
> > then great, go for it.
> >
> > The spinning up/tearing down of threads is not expensive, at least in my
> > measured use case.
> > As long as those routines allow you to recursively call those routines,
> > then great, go for it.
>
> Right, which is why I've asked at least 4 times for it to be looked into.
> That doesn't mean mean use the first thing you see and check it in right
> away, it means please see if there is some way to use existing code to solve
> the problem. If it turns out this doesn't allow recursive use, then a
> followup would be to understand why not, and if it is hard to change.
>
> So again, please look into this and see if it will work for your needs.
It seems there are two possible long term fixes:
1. Separate thread pool
2. Make parallel_for support recursive use.
Personally I'm in favor of 2. @ruiu brought it up as a preferred goal too.
The q is then how to proceed in the short term:
1. Wait for parallel_for to land in llvm, make it support recursion (hopefully
Windows ConcRT supports it too?), then convert this change and commit.
2. Change TaskMapOverInt to take a ThreadPool, then have it use a private
ThreadPool for now, then easily convert to the #1 above later.
3. Change TaskMapOverInt to support recursion now (which will also require
changes to lldb::TaskPool, IMO), then do #1 above once it lands. Advantage is
we prove it works, which hopefully makes moving it upstream easier.
Any preferences on how to proceed?
Repository:
rL LLVM
https://reviews.llvm.org/D32597
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
