<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:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
p
{mso-style-priority:99;
mso-margin-top-alt:auto;
margin-right:0in;
mso-margin-bottom-alt:auto;
margin-left:0in;
font-size:12.0pt;
font-family:"Times New Roman",serif;}
span.EmailStyle18
{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=blue vlink=purple><div class=WordSection1><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>Hi George,<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'>After digging a little deeper, it appears that readonly calls showing up as MemoryDefs is only happening on an EarlyCSE test that is using the new pass manager (test/Transforms/EarlyCSE/basic.ll test5 if you’re curious), so I suspect it is an issue with the new pass manager setup code for either MemorySSA, my changes to EarlyCSE, the test run command line or something else not related to the MemorySSA code proper.<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><div style='mso-element:para-border-div;border:dashed #2F6FAB 1.0pt;padding:12.0pt 12.0pt 12.0pt 12.0pt;background:#F9F9F9'><p class=MsoNormal style='line-height:15.6pt;background:#F9F9F9;border:none;padding:0in'><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>--<o:p></o:p></span></p><p class=MsoNormal style='line-height:15.6pt;background:#F9F9F9;border:none;padding:0in'><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>Geoff Berry<o:p></o:p></span></p><p class=MsoNormal style='line-height:15.6pt;background:#F9F9F9;border:none;padding:0in'><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>Employee of Qualcomm Innovation Center, Inc.<o:p></o:p></span></p><p class=MsoNormal style='line-height:15.6pt;background:#F9F9F9;border:none;padding:0in'><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project<o:p></o:p></span></p></div><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'><o:p> </o:p></span></p><div style='border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt'><div><div style='border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in'><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'> George Burgess [mailto:gbiv@google.com] <br><b>Sent:</b> Wednesday, April 20, 2016 3:29 PM<br><b>To:</b> Geoff Berry <gberry@codeaurora.org><br><b>Cc:</b> Daniel Berlin <dberlin@dberlin.org>; llvm-dev <llvm-dev@lists.llvm.org><br><b>Subject:</b> Re: [LICM][MemorySSA] Converting LICM pass to use MemorySSA to avoid AliasSet collapse issue<o:p></o:p></span></p></div></div><p class=MsoNormal><o:p> </o:p></p><div><div><p class=MsoNormal>Hi!<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>> readonly calls are treated as clobbers by MemorySSA which leads to extra walking of MemoryDefs to not regress some EarlyCSE test cases</span><o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><p class=MsoNormal>Can you share a testcase for this too, please? Specifically, the first test in test/Transforms/Util/MemorySSA/function-mem-attrs.ll shows that we should treat readonly calls + calls to readonly functions as MemoryUses, so if we're thinking they're clobbers somehow, that sounds like a bug...<o:p></o:p></p><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>Thank you!<o:p></o:p></p></div><div><p class=MsoNormal>George<o:p></o:p></p><div><p class=MsoNormal><o:p> </o:p></p><div><p class=MsoNormal>On Wed, Apr 20, 2016 at 12:06 PM, Geoff Berry <<a href="mailto:gberry@codeaurora.org" target="_blank">gberry@codeaurora.org</a>> wrote:<o:p></o:p></p><blockquote style='border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in'><div><div><p><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>1)</span><span style='font-size:7.0pt;color:#1F497D'> </span><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>Sounds good. This isn’t holding me up so I’ll just try to keep an eye out for these changes.</span><o:p></o:p></p><p><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'> </span><o:p></o:p></p><p><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>2)</span><span style='font-size:7.0pt;color:#1F497D'> </span><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>I’ve attached an example IR file and debug log of where the caching is going bad. It depends on my changes to EarlyCSE, but hopefully it is clear from the debug output what is going on. Let me know if there is a better way to get this repro case to you. Also, I’ll be on IRC for the next couple of hours if you would like to have a quicker discussion.</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'> </span><o:p></o:p></p><div style='border:dashed #2F6FAB 1.0pt;padding:12.0pt 12.0pt 12.0pt 12.0pt'><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;line-height:15.6pt;background:#F9F9F9'><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>--</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;line-height:15.6pt;background:#F9F9F9'><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>Geoff Berry</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;line-height:15.6pt;background:#F9F9F9'><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>Employee of Qualcomm Innovation Center, Inc.</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;line-height:15.6pt;background:#F9F9F9'><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project</span><o:p></o:p></p></div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'> </span><o:p></o:p></p><div style='border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt'><div><div style='border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in'><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><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'> Daniel Berlin [mailto:<a href="mailto:dberlin@dberlin.org" target="_blank">dberlin@dberlin.org</a>] <br><b>Sent:</b> Wednesday, April 20, 2016 1:06 PM<br><b>To:</b> Geoff Berry <<a href="mailto:gberry@codeaurora.org" target="_blank">gberry@codeaurora.org</a>>; George Burgess <<a href="mailto:gbiv@google.com" target="_blank">gbiv@google.com</a>><br><b>Cc:</b> llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>><br><b>Subject:</b> Re: [LICM][MemorySSA] Converting LICM pass to use MemorySSA to avoid AliasSet collapse issue</span><o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>On Wed, Apr 20, 2016 at 9:58 AM, Geoff Berry <<a href="mailto:gberry@codeaurora.org" target="_blank">gberry@codeaurora.org</a>> wrote:<o:p></o:p></p><blockquote style='border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt'><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>Hi Daniel,</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'> </span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>Thanks for the info. I’ve started looking into converting EarlyCSE to use MemorySSA first since 1) I don’t think it needs any additional MemorySSA update API and 2) the particular case I’m looking at needs EarlyCSE to catch more load cases before LICM to be profitable.</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>I have a prototype working, but have run into two issues:</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'> </span><o:p></o:p></p><p><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>1)</span><span style='font-size:7.0pt;color:#1F497D'> </span><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>readonly calls are treated as clobbers by MemorySSA which leads to extra walking of MemoryDefs to not regress some EarlyCSE test cases. This isn’t a huge deal, I’m just wondering if it is intentional or something that just hasn’t been gotten to yet.</span><o:p></o:p></p></div></div></blockquote><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>George is working on the optimizations, of which this is one.<o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>I think this is one of the ones his current patch (under review) addresses.<o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div><blockquote style='border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt'><div><div><p><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>2)</span><span style='font-size:7.0pt;color:#1F497D'> </span><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>There seems to be a bug in the CachingMemorySSAWalker invalidation causing it to return MemoryAccess nodes that have been removed. In the case I’m seeing, a call node is removed from MemorySSA which causes CachingMemorySSAWalker::invalidateInfo() to clear the CachedUpwardsClobberingCall map. However, this same call node is present as a value in the CachedUpwardsClobberingAccess map,</span><o:p></o:p></p></div></div></blockquote><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>Unless i'm missing something, this should not have happened, and we should assert they are not being added to the cache.<o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>The truth is the caching parts are complicated and ugly. It was meant to be a pretty simple cache, but it's known to be inefficient (memory wise) and it's on the list of things to clean up and make sane.<o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;margin-bottom:12.0pt'>Do you have a testcase where this happens?<o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>A quick glance says we check whether it's a call in all the right places, which means there must be a place we are not *setting* isCall properly.<o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div></div></div></div></div></div></div></div></div></blockquote></div><p class=MsoNormal><o:p> </o:p></p></div></div></div></div></div></body></html>