This revision was automatically updated to reflect the committed changes.
Closed by commit rL316772: Add specific ppc64le hardware watchpoint handler
(authored by labath).
Changed prior to commit:
https://reviews.llvm.org/D38897?vs=119885&id=120632#toc
Repository:
rL LLVM
https://reviews.ll
gut added a subscriber: eugene.
gut added a comment.
In https://reviews.llvm.org/D38897#908283, @gut wrote:
> In https://reviews.llvm.org/D38897#903581, @clayborg wrote:
>
> > Looks fine. Thanks for doing the changes.
>
>
> Hi, it's been already 4 days since this patch was accepted but not merged
Hi,
None of us have commit access. Please submit for us.
Thanks once again
> -Original Message-
> From: Zachary Turner [mailto:ztur...@google.com]
> Sent: quinta-feira, 26 de outubro de 2017 16:34
> To: reviews+d38897+public+f7e86848ad3ba...@reviews.llvm.org
> Cc: Ana Julia Pereira Caeta
Nothing happened internally. Usually people submit their own patches after
they're accepted. If neither of you have commit access though, then you'll
need to let us know so that we can submit on your behalf.
On Thu, Oct 26, 2017 at 11:30 AM Gustavo Serra Scalet via Phabricator <
revi...@reviews.
gut added a comment.
In https://reviews.llvm.org/D38897#903581, @clayborg wrote:
> Looks fine. Thanks for doing the changes.
Hi, it's been already 4 days since this patch was accepted but not merged. Did
something happen internally or we just need to wait a little longer?
https://reviews.llv
clayborg added a comment.
Yeah, I meant to suggest to remove that. Remove and submit as needed. Thanks
again.
https://reviews.llvm.org/D38897
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/l
anajuliapc updated this revision to Diff 119885.
anajuliapc added a comment.
- Remove unused enum
https://reviews.llvm.org/D38897
Files:
include/lldb/lldb-enumerations.h
source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp
source/Plugins/Process/Linux/NativeRegisterContextL
anajuliapc added inline comments.
Comment at:
source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp:59
+
+ enum watch_mode {
+write_mode = 1,
I just realized I forgot to remove the enum I've created before. I know you
already accepted this pa
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.
Looks fine. Thanks for doing the changes.
https://reviews.llvm.org/D38897
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://li
anajuliapc updated this revision to Diff 119847.
anajuliapc marked an inline comment as done.
anajuliapc added a comment.
- Use bitwise OR operator
https://reviews.llvm.org/D38897
Files:
include/lldb/lldb-enumerations.h
source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp
s
gut added inline comments.
Comment at:
source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp:292
+break;
+ case (eWatchpointKindRead + eWatchpointKindWrite):
+rw_mode = PPC_BREAKPOINT_TRIGGER_RW;
As these enums are bitmasks, the traditiona
anajuliapc updated this revision to Diff 119835.
anajuliapc added a comment.
- Switch over read and write eWatchpointKind to match with watch_flags
https://reviews.llvm.org/D38897
Files:
include/lldb/lldb-enumerations.h
source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp
s
clayborg added inline comments.
Comment at:
source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp:284
+ switch (watch_flags) {
+ case write_mode:
+rw_mode = PPC_BREAKPOINT_TRIGGER_WRITE;
anajuliapc wrote:
> clayborg wrote:
> > We should use t
anajuliapc added inline comments.
Comment at:
source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp:284
+ switch (watch_flags) {
+ case write_mode:
+rw_mode = PPC_BREAKPOINT_TRIGGER_WRITE;
clayborg wrote:
> We should use the lldb::WatchpointK
clayborg added inline comments.
Comment at:
source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp:284
+ switch (watch_flags) {
+ case write_mode:
+rw_mode = PPC_BREAKPOINT_TRIGGER_WRITE;
We should use the lldb::WatchpointKind from lldb-enume
New patch is fine. Lgtm
On Thu, Oct 19, 2017 at 4:56 AM Ana Julia Caetano via Phabricator <
revi...@reviews.llvm.org> wrote:
> anajuliapc added inline comments.
>
>
>
> Comment at:
> source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp:327-333
> + for (uint32_t i =
anajuliapc added inline comments.
Comment at:
source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp:327-333
+ for (uint32_t i = 0; i < m_max_hwp_supported; i++) {
+if ((m_hwp_regs[i].control & 1) == 0) {
+ wp_index = i; // Mark last free slot
+} else
anajuliapc updated this revision to Diff 119567.
anajuliapc marked 2 inline comments as done.
anajuliapc added a comment.
- Change numeric values to enum
- Use llvm's functions to align addresses
https://reviews.llvm.org/D38897
Files:
source/Plugins/Process/Linux/NativeRegisterContextLinux_pp
anajuliapc added a comment.
I applied some of the suggestions. I'm still working on the others, since I may
need to change some logic
Thanks for the comments
https://reviews.llvm.org/D38897
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
anajuliapc updated this revision to Diff 119336.
anajuliapc marked 6 inline comments as done.
anajuliapc added a comment.
- Add values to header file
- Use std::array to declare m_hwp_regs
- Use llvm's mask
- Remove switch and use assert instead
- Remove unnecessary 'else'
- Change return
https:
anajuliapc added inline comments.
Comment at:
source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp:374
+ uint32_t tempControl = m_hwp_regs[wp_index].control;
+ long * tempSlot = reinterpret_cast(m_hwp_regs[wp_index].slot);
+
zturner wrote:
> `re
zturner added a comment.
In https://reviews.llvm.org/D38897#897378, @gut wrote:
> Thanks for supporting this change. I guess @anajuliapc will add you both as
> reviewer as soon as she updates this patch.
>
> BTW, I agree that patches should be improving code quality but I wanted to
> highlight
gut added a comment.
Thanks for supporting this change. I guess @anajuliapc will add you both as
reviewer as soon as she updates this patch.
BTW, I agree that patches should be improving code quality but I wanted to
highlight that these changes were actually based on the current ARM64
implemen
clayborg added a comment.
Feel free to add me as a reviewer. Zach as well.
https://reviews.llvm.org/D38897
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
clayborg added a comment.
I agree with all of Zach's suggestions.
Comment at:
source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.h:98-99
+uint32_t refcount; // Serves as enable/disable and reference counter.
+long slot; // Saves the value
zturner added inline comments.
Comment at:
source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp:90-93
+ // 16 is just a maximum value, query hardware for actual watchpoint count
+ m_max_hwp_supported = 16;
+ m_max_hbp_supported = 16;
+ m_refresh_hwdebug_info =
anajuliapc created this revision.
Herald added subscribers: kbarton, nemanjai.
Add hardware watchpoint funcionality for ppc64le
https://reviews.llvm.org/D38897
Files:
source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp
source/Plugins/Process/Linux/NativeRegisterContextLinux_
27 matches
Mail list logo
