<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman",serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:#0563C1;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:#954F72;
text-decoration:underline;}
span.EmailStyle17
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;
font-family:"Calibri",sans-serif;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="#0563C1" vlink="#954F72">
<div class="WordSection1">
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">Thanks, Yaron.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">This change looks good to me. I didn’t get to it earlier, and I don’t want to commit it now in case it this late in the day, but I’ll commit this first thing
tomorrow.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D">-Andy<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><a name="_MailEndCompose"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D"><o:p> </o:p></span></a></p>
<p class="MsoNormal"><b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">From:</span></b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif"> Yaron Keren [mailto:yaron.keren@gmail.com]
<br>
<b>Sent:</b> Wednesday, November 11, 2015 9:03 AM<br>
<b>To:</b> reviews+D14454+public+ed63114aca7e347a@reviews.llvm.org; Phabricator <reviews@reviews.llvm.org><br>
<b>Cc:</b> Kaylor, Andrew <andrew.kaylor@intel.com>; Reid Kleckner <rnk@google.com>; David Majnemer <david.majnemer@gmail.com>; jotrem@microsoft.com; llvm-commits <llvm-commits@lists.llvm.org><br>
<b>Subject:</b> Re: [PATCH] D14454: [WinEH] Fix mutli-parent cloning<o:p></o:p></span></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal">Hi Andy,<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">The change from std::set to SetVector gains stable iteration order but loses stable iterators...<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">Specifically, it invalidates BlockColors[BB].end() after every remove(), so it should not be cached in the End variable. Even if we solve this by testing It != BlockColors[BB].end(), after removing the last item 'It' will be at the (new
after removal) end()+1 and then compared to the (new after removal) end() which is probably undefined behaviour for an iterator. Finally, erasing items in a loop in a SetVector is quadratic complexity.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">To solve all at once, we could split the loop into two steps:<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<div>
<p class="MsoNormal"> for (BasicBlock *ContainingFunclet : BlockColors[BB])<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"> if (ContainingFunclet != CorrectColor)<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"> FuncletBlocks[ContainingFunclet].erase(BB);<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"> BlockColors[BB].remove_if([&](BasicBlock *ContainingFunclet) {<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"> return ContainingFunclet != CorrectColor;<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"> });<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</div>
<div>
<p class="MsoNormal">Please feel free to modify the attached patch and commit,<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Yaron<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<p class="MsoNormal" dir="RTL" style="text-align:right;direction:rtl;unicode-bidi:embed">
<span dir="LTR"><o:p> </o:p></span></p>
</div>
</div>
</body>
</html>