<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">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.<div class=""><br class=""></div><div class="">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.</div><div class=""><br class=""></div><div class="">Greg</div><div class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Sep 5, 2017, at 2:06 PM, Davide Italiano <<a href="mailto:dccitaliano@gmail.com" class="">dccitaliano@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><span style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">On Tue, Sep 5, 2017 at 2:03 PM, Jason Molenda <</span><a href="mailto:jmolenda@apple.com" style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">jmolenda@apple.com</a><span style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">> wrote:</span><br style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class=""><br class=""><br class=""><blockquote type="cite" class="">On Sep 5, 2017, at 1:34 PM, Davide Italiano <<a href="mailto:dccitaliano@gmail.com" class="">dccitaliano@gmail.com</a>> wrote:<br class=""><br class="">On Tue, Sep 5, 2017 at 1:23 PM, Jason Molenda <<a href="mailto:jmolenda@apple.com" class="">jmolenda@apple.com</a>> wrote:<br class=""><blockquote type="cite" class="">Hi Davide, sorry I was offline for this discussion.<br class=""><br class="">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.<br class=""><br class="">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.<br class=""><br class="">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<br class=""><br class=""> const char *preserved_registers[] = { "r12", "r13", "r14", "r15", "rbp", "ebp", "rbx", "ebx",<br class=""> "rip", "eip", "rsp", "esp", "sp", "fp", "pc", NULL };<br class=""><br class=""> for (int i = 0; preserved_registers[i] != NULL; i++)<br class=""> if (strcmp (reg, preserved_registers[i]) == 0)<br class=""> return true<br class=""> return false;<br class=""><br class="">?<br class=""><br class=""><br class="">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.<br class=""><br class="">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?<br class=""><br class=""></blockquote><br class="">Hi Jason,<br class="">I hoped to receive comments, so, thank you. I profiled lldb a bit<br class="">recently and I never saw this function showing up in the profile.<br class="">That said, I agree we shouldn't completely give up performances for<br class="">readability in this case. [In particular, I'm more worried about the<br class="">increase in code size].<br class=""></blockquote><br class="">When I first looked at this function compiled -O0 it was 880 instructions long and I laughed out loud. :)<br class=""><br class="">I don't feel strongly about this, your change is fine, I was mostly curious if I was missing something.<br class=""><br class="">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.<br class=""><br class=""></blockquote><br style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Yes, I agree, I'll update my checkout and push that change. I'll also</span><br style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">try to see if we can standardize a style across all the ABIs, and what</span><br style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">you propose seems the right tradeoff between perf and readability.</span><br style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">It's unfortunate that StringSwitch generates less than ideal code</span><br style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">here, I guess the concept of zero-cost abstraction needs to be refined</span><br style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">for this very abstraction. I'll open an LLVM bug and try to take a</span><br style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">look.</span><br style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Thanks for taking the time to look at this further.</span><br style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">--</span><br style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Menlo-Regular; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Davide</span></div></blockquote></div><br class=""></div></body></html>