vsk added a comment.
I'm not sure if the current revision of the patch reflects the long-term
testing strategy. But if so, I'm quite concerned about the proliferation of
large binary files in the repo (like ld.so, or the raw trace binary itself).
These are opaque blobs that are hard to understand. Also, each time we add one,
we're imposing a sizeable tax on everyone working with the llvm-project
monorepo.
One possible alternative:
- Design a textual description for the raw trace contents, and possibly a way
to convert an existing trace file into this textual format
- Check in assembly, and use llvm-mc/clang to generate executables during
testing
================
Comment at: lldb/tools/intel-features/intel-pt/Decoder.cpp:251
+#include <fstream>
+
----------------
Please group your includes.
================
Comment at: lldb/tools/intel-features/intel-pt/Decoder.cpp:260
Buffer &pt_buffer = threadTraceInfo.GetPTBuffer();
+
lldb::SBError error;
----------------
unintentional whitespace diff?
================
Comment at: lldb/tools/intel-features/intel-pt/Decoder.cpp:301
+ std::ofstream of("/tmp/multi-file.trace", std::ios::binary | std::ios::out);
+ for (auto bait : pt_buffer)
----------------
Not sure what this is in aid of?
================
Comment at: llvm/include/llvm/Support/Error.h:1016
+/// expected-producer might be more clearly refactored to return an
Optional<T>.
+template <typename T> inline void consumeExpected(Expected<T> E) {
+ if (!E)
----------------
Probably best to not add another escape-hatch to permit fast and loose error
handling. This seems to be used in a lambda passed to TraverseInstructions.
There might be a way to avoid invoking the callback in the case where the
expected value is thrown away.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D89283/new/
https://reviews.llvm.org/D89283
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
