Re: [Lldb-commits] [PATCH] D37420: [ABI] Rewrite RegisterIsCalleeSaved

The C-string table and strcmp solution is fine, but I think the StringSwitch method is strictly better. It's both faster (although I think we agreed that speed doesn't matter in this case) //and// more readable. Another alternative would be: constexpr StringRef Registers[] = {"r12", "r13",

Re: [Lldb-commits] [PATCH] D37420: [ABI] Rewrite RegisterIsCalleeSaved

I don't think anyone disagrees with that -- the simple c-string table & strcmp solution is fine. It's fun to muse about different approaches like this, though. More generally, I think the way lldb handles registers is one of the less well-designed parts of the debugger and it's something I

Re: [Lldb-commits] [PATCH] D37420: [ABI] Rewrite RegisterIsCalleeSaved

Unless it's showing up on a profile, isn't all of this just a solution looking for a problem? Davide claims neither the original or updated code shows up on any profiles, so why don't we just default to readable? On Tue, Sep 5, 2017 at 3:17 PM Greg Clayton wrote: > We

Re: [Lldb-commits] [PATCH] D37420: [ABI] Rewrite RegisterIsCalleeSaved

We should actually populate the register info with a cached value of this so we do this once per process? > On Sep 5, 2017, at 3:12 PM, Jason Molenda wrote: > > This method gets called every time we try to read a register in the unwinder > when we're above stack frame 0.

Re: [Lldb-commits] [PATCH] D37420: [ABI] Rewrite RegisterIsCalleeSaved

This method gets called every time we try to read a register in the unwinder when we're above stack frame 0. The unwinder won't try to fetch volatile (non-callee-spilled) registers, and it uses this ABI method to check before trying to retrieve the reg. We could switch the preserved register

Re: [Lldb-commits] [PATCH] D37420: [ABI] Rewrite RegisterIsCalleeSaved

You could also use a collection of lldb_private::ConstString objects. Comparing those are pointer compares since lldb_private::ConstString unique the strings into a string pool. lldb_private::UniqueCStringMap has a very efficient way to do this if you need an example. Not sure how many times

Re: [Lldb-commits] [PATCH] D37420: [ABI] Rewrite RegisterIsCalleeSaved

Ok last message. I bet it's because the patch writes std::string Name(reg_info->Name); change this to StringRef and it should be fine. I'd be curious to see how many instructions that generates. On Tue, Sep 5, 2017 at 2:12 PM Zachary Turner wrote: > I noticed you said it

Re: [Lldb-commits] [PATCH] D37420: [ABI] Rewrite RegisterIsCalleeSaved

I noticed you said it generates new and delete. I find that strange, we should look into why that's happening because it's not supposed to be. On Tue, Sep 5, 2017 at 2:07 PM Zachary Turner wrote: > StringSwitch doesn't create any std::strings (doing so would allocate >

Re: [Lldb-commits] [PATCH] D37420: [ABI] Rewrite RegisterIsCalleeSaved

StringSwitch doesn't create any std::strings (doing so would allocate memory), but it does do the memcmp. Unless it's in a hot path I think optimizing for readability is the right choice. On Tue, Sep 5, 2017 at 2:02 PM Jason Molenda wrote: > > > > On Sep 5, 2017, at 1:34

Re: [Lldb-commits] [PATCH] D37420: [ABI] Rewrite RegisterIsCalleeSaved

On Tue, Sep 5, 2017 at 2:03 PM, Jason Molenda wrote: > > >> On Sep 5, 2017, at 1:34 PM, Davide Italiano wrote: >> >> On Tue, Sep 5, 2017 at 1:23 PM, Jason Molenda wrote: >>> Hi Davide, sorry I was offline for this discussion. >>>

Re: [Lldb-commits] [PATCH] D37420: [ABI] Rewrite RegisterIsCalleeSaved

On Tue, Sep 5, 2017 at 1:23 PM, Jason Molenda wrote: > Hi Davide, sorry I was offline for this discussion. > > I was a little curious about llvm::StringSwitch, I hadn't seen it before. Is > it creating std::string's for all of these strings, then memcmp'ing the > contents?

Re: [Lldb-commits] [PATCH] D37420: [ABI] Rewrite RegisterIsCalleeSaved

Hi Davide, sorry I was offline for this discussion. I was a little curious about llvm::StringSwitch, I hadn't seen it before. Is it creating std::string's for all of these strings, then memcmp'ing the contents? Greg originally wrote these RegisterIsCalleeSaved() methods in the ABI's hand

[Lldb-commits] [lldb] r312562 - Fix DW_FORM_strp parsing

Author: jankratochvil Date: Tue Sep 5 12:01:01 2017 New Revision: 312562 URL: http://llvm.org/viewvc/llvm-project?rev=312562=rev Log: Fix DW_FORM_strp parsing Differential revision: https://reviews.llvm.org/D37441 Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp

[Lldb-commits] [PATCH] D37295: [lldb] Adjust UpdateExternalModuleListIfNeeded method for the case of *.dwo

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Just add a more descriptive comment that states this exact issue more clearly as I didn't understand the issue from the comment that is there now (the comment that starts on line 1644).

[Lldb-commits] [PATCH] D37295: [lldb] Adjust UpdateExternalModuleListIfNeeded method for the case of *.dwo

alexshap added a comment. ping Repository: rL LLVM https://reviews.llvm.org/D37295 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits