<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">I still disagree but I’m not going to spend more time on this discussion. Move it over to legalization if you must.<div class=""><div><br class=""><blockquote type="cite" class=""><div class="">On Jan 28, 2021, at 8:15 AM, Devadasan, Christudasan <<a href="mailto:Christudasan.Devadasan@amd.com" class="">Christudasan.Devadasan@amd.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><meta charset="UTF-8" class=""><div class="WordSection1" style="page: WordSection1; caret-color: rgb(0, 0, 0); font-family: Helvetica; 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; text-decoration: none;"><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class="">Hi Amara,<span class="Apple-converted-space"> </span><o:p class=""></o:p></div><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class="">If you still have any objection against moving the expansion of these memory intrinsics entirely into legalizer, please let us know.<o:p class=""></o:p></div><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class="">Much appreciate it.<o:p class=""></o:p></div><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class="">Regards,<o:p class=""></o:p></div><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class="">CD<o:p class=""></o:p></div><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div><div class=""><div style="border-style: solid none none; border-top-width: 1pt; border-top-color: rgb(225, 225, 225); padding: 3pt 0in 0in;" class=""><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><b class="">From:</b><span class="Apple-converted-space"> </span>Devadasan, Christudasan<span class="Apple-converted-space"> </span><br class=""><b class="">Sent:</b><span class="Apple-converted-space"> </span>Monday, December 21, 2020 7:40 PM<br class=""><b class="">To:</b><span class="Apple-converted-space"> </span>Matt Arsenault <<a href="mailto:arsenm2@gmail.com" class="">arsenm2@gmail.com</a>>; Amara Emerson <<a href="mailto:amara@apple.com" class="">amara@apple.com</a>><br class=""><b class="">Cc:</b><span class="Apple-converted-space"> </span><a href="mailto:llvm-dev@lists.llvm.org" class="">llvm-dev@lists.llvm.org</a><br class=""><b class="">Subject:</b><span class="Apple-converted-space"> </span>RE: [llvm-dev] [GlobalISel] Legalization for memcpy family intrinsics<o:p class=""></o:p></div></div></div><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class="">Can we conclude this discussion?<o:p class=""></o:p></div><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class="">I believe legalizer is the right place for this expansion.<o:p class=""></o:p></div><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class="">Regards,<o:p class=""></o:p></div><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class="">CD<o:p class=""></o:p></div><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div><div class=""><div style="border-style: solid none none; border-top-width: 1pt; border-top-color: rgb(225, 225, 225); padding: 3pt 0in 0in;" class=""><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><b class="">From:</b><span class="Apple-converted-space"> </span>Matt Arsenault <<a href="mailto:whatmannerofburgeristhis@gmail.com" style="color: blue; text-decoration: underline;" class="">whatmannerofburgeristhis@gmail.com</a>><span class="Apple-converted-space"> </span><b class="">On Behalf Of<span class="Apple-converted-space"> </span></b>Matt Arsenault<br class=""><b class="">Sent:</b><span class="Apple-converted-space"> </span>Thursday, December 17, 2020 7:38 AM<br class=""><b class="">To:</b><span class="Apple-converted-space"> </span>Amara Emerson <<a href="mailto:amara@apple.com" style="color: blue; text-decoration: underline;" class="">amara@apple.com</a>><br class=""><b class="">Cc:</b><span class="Apple-converted-space"> </span>Devadasan, Christudasan <<a href="mailto:Christudasan.Devadasan@amd.com" style="color: blue; text-decoration: underline;" class="">Christudasan.Devadasan@amd.com</a>>;<span class="Apple-converted-space"> </span><a href="mailto:llvm-dev@lists.llvm.org" style="color: blue; text-decoration: underline;" class="">llvm-dev@lists.llvm.org</a><br class=""><b class="">Subject:</b><span class="Apple-converted-space"> </span>Re: [llvm-dev] [GlobalISel] Legalization for memcpy family intrinsics<o:p class=""></o:p></div></div></div><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class="">[CAUTION: External Email]<span class="Apple-converted-space"> </span><o:p class=""></o:p></div><div class=""><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div><div class=""><p class="MsoNormal" style="margin: 0in 0in 12pt; font-size: 11pt; font-family: Calibri, sans-serif;"><o:p class=""> </o:p></p><blockquote style="margin-top: 5pt; margin-bottom: 5pt;" class=""><div class=""><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class="">On Dec 16, 2020, at 13:52, Amara Emerson via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" style="color: blue; text-decoration: underline;" class="">llvm-dev@lists.llvm.org</a>> wrote:<o:p class=""></o:p></div></div><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div><div class=""><div class=""><div class=""><p class="MsoNormal" style="margin: 0in 0in 12pt; font-size: 11pt; font-family: Calibri, sans-serif;"><o:p class=""> </o:p></p><blockquote style="margin-top: 5pt; margin-bottom: 5pt;" class=""><div class=""><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class="">On Dec 16, 2020, at 6:47 AM, Devadasan, Christudasan <<a href="mailto:Christudasan.Devadasan@amd.com" style="color: blue; text-decoration: underline;" class="">Christudasan.Devadasan@amd.com</a>> wrote:<o:p class=""></o:p></div></div><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div><div class=""><div class=""><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class="">Hi All,<o:p class=""></o:p></div></div><div class=""><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""> <o:p class=""></o:p></div></div><div class=""><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class="">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: rgb(23, 43, 77); background-color: white; background-position: initial initial; background-repeat: initial initial;" class="">found that the combiner functions optimizeMemcpy, optimizeMemmove and optimizeMemset implemented in CombinerHelper.cpp pretty much handle these lowering.</span><o:p class=""></o:p></div></div><div class=""><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><span style="font-size: 10.5pt; font-family: -apple-system; color: rgb(23, 43, 77); background-color: white; background-position: initial initial; background-repeat: initial initial;" class=""> </span><o:p class=""></o:p></div></div><div class=""><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><span style="font-size: 10.5pt; font-family: -apple-system; color: rgb(23, 43, 77); background-color: white; background-position: initial initial; background-repeat: initial initial;" class="">Can we move these functions to Legalizer entirely and perform a custom legalization to handle their expansion?</span><o:p class=""></o:p></div></div><div class=""><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><span style="font-size: 10.5pt; font-family: -apple-system; color: rgb(23, 43, 77); background-color: white; background-position: initial initial; background-repeat: initial initial;" class="">The reason is, in the Combiner it is more of an optimization action rather than something required always for correctness.</span><o:p class=""></o:p></div></div></div></blockquote><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class="">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 class=""></o:p></div></div></div></div></blockquote><div class=""><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div></div><div class=""><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class="">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 class=""></o:p></div></div><div class=""><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div></div><blockquote style="margin-top: 5pt; margin-bottom: 5pt;" class=""><div class=""><div class=""><div class=""><blockquote style="margin-top: 5pt; margin-bottom: 5pt;" class=""><div class=""><div class=""><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><span style="font-size: 10.5pt; font-family: -apple-system; color: rgb(23, 43, 77); background-color: white; background-position: initial initial; background-repeat: initial initial;" class=""> </span><o:p class=""></o:p></div></div><div class=""><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><span style="font-size: 10.5pt; font-family: -apple-system; color: rgb(23, 43, 77); background-color: white; background-position: initial initial; background-repeat: initial initial;" class="">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 class=""></o:p></div></div><div class=""><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><span style="font-size: 10.5pt; font-family: -apple-system; color: rgb(23, 43, 77); background-color: white; background-position: initial initial; background-repeat: initial initial;" class="">In that way, both Legalizer and Combiner can take advantage of them.</span><o:p class=""></o:p></div></div></div></blockquote><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class="">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 class=""></o:p></div></div></div></div></blockquote><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div></div><div class=""><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class="">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 class=""></o:p></div></div><div class=""><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div></div><div class=""><div style="margin: 0in; font-size: 11pt; font-family: Calibri, sans-serif;" class="">-Matt</div></div></div></div></div></blockquote></div><br class=""></div></body></html>