<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;}
@font-face
        {font-family:Kartika;
        panose-1:2 2 5 3 3 4 4 6 2 3;}
@font-face
        {font-family:-apple-system;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
        {margin:0in;
        font-size:11.0pt;
        font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
        {mso-style-priority:99;
        color:blue;
        text-decoration:underline;}
span.apple-converted-space
        {mso-style-name:apple-converted-space;}
span.EmailStyle21
        {mso-style-type:personal-reply;
        font-family:"Calibri",sans-serif;
        color:windowtext;}
.MsoChpDefault
        {mso-style-type:export-only;
        font-size:10.0pt;}
@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" style="word-wrap:break-word">
<div class="WordSection1">
<p class="MsoNormal">Hi Amara, <o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">If you still have any objection against moving the expansion of these memory intrinsics entirely into legalizer, please let us know.<o:p></o:p></p>
<p class="MsoNormal">Much appreciate it.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Regards,<o:p></o:p></p>
<p class="MsoNormal">CD<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b>From:</b> Devadasan, Christudasan <br>
<b>Sent:</b> Monday, December 21, 2020 7:40 PM<br>
<b>To:</b> Matt Arsenault <arsenm2@gmail.com>; Amara Emerson <amara@apple.com><br>
<b>Cc:</b> llvm-dev@lists.llvm.org<br>
<b>Subject:</b> RE: [llvm-dev] [GlobalISel] Legalization for memcpy family intrinsics<o:p></o:p></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Can we conclude this discussion?<o:p></o:p></p>
<p class="MsoNormal">I believe legalizer is the right place for this expansion.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Regards,<o:p></o:p></p>
<p class="MsoNormal">CD<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b>From:</b> Matt Arsenault <<a href="mailto:whatmannerofburgeristhis@gmail.com">whatmannerofburgeristhis@gmail.com</a>>
<b>On Behalf Of </b>Matt Arsenault<br>
<b>Sent:</b> Thursday, December 17, 2020 7:38 AM<br>
<b>To:</b> Amara Emerson <<a href="mailto:amara@apple.com">amara@apple.com</a>><br>
<b>Cc:</b> Devadasan, Christudasan <<a href="mailto:Christudasan.Devadasan@amd.com">Christudasan.Devadasan@amd.com</a>>;
<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a><br>
<b>Subject:</b> Re: [llvm-dev] [GlobalISel] Legalization for memcpy family intrinsics<o:p></o:p></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">[CAUTION: External Email] <o:p></o:p></p>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt"><o:p> </o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<p class="MsoNormal">On Dec 16, 2020, at 13:52, Amara Emerson via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>> wrote:<o:p></o:p></p>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt"><o:p> </o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<p class="MsoNormal">On Dec 16, 2020, at 6:47 AM, Devadasan, Christudasan <<a href="mailto:Christudasan.Devadasan@amd.com">Christudasan.Devadasan@amd.com</a>> wrote:<o:p></o:p></p>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal">Hi All,<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">I was trying to legalize the family of memcpy intrinsics (memcpy, memmove and memset) for AMDGPU and<span class="apple-converted-space"> </span><span style="font-size:10.5pt;font-family:-apple-system;color:#172B4D;background:white">found
 that the combiner functions optimizeMemcpy, optimizeMemmove and optimizeMemset implemented in CombinerHelper.cpp pretty much handle these lowering.</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:-apple-system;color:#172B4D;background:white"> </span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:-apple-system;color:#172B4D;background:white">Can we move these functions to Legalizer entirely and perform a custom legalization to handle their expansion?</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:-apple-system;color:#172B4D;background:white">The reason is, in the Combiner it is more of an optimization action rather than something required always for correctness.</span><o:p></o:p></p>
</div>
</div>
</blockquote>
<p class="MsoNormal">Right, that’s how this is implemented in SelectionDAG too, it’s an optimization that may or may not fire depending on the heuristics.<o:p></o:p></p>
</div>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">This is a largely a function of SelectionDAG not supporting the necessary loop-based expansions since you can’t introduce control flow. The current scheme is a hacky split between IR and DAG expansions.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:-apple-system;color:#172B4D;background:white"> </span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:-apple-system;color:#172B4D;background:white">On the other hand, if these combiner expansions turned out to be unavoidable, we should move these function into a common file.</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:-apple-system;color:#172B4D;background:white">In that way, both Legalizer and Combiner can take advantage of them.</span><o:p></o:p></p>
</div>
</div>
</blockquote>
<p class="MsoNormal">I think refactoring it to be shared is ok, but I and others disagree that this is a legalization issue rather than a combiner. There is no question of legality here, the target should be able to handle these opcodes. If they’re not legal
 for your target, then you could simply not use the expansion combines and handle them using custom legalization like any other operation<o:p></o:p></p>
</div>
</div>
</div>
</blockquote>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">There is a question of legality, just as with every other opcode. The legalizer should support the necessary expansion, and these shouldn’t be special.The expansion can be done context free, so I think it fundamentally isn't a combine.
 Splitting the expansion decisions between two places would be just as messy as the DAG<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">-Matt<o:p></o:p></p>
</div>
</div>
</div>
</body>
</html>