abhishek.aggarwal added a comment.
In https://reviews.llvm.org/D33035#833876, @clayborg wrote:
> I was out on vacation, sorry for the delay. Looks good.
Thanks a lot Greg for your feedback. I am merging the patch now.
https://reviews.llvm.org/D33035
abhishek.aggarwal added a comment.
Hi Greg .. Can you please review the changes? Please let me know if something
needs to be changed. Thanks :)
https://reviews.llvm.org/D33035
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.l
abhishek.aggarwal added a comment.
In https://reviews.llvm.org/D33035#816337, @clayborg wrote:
> I am not sure how sensitive typemaps are to the names of the arguments? Maybe
> we can just have you use different names and then the two typemaps won't
> collide? You can also write some manual cod
abhishek.aggarwal added a comment.
In https://reviews.llvm.org/D33035#803801, @clayborg wrote:
> Much better on the API with Swig. Just a few inlined questions around
> GetRawBytes.
Hi Greg .. Thanks for feedback. My comments are inlined.
Comment at: tools/intel-features/in
abhishek.aggarwal updated this revision to Diff 105434.
abhishek.aggarwal added a comment.
Removed std::vector<> from public APIs
https://reviews.llvm.org/D33035
Files:
tools/CMakeLists.txt
tools/intel-features/CMakeLists.txt
tools/intel-features/README.txt
tools/intel-features/cli-wrap
abhishek.aggarwal added a comment.
In https://reviews.llvm.org/D33035#799656, @clayborg wrote:
> In https://reviews.llvm.org/D33035#799404, @abhishek.aggarwal wrote:
>
> > In https://reviews.llvm.org/D33035#799058, @clayborg wrote:
> >
> > > So std::vector shouldn't be used in a public API. You s
abhishek.aggarwal added a comment.
In https://reviews.llvm.org/D33035#799058, @clayborg wrote:
> So std::vector shouldn't be used in a public API. You should make a class
> that wraps your objects. LLDB's public API has lldb::SBInstruction and
> lldb::SBInstructionList as examples. std::vector
abhishek.aggarwal added a comment.
In https://reviews.llvm.org/D33035#798885, @labath wrote:
> In https://reviews.llvm.org/D33035#798857, @abhishek.aggarwal wrote:
>
> > Hi Pavel .. I could remove all exception handling code from source files.
> > However, I am still wondering how to disable exc
abhishek.aggarwal added a comment.
Hi Pavel .. I could remove all exception handling code from source files.
However, I am still wondering how to disable exception handling while providing
python functions for C++ APIs of the features. Can you suggest something here?
The whole problem is occurr
abhishek.aggarwal updated this revision to Diff 105163.
abhishek.aggarwal added a comment.
Removed exception handling code
https://reviews.llvm.org/D33035
Files:
tools/CMakeLists.txt
tools/intel-features/CMakeLists.txt
tools/intel-features/README.txt
tools/intel-features/cli-wrapper.cpp
abhishek.aggarwal added inline comments.
Comment at: tools/intel-features/intel-pt/Decoder.cpp:411
+std::string image_path(image_complete_path, path_length);
+try {
+ readExecuteSectionInfos.emplace_back(
labath wrote:
> abhishek.aggarwal
abhishek.aggarwal added a comment.
Thanks for your review Pavel. My comments are inlined. Let me know your opinion
:)
Comment at: tools/intel-features/intel-pt/Decoder.cpp:411
+std::string image_path(image_complete_path, path_length);
+try {
+ readExec
abhishek.aggarwal updated this revision to Diff 104430.
abhishek.aggarwal added a comment.
Fixed one minor thing in CMakeFile
https://reviews.llvm.org/D33035
Files:
tools/CMakeLists.txt
tools/intel-features/CMakeLists.txt
tools/intel-features/README.txt
tools/intel-features/cli-wrapper.
abhishek.aggarwal updated this revision to Diff 104376.
abhishek.aggarwal added a comment.
Cmake files related changes
- Using lldb's cmake functions insted of vanilla ones for cmake files
https://reviews.llvm.org/D33035
Files:
tools/CMakeLists.txt
tools/intel-features/CMakeLists.txt
too
abhishek.aggarwal added a comment.
Hi Pavel .. I have made the changes you suggested. My apologies for
misinterpreting your previous comments but during written communications, it is
sometimes difficult to interpret everything correctly. I have tried following
LLDB's coding conventions and guid
abhishek.aggarwal updated this revision to Diff 103924.
abhishek.aggarwal added a comment.
Changes after feedback
- Created static libs for each hardware feature to combine it to generate
single shared library
- Removed error codes decoding logic and directly using error strings
https://review
abhishek.aggarwal added a comment.
Hi Pavel .. My comments are inlined. Please let me know if you have more
concerns. I am submitting the patch with all the proposed changes.
Comment at: tools/intel-features/CMakeLists.txt:50
+install(TARGETS lldbIntelFeatures
+ LIBRARY DESTI
abhishek.aggarwal added a subscriber: ravitheja.
abhishek.aggarwal added inline comments.
Comment at: tools/intel-pt/CMakeLists.txt:53
+
+if (NOT LLDB_DISABLE_PYTHON)
+ target_link_libraries(lldbIntelPT PRIVATE
labath wrote:
> All of this needs to go away. I thi
abhishek.aggarwal added a comment.
In https://reviews.llvm.org/D33035#754640, @labath wrote:
> I don't really like that we are adding a public shared library for every tiny
> intel feature. Could we at least merge this "plugin" with the existing
> "intel-mpx plugin" to create one "intel support
abhishek.aggarwal updated this revision to Diff 100376.
abhishek.aggarwal added a comment.
Updating https://reviews.llvm.org/D33434: Added new API to SBStructuredData
class
- Removed inferior from test case (not required)
- fixed enum scope
https://reviews.llvm.org/D33434
Files:
include/lld
abhishek.aggarwal marked 2 inline comments as done.
abhishek.aggarwal added a comment.
Thanks for your suggestions. I have made changes according to feedback and
submitting it here.
https://reviews.llvm.org/D33434
___
lldb-commits mailing list
lldb
abhishek.aggarwal added a comment.
My comments are inlined. Please let me know if something still needs to be
changed.
Comment at: include/lldb/API/SBStructuredData.h:60
+ //--
+ size_t GetSize() const;
+
--
abhishek.aggarwal updated this revision to Diff 100108.
abhishek.aggarwal marked 9 inline comments as done.
abhishek.aggarwal added a comment.
Updating https://reviews.llvm.org/D33434: Added new API to SBStructuredData
class
- Made changes according to feedback
https://reviews.llvm.org/D33434
abhishek.aggarwal added inline comments.
Comment at: tools/CMakeLists.txt:8
add_subdirectory(lldb-mi)
+option(LLDB_BUILD_INTEL_PT "Enable Building of Intel(R) Processor Trace Tool"
OFF)
+if (LLDB_BUILD_INTEL_PT)
clayborg wrote:
> Can we default this to enabled?
abhishek.aggarwal abandoned this revision.
abhishek.aggarwal added a comment.
Abandoning this change due to formatting problem with commit message.
https://reviews.llvm.org/D33034
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://list
25 matches
Mail list logo
