<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=""><div class="">I agree with Paul that we need to formalise the compatibility policy before we start removing support for old intrinsics. Do we want a deprecation warning of some kind for the use of any intrinsic used in auto-upgrade, would that actually be useful or just a nuisance?</div><div class=""><br class=""></div><div class="">In the meantime I’m happy to help fix any missing test coverage.</div><br class=""><div><blockquote type="cite" class=""><div class="">On 20 Sep 2017, at 22:16, Robinson, Paul via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" class="">llvm-dev@lists.llvm.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div class="WordSection1" style="page: WordSection1; font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class="">I think the general backward-compatibility story has been kind of vague for a while.  There was some talk about it at the time of the version-numbering change, but I don't remember if it came to any kind of solid conclusion.<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class=""><o:p class=""> </o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class="">I think the handling of the old X86 intrinsics would want to follow the general compatibility policy, assuming we can all agree on one.  There shouldn't be a special case for those IMO.  So, starting a new non-X86-specific thread about backward compatibility would be appropriate.<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class="">--paulr<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><a name="_MailEndCompose" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class=""><o:p class=""> </o:p></span></a></div><div style="border-style: none none none solid; border-left-color: blue; border-left-width: 1.5pt; padding: 0in 0in 0in 4pt;" class=""><div class=""><div style="border-style: solid none none; border-top-color: rgb(181, 196, 223); border-top-width: 1pt; padding: 3pt 0in 0in;" class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><b class=""><span style="font-size: 10pt; font-family: Tahoma, sans-serif;" class="">From:</span></b><span style="font-size: 10pt; font-family: Tahoma, sans-serif;" class=""><span class="Apple-converted-space"> </span>llvm-dev [<a href="mailto:llvm-dev-bounces@lists.llvm.org" style="color: purple; text-decoration: underline;" class="">mailto:llvm-dev-bounces@lists.llvm.org</a>]<span class="Apple-converted-space"> </span><b class="">On Behalf Of<span class="Apple-converted-space"> </span></b>Craig Topper via llvm-dev<br class=""><b class="">Sent:</b><span class="Apple-converted-space"> </span>Wednesday, September 20, 2017 1:01 PM<br class=""><b class="">To:</b><span class="Apple-converted-space"> </span>Daniel Berlin<br class=""><b class="">Cc:</b><span class="Apple-converted-space"> </span>llvm-dev<br class=""><b class="">Subject:</b><span class="Apple-converted-space"> </span>Re: [llvm-dev] RFC: [X86] Can we begin removing AutoUpgrade support for x86 instrinsics added in early 3.X versions<o:p class=""></o:p></span></div></div></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""> </o:p></div><div class=""><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">Many of the older autoupgrades have no test cases because I think when we upgraded them we just replace all the code in the tests with native IR. So for some of the code we don't even know if it works.<o:p class=""></o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""> </o:p></div></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">I don't really want to watch the amount of code here continue to grow indefinitely. It's pretty poorly structured and has been up against the MSVC cascaded if/else limit a couple times. I think they allow about 128. Of course this is fixable by better structuring, but we'd probably want to fix the lack of tests to be more confident about not breaking it.<o:p class=""></o:p></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""> </o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">The ridiculous number of string compares in the code might be slow, but I'm not sure. I've never tried to profile it. We don't divide up the string compares based on first letters or anything so I think we run through a lot of memcmps.<o:p class=""></o:p></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""> </o:p></div></div><div class=""><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">Unfortunately, the bulk of the upgrade code was added in the 3.7, 3.9, 4.0 timeframe so the big reduction in code probably requires several more years. But I wanted to start a conversation about what our compability story looks like going forward.<o:p class=""></o:p></div></div></div></div></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><br clear="all" class=""><o:p class=""></o:p></div><div class=""><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">~Craig<o:p class=""></o:p></div></div></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""> </o:p></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">On Wed, Sep 20, 2017 at 12:31 PM, Daniel Berlin <<a href="mailto:dberlin@dberlin.org" target="_blank" style="color: purple; text-decoration: underline;" class="">dberlin@dberlin.org</a>> wrote:<o:p class=""></o:p></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">Is there a reason why?<br class="">IE is it hard to maintain, slow, or are you just worried it will break? or something else?<o:p class=""></o:p></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""> </o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">(I'm not opposed in any way, literally just want to understand the motivation)<o:p class=""></o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""> </o:p></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""> </o:p></div></div></div></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""> </o:p></div><div class=""><div class=""><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">On Wed, Sep 20, 2017 at 12:20 PM, Craig Topper via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank" style="color: purple; text-decoration: underline;" class="">llvm-dev@lists.llvm.org</a>> wrote:<o:p class=""></o:p></div></div></div><blockquote style="border-style: none none none solid; border-left-color: rgb(204, 204, 204); border-left-width: 1pt; padding: 0in 0in 0in 6pt; margin-left: 4.8pt; margin-right: 0in;" class=""><div class=""><div class=""><div class=""><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">We have quite a lot of code in AutoUpgrade.cpp to upgrade X86 intrinsics that have been replaced with native IR over the years. Has enough time and/or versions passed that we can begin phasing out some of this code?<o:p class=""></o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""> </o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">As I'm writing these we don't seem to have tests for a lot of the older upgrades. We've done better at this in the last few years.<o:p class=""></o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""> </o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">3.1 added upgrade for:<o:p class=""></o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">x86.sse2.pcmpeq.*  - we have almost no test cases for this<o:p class=""></o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">x86.sse2.pcmpgt.* - we no test cases for this<o:p class=""></o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">x86.avx2.pcmpeq.*  - we have no test cases<o:p class=""></o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">x86.avx2.pcmpgt.* - we have no test cases for this<o:p class=""></o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">x86.avx.vpermil.* - we do test this<o:p class=""></o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""> </o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">3.2 added upgrade for:<o:p class=""></o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">x86.avx.movnt.* - we have tests for this<o:p class=""></o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">x86.xop.vpcom* - we have tests for this<o:p class=""></o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">x86.sse41.ptest.* had its signature chagned and we upgrade from the old signature. We don't have tests for the old signature.<o:p class=""></o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">x86.xop.vfrcz.ss/sd had an argument dropped that we upgrade for. We don't have any tests for the old signature.<o:p class=""></o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""> </o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">3.3 had no upgrades<o:p class=""></o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""> </o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">3.4 removed:<o:p class=""></o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">x86.sse42.crc32.64.8 we do have tests that use it<o:p class=""></o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""> </o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">For the complete list of intrinsic upgrade support, there's an annotated list in ShouldUpgradeX86Intrinsic in AutoUpgrade.cpp<o:p class=""></o:p></div></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="color: rgb(136, 136, 136);" class=""><br clear="all" class=""><span class="m8648527886154158176hoenzb"><o:p class=""></o:p></span></span></div><div class=""><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="color: rgb(136, 136, 136);" class="">~Craig</span></div></div></div></div></div></div></blockquote></div></div></div></div></div></div></div></blockquote></div><br class="">
<br class=""></body></html>