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 name table to be ConstString's and pay a
one-time encoding cost for them, but the 'struct RegisterInfo' stores its
register name as a c-string so we'd need to encode that into a ConstString
every time we enter the method.
(or change RegisterInfo to have a ConstString rep of the register name instead
of a c-string. which wouldn't be a bad idea.)
> On Sep 5, 2017, at 3:04 PM, Greg Clayton <clayb...@gmail.com> wrote:
>
> 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 we call this function. If it is once per target run,
> then who cares. If it is every time we stop, then we should look a little
> closer.
>
> Greg
>
>> On Sep 5, 2017, at 2:06 PM, Davide Italiano <dccitali...@gmail.com> wrote:
>>
>> On Tue, Sep 5, 2017 at 2:03 PM, Jason Molenda <jmole...@apple.com> wrote:
>>>
>>>
>>>> On Sep 5, 2017, at 1:34 PM, Davide Italiano <dccitali...@gmail.com> wrote:
>>>>
>>>> On Tue, Sep 5, 2017 at 1:23 PM, Jason Molenda <jmole...@apple.com> 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? Greg originally wrote these RegisterIsCalleeSaved()
>>>>> methods in the ABI's hand optimizing the character comparisons for
>>>>> efficiency, sacrificing readability in the process big-time but we added
>>>>> the comments to make it easier to follow.
>>>>>
>>>>> This version is much easier to read but loses the efficiency. Looking at
>>>>> the assembly generated by clang -Os, we're getting the same of the input
>>>>> string and then running memcmp() against all of the c-strings.
>>>>>
>>>>> If we're going to ignore the efficiency that Greg was shooting for here,
>>>>> why not write it with an array of c-strings and strcmp, like
>>>>>
>>>>> const char *preserved_registers[] = { "r12", "r13", "r14", "r15",
>>>>> "rbp", "ebp", "rbx", "ebx",
>>>>> "rip", "eip", "rsp", "esp", "sp", "fp", "pc", NULL };
>>>>>
>>>>> for (int i = 0; preserved_registers[i] != NULL; i++)
>>>>> if (strcmp (reg, preserved_registers[i]) == 0)
>>>>> return true
>>>>> return false;
>>>>>
>>>>> ?
>>>>>
>>>>>
>>>>> The original version, as hard to read as it was, compiles down to 60
>>>>> instructions with no memory allocations or function calls with clang -Os.
>>>>> Using llvm::StringSwitch is 184, with new, delete, memcpy, memcmp
>>>>> function calls. The strcmp() one weighs in around 30-35 instructions
>>>>> with calls to strcmp.
>>>>>
>>>>> I don't think this function is especially hot, I don't know if Greg's
>>>>> original focus on performance here was really the best choice. But if
>>>>> we're going to give up some performance, we could go the more generic
>>>>> strmp() route just as easily, couldn't we?
>>>>>
>>>>
>>>> Hi Jason,
>>>> I hoped to receive comments, so, thank you. I profiled lldb a bit
>>>> recently and I never saw this function showing up in the profile.
>>>> That said, I agree we shouldn't completely give up performances for
>>>> readability in this case. [In particular, I'm more worried about the
>>>> increase in code size].
>>>
>>> When I first looked at this function compiled -O0 it was 880 instructions
>>> long and I laughed out loud. :)
>>>
>>> I don't feel strongly about this, your change is fine, I was mostly curious
>>> if I was missing something.
>>>
>>> I wouldn't want to make extra work for equivalent readability/performance
>>> (IMO) unless you want to. I think many of the other ABI plugins have
>>> similar code in them; if I were changing any others, I would use the
>>> simpler loop & strcmp() method I think.
>>>
>>
>> Yes, I agree, I'll update my checkout and push that change. I'll also
>> try to see if we can standardize a style across all the ABIs, and what
>> you propose seems the right tradeoff between perf and readability.
>> It's unfortunate that StringSwitch generates less than ideal code
>> here, I guess the concept of zero-cost abstraction needs to be refined
>> for this very abstraction. I'll open an LLVM bug and try to take a
>> look.
>>
>> Thanks for taking the time to look at this further.
>>
>> --
>> Davide
>
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
