[Lldb-commits] [PATCH] D41725: [lldb] [test] Fix missing HAVE_LIBZ for tests in stand-alone builds
labath added a comment. Maybe we could just change the lit script to use the value of `LLVM_ENABLE_ZLIB` as the trigger? That should work in both standalone and non-standalone builds (and it seems more clean -- I'm not sure why llvm does not do the same)? https://reviews.llvm.org/D41725 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D41726: [test] Use full PATH lookup for tools
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. lgtm, thanks. https://reviews.llvm.org/D41726 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D41702: Add SysV Abi for PPC64le
labath added a comment. Is the only difference between ppc64 and ppc64le ABIs in the endianness of the values? If so, could we make one unified ABI which takes the endianness as an argument (in the constructor, or as a template argument, or deduces it from target endiannes, ...) ? https://reviews.llvm.org/D41702 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D41725: [lldb] [test] Fix missing HAVE_LIBZ for tests in stand-alone builds
mgorny added a comment. Maybe we could. I presumed people are using the separate variables for some reason. We'd have to check that it's properly sanitized when building in-tree and given to cmake though. https://reviews.llvm.org/D41725 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D41745: Handle O reply packets during qRcmd
labath added a comment. The ProcessGdbRemote part is not really testable in it's current form, but for the rest, you should be able to write a gdbremoteclient unittest (see unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp). Comment at: source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h:53 + StringExtractorGDBRemote &response, + std::function output_callback); + this could be an llvm::function_ref (more lightweight than std::function). Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:5157 Stream &output_strm = result.GetOutputStream(); + process->GetGDBRemote().SendPacketHandlingOutput( + packet.GetString(), response, I think the API would be cleaner if the output callback was an argument to the standard SendPacket function, where the (default) value of null would mean "don't do anything special with the O packets", although that might just be a personal preference. However, even if we choose to have separate functions as the API, I believe they should delegate to a single unified implementation internally. https://reviews.llvm.org/D41745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D41725: [lldb] [test] Fix missing HAVE_LIBZ for tests in stand-alone builds
labath added a comment. In https://reviews.llvm.org/D41725#969183, @mgorny wrote: > Maybe we could. I presumed people are using the separate variables for some > reason. We'd have to check that it's properly sanitized when building in-tree > and given to cmake though. I would expect that reason is purely historical. While working on that patch I got the feeling that there is a lot of confusion in how to check for zlib presence. All everyone wants to know is whether zlib is present and working, so there should not be a need for different checks. We certainly don't need anything fancy here. https://reviews.llvm.org/D41725 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D41725: [lldb] [test] Fix missing HAVE_LIBZ for tests in stand-alone builds
mgorny added a comment. Well, it's actually more complex. In Gentoo we want to let the user choose whether he wants to build against zlib or without zlib support. So in the end there are two uses to be considered: 1. When the value is merely passed down from LLVM to indicate state of compression support in LLVM. AFAIR that was the case with clang -- we merely wanted to know if we can run tests that require LLVM to have compression support. 2. When the value is actually used to use zlib locally. To be honest, in this case I believe we should have a separate check & switch. FWICS, this is the case with LLDB. So I don't think using LLVM_ENABLE_ZLIB here is correct, and my patch isn't correct as well. That said, is there a reason for LLDB to use zlib directly instead of using the LLVM support compression library? https://reviews.llvm.org/D41725 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r321932 - [test] Use full PATH lookup for tools
Author: mgorny Date: Sat Jan 6 02:20:25 2018 New Revision: 321932 URL: http://llvm.org/viewvc/llvm-project?rev=321932&view=rev Log: [test] Use full PATH lookup for tools Use full PATH when looking up test tools rather than just llvm tools directory. r320813 has added a lookup for 'lldb-test' which is part of LLDB tools rather than LLVM, and therefore is not present in llvm_tools_dir before LLDB is installed. While technically we could introduce separate per-directory lookup logic, there is no real reason not to use the PATH formed earlier here, and this is what other tools are doing. Differential Revision: https://reviews.llvm.org/D41726 Modified: lldb/trunk/lit/lit.cfg Modified: lldb/trunk/lit/lit.cfg URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/lit.cfg?rev=321932&r1=321931&r2=321932&view=diff == --- lldb/trunk/lit/lit.cfg (original) +++ lldb/trunk/lit/lit.cfg Sat Jan 6 02:20:25 2018 @@ -91,11 +91,11 @@ for pattern in [r"\bFileCheck\b", pattern) tool_pipe = tool_match.group(2) tool_name = tool_match.group(4) -tool_path = lit.util.which(tool_name, config.llvm_tools_dir) +tool_path = lit.util.which(tool_name, config.environment['PATH']) if not tool_path: # Warn, but still provide a substitution. lit_config.note( -'Did not find ' + tool_name + ' in ' + config.llvm_tools_dir) +'Did not find ' + tool_name + ' in ' + config.environment['PATH']) config.substitutions.append((pattern, tool_pipe + tool_path)) # Shell execution ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D41726: [test] Use full PATH lookup for tools
This revision was automatically updated to reflect the committed changes. Closed by commit rL321932: [test] Use full PATH lookup for tools (authored by mgorny, committed by ). Changed prior to commit: https://reviews.llvm.org/D41726?vs=128609&id=128843#toc Repository: rL LLVM https://reviews.llvm.org/D41726 Files: lldb/trunk/lit/lit.cfg Index: lldb/trunk/lit/lit.cfg === --- lldb/trunk/lit/lit.cfg +++ lldb/trunk/lit/lit.cfg @@ -91,11 +91,11 @@ pattern) tool_pipe = tool_match.group(2) tool_name = tool_match.group(4) -tool_path = lit.util.which(tool_name, config.llvm_tools_dir) +tool_path = lit.util.which(tool_name, config.environment['PATH']) if not tool_path: # Warn, but still provide a substitution. lit_config.note( -'Did not find ' + tool_name + ' in ' + config.llvm_tools_dir) +'Did not find ' + tool_name + ' in ' + config.environment['PATH']) config.substitutions.append((pattern, tool_pipe + tool_path)) # Shell execution Index: lldb/trunk/lit/lit.cfg === --- lldb/trunk/lit/lit.cfg +++ lldb/trunk/lit/lit.cfg @@ -91,11 +91,11 @@ pattern) tool_pipe = tool_match.group(2) tool_name = tool_match.group(4) -tool_path = lit.util.which(tool_name, config.llvm_tools_dir) +tool_path = lit.util.which(tool_name, config.environment['PATH']) if not tool_path: # Warn, but still provide a substitution. lit_config.note( -'Did not find ' + tool_name + ' in ' + config.llvm_tools_dir) +'Did not find ' + tool_name + ' in ' + config.environment['PATH']) config.substitutions.append((pattern, tool_pipe + tool_path)) # Shell execution ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D41702: Add SysV Abi for PPC64le
hfinkel added a comment. In https://reviews.llvm.org/D41702#969179, @labath wrote: > Is the only difference between ppc64 and ppc64le ABIs in the endianness of > the values? > If so, could we make one unified ABI which takes the endianness as an > argument (in the constructor, or as a template argument, or deduces it from > target endiannes, ...) ? The ABIs have some other differences. The largest difference between the ABIs is how indirect-calls (and, thus, function pointers) work. There are some other more-minor differences, for example, some of the call-frame offsets are different. It still might be possible to unify the support (we certainly have one backend in LLVM for both), but it's a bit more involved than just switching the endianness. https://reviews.llvm.org/D41702 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
