[Lldb-commits] [PATCH] D77790: [NFC] Add a test for the inferior memory cache (mainly L1)

2020-04-11 Thread Jaroslav Sevcik via Phabricator via lldb-commits
jarin added a comment. In D77790#1975324 , @clayborg wrote: > In D77790#1974047 , @jarin wrote: > > > It appears it is really hard to reach agreement about this, so another > > alternative is I submit a bug report

[Lldb-commits] [PATCH] D77790: [NFC] Add a test for the inferior memory cache (mainly L1)

2020-04-10 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. In D77790#1974047 , @jarin wrote: > Regarding the callback idea, I have bad experience with callbacks because > they break if the code is not crafted for re-entrancy and they are much > harder to understand. That change feels ou

[Lldb-commits] [PATCH] D77790: [NFC] Add a test for the inferior memory cache (mainly L1)

2020-04-10 Thread Jaroslav Sevcik via Phabricator via lldb-commits
jarin added a comment. Regarding the callback idea, I have bad experience with callbacks because they break if the code is not crafted for re-entrancy and they are much harder to understand. That change feels out of scope for just adding a test and fixing an unrelated bug. Adding the SetCacheL

[Lldb-commits] [PATCH] D77790: [NFC] Add a test for the inferior memory cache (mainly L1)

2020-04-09 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. Mostly good, but it seems weird to supply the cache line size when calling the MemoryCache::Clear() function. We also don't seem to be catching updates to the cache line size property and invalidating the cache when and only when the property is modified, but we seem t

[Lldb-commits] [PATCH] D77790: [NFC] Add a test for the inferior memory cache (mainly L1)

2020-04-09 Thread Jaroslav Sevcik via Phabricator via lldb-commits
jarin added inline comments. Comment at: lldb/source/Target/Memory.cpp:63 if (pos != m_L1_cache.begin()) { - --pos; + pos--; } labath wrote: > I guess this is a leftover from splitting the patches? > > Speaking of post-increment the [[ > htt

[Lldb-commits] [PATCH] D77790: [NFC] Add a test for the inferior memory cache (mainly L1)

2020-04-09 Thread Jaroslav Sevcik via Phabricator via lldb-commits
jarin updated this revision to Diff 256257. jarin marked 2 inline comments as done. jarin added a comment. Addressed reviewer comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77790/new/ https://reviews.llvm.org/D77790 Files: lldb/include/

[Lldb-commits] [PATCH] D77790: [NFC] Add a test for the inferior memory cache (mainly L1)

2020-04-09 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Thanks for splitting this up. This looks fine to me, modulo some nits, but lets wait @clayborg has to say. Comment at: lldb/source/Target/Memory.cpp:63 if (pos != m_L1_cache.begin()) { - --pos; + pos--; } I guess th

[Lldb-commits] [PATCH] D77790: [NFC] Add a test for the inferior memory cache (mainly L1)

2020-04-09 Thread Jaroslav Sevcik via Phabricator via lldb-commits
jarin created this revision. jarin added reviewers: labath, clayborg. jarin added a project: LLDB. Herald added subscribers: lldb-commits, mgorny. This patch adds a test for L1 of the inferior's memory cache and makes the cache testable. This is mostly in preparation