<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Nov 17, 2016, at 1:45 PM, Sam McCall <<a href="mailto:sammccall@google.com" class="">sammccall@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote">On Thu, Nov 17, 2016 at 10:29 PM, Jim Ingham <span dir="ltr" class=""><<a href="mailto:jingham@apple.com" target="_blank" class="">jingham@apple.com</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class="">Ah, good catch. That is not right. If there’s inlined function information, you want to make sure you haven’t gone from one inlined function to another. But the symbol is always going to be the same in this case.</div></blockquote><div class="">OK, I've updated the patch to return false in this case.</div><div class=""><br class=""></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class=""><div class="">Actually, the old code is a overly restrictive by comparing the blocks directly. What you really want to test is that if either side is in an inlined function, both sides are still inside the same inlined function instantiation. We should really compare the ContainingInlinedBlock of the current blocks if there is inlined info.</div></div></blockquote><div class="">Happy to take a stab at this - I'm not too familiar with the structures here so I'll read a bit and send a new patch if that's OK!</div></div></div></div></div></blockquote><div><br class=""></div><div>That’s fine.</div><div><br class=""></div><div>Jim</div><div><br class=""></div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote"><div class=""><br class=""></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class=""><div class="">This stuff is a PITA to write tests for, too, since it is hard to predict how the compiler will emit inlined code...</div><span class="HOEnZb"><font color="#888888" class=""><div class=""><br class=""></div></font></span><div class=""><span class="HOEnZb"><font color="#888888" class="">Jim</font></span><div class=""><div class="h5"><br class=""><div class=""><br class=""><div class=""><blockquote type="cite" class=""><div class="">On Nov 17, 2016, at 1:13 PM, Sam McCall <<a href="mailto:sammccall@google.com" target="_blank" class="">sammccall@google.com</a>> wrote:</div><br class="m_-4906574601768939979Apple-interchange-newline"><div class=""><div dir="ltr" class="">Thanks. I just noticed, this also changes the behavior (now returns true) if:<div class=""> - comp_unit and function both match<br class=""><div class=""> - block->GetInlinedFunctionInfo != nullptr</div><div class=""> - blocks aren't equal</div></div><div class=""> - symbols are equal</div><div class=""><br class=""></div><div class="">Is this correct? Otherwise I'll fix this case to match the old behavior.</div></div><div class="gmail_extra"><br class=""><div class="gmail_quote">On Thu, Nov 17, 2016 at 10:09 PM, Jim Ingham <span dir="ltr" class=""><<a href="mailto:jingham@apple.com" target="_blank" class="">jingham@apple.com</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">jingham accepted this revision.<br class="">
jingham added a comment.<br class="">
This revision is now accepted and ready to land.<br class="">
<br class="">
That way of doing it is fine too.<br class="">
<br class="">
<br class="">
<a href="https://reviews.llvm.org/D26804" rel="noreferrer" target="_blank" class="">https://reviews.llvm.org/D2680<wbr class="">4</a><br class="">
<br class="">
<br class="">
<br class="">
</blockquote></div><br class=""></div>
</div></blockquote></div><br class=""></div></div></div></div></div></blockquote></div><br class=""></div></div>
</div></blockquote></div><br class=""></body></html>