<div dir="ltr"><div>The intent of r227983 is simply to revert the wrong behavior introduced by r224330 for *SSE* codegen.<br>If r224330 was supposed to change behavior of AVX512, it should have included test cases to validate that, right? These should be separate patches.<br><br>> You removed FeatureVectorUAMem form HSW, but you did not add FeatureSSEUnalignedMem<br><br></div><div>Correct - AFAIK, Haswell doesn't have this feature. Perhaps we need to change 'memop' to 'sse_memop' to make it clearer that memops are only for use with SSE...or just modify the 'load' defs to include this predicate somehow:<br><br>// Like 'load', but uses special alignment checks suitable for use in<br>// memory operands in most SSE instructions, which are required to<br>// be naturally aligned on some targets but not on others. If the subtarget<br>// allows unaligned accesses, match any load, though this may require<br>// setting a feature bit in the processor (on startup, for example).<br>// Opteron 10h and later implement such a feature.<br>def memop : PatFrag<(ops node:$ptr), (load node:$ptr), [{<br> return Subtarget->hasSSEUnalignedMem()<br> || cast<LoadSDNode>(N)->getAlignment() >= 16;<br>}]>;<br><br><br></div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Feb 5, 2015 at 1:11 AM, Demikhovsky, Elena <span dir="ltr"><<a href="mailto:elena.demikhovsky@intel.com" target="_blank">elena.demikhovsky@intel.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div link="blue" vlink="purple" lang="EN-US">
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">It can use load, I agree.
<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">But AVX512 code should be changed before committing the patch.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal" style="margin-left:36.0pt">
<u></u><span style="font-family:"Calibri","sans-serif";color:#31849b"><span>-<span style="font:7.0pt "Times New Roman"">
</span></span></span><u></u><span dir="LTR"></span><b><i><span style="color:#31849b"> Elena<u></u><u></u></span></i></b></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> Craig Topper [mailto:<a href="mailto:craig.topper@gmail.com" target="_blank">craig.topper@gmail.com</a>]
<br>
<b>Sent:</b> Thursday, February 05, 2015 10:04</span></p><div><div class="h5"><br>
<b>To:</b> Demikhovsky, Elena<br>
<b>Cc:</b> Sanjay Patel; llvm-commits<br>
<b>Subject:</b> Re: [llvm] r227983 - Fix program crashes due to alignment exceptions generated for SSE memop instructions (PR22371).<u></u><u></u></div></div><p></p><div><div class="h5">
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<p class="MsoNormal">Why is AVX512 not using load?<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<p class="MsoNormal">On Wed, Feb 4, 2015 at 11:59 PM, Demikhovsky, Elena <<a href="mailto:elena.demikhovsky@intel.com" target="_blank">elena.demikhovsky@intel.com</a>> wrote:<u></u><u></u></p>
<div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">We use it in AVX512.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal" style="margin-left:36.0pt">
<span style="font-family:"Calibri","sans-serif";color:#31849b">-</span><span style="font-size:7.0pt;color:#31849b">
</span><b><i><span style="color:#31849b"> Elena</span></i></b><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> Craig Topper [mailto:<a href="mailto:craig.topper@gmail.com" target="_blank">craig.topper@gmail.com</a>]
<br>
<b>Sent:</b> Thursday, February 05, 2015 09:52<br>
<b>To:</b> Demikhovsky, Elena<br>
<b>Cc:</b> Sanjay Patel; llvm-commits</span><u></u><u></u></p>
<div>
<div>
<p class="MsoNormal"><br>
<b>Subject:</b> Re: [llvm] r227983 - Fix program crashes due to alignment exceptions generated for SSE memop instructions (PR22371).<u></u><u></u></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<p class="MsoNormal">I thought we had converted all of AVX/AVX2 to use 'loadXXX' instead of 'memopXXX'?<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<p class="MsoNormal">On Wed, Feb 4, 2015 at 11:44 PM, Demikhovsky, Elena <<a href="mailto:elena.demikhovsky@intel.com" target="_blank">elena.demikhovsky@intel.com</a>> wrote:<u></u><u></u></p>
<p class="MsoNormal">Hi,<br>
You removed FeatureVectorUAMem form HSW, but you did not add FeatureSSEUnalignedMem<br>
FeatureAVX2,<br>
FeatureCMPXCHG16B,<br>
FeatureFastUAMem,<br>
- FeatureVectorUAMem,<br>
FeaturePOPCNT,<br>
FeatureAES,<br>
FeaturePCLMUL,<br>
<br>
How memop should work now on AVX/AVX2? Will it require alignment now?<br>
<br>
def memop : PatFrag<(ops node:$ptr), (load node:$ptr), [{<br>
- return Subtarget->hasVectorUAMem()<br>
+ return Subtarget->hasSSEUnalignedMem()<br>
|| cast<LoadSDNode>(N)->getAlignment() >= 16;<br>
}]>;<br>
<br>
- Elena<br>
<br>
<br>
-----Original Message-----<br>
From: Kuperstein, Michael M<br>
Sent: Wednesday, February 04, 2015 21:41<br>
To: Hans Wennborg; Nadav Rotem; Demikhovsky, Elena<br>
Cc: Sanjay Patel; llvm-commits<br>
Subject: RE: [llvm] r227983 - Fix program crashes due to alignment exceptions generated for SSE memop instructions (PR22371).<br>
<br>
I understand this on roughly the same level as Nadav does (looks correct, and should be safer than the current situation), but Elena should be able to give an authoritative answer.<br>
Elena, could you comment?<br>
<br>
Michael<br>
<br>
-----Original Message-----<br>
From: <a href="mailto:hwennborg@google.com" target="_blank">hwennborg@google.com</a> [mailto:<a href="mailto:hwennborg@google.com" target="_blank">hwennborg@google.com</a>] On Behalf Of Hans Wennborg<br>
Sent: Wednesday, February 04, 2015 20:53<br>
To: Nadav Rotem<br>
Cc: Sanjay Patel; llvm-commits; Demikhovsky, Elena; Kuperstein, Michael M<br>
Subject: Re: [llvm] r227983 - Fix program crashes due to alignment exceptions generated for SSE memop instructions (PR22371).<br>
<br>
Elena, Michael: is this safe for merging to 3.6?<br>
<br>
As far as I understand it's basically a revert or r224330.<br>
<br>
On Tue, Feb 3, 2015 at 3:44 PM, Nadav Rotem <<a href="mailto:nrotem@apple.com" target="_blank">nrotem@apple.com</a>> wrote:<br>
> The change looks good to me, but I would wait a day and let Elena or<br>
> Michael review it.<br>
><br>
> On Feb 3, 2015, at 3:19 PM, Hans Wennborg <<a href="mailto:hans@chromium.org" target="_blank">hans@chromium.org</a>> wrote:<br>
><br>
> On Tue, Feb 3, 2015 at 9:13 AM, Sanjay Patel <<a href="mailto:spatel@rotateright.com" target="_blank">spatel@rotateright.com</a>> wrote:<br>
><br>
> Author: spatel<br>
> Date: Tue Feb 3 11:13:04 2015<br>
> New Revision: 227983<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=227983&view=rev" target="_blank">
http://llvm.org/viewvc/llvm-project?rev=227983&view=rev</a><br>
> Log:<br>
> Fix program crashes due to alignment exceptions generated for SSE<br>
> memop instructions (PR22371).<br>
><br>
> r224330 introduced a bug by misinterpreting the "FeatureVectorUAMem" bit.<br>
> The commit log says that change did not affect anything, but that's<br>
> not correct.<br>
> That change allowed SSE instructions to have unaligned mem operands<br>
> folded into math ops, and that's not allowed in the default<br>
> specification for any SSE variant.<br>
><br>
> The bug is exposed when compiling for an AVX-capable CPU that had this<br>
> feature flag but without enabling AVX codegen. Another mistake in<br>
> r224330 was not adding the feature flag to all AVX CPUs; the AMD chips<br>
> were excluded.<br>
><br>
> This is part of the fix for PR22371 (<br>
> <a href="http://llvm.org/bugs/show_bug.cgi?id=22371" target="_blank">http://llvm.org/bugs/show_bug.cgi?id=22371</a> ).<br>
><br>
> This feature bit is SSE-specific, so I've renamed it to<br>
> "FeatureSSEUnalignedMem".<br>
> Changed the existing test case for the feature bit to reflect the new<br>
> name and renamed the test file itself to better reflect the feature.<br>
> Added runs to fold-vex.ll to check for the failing codegen.<br>
><br>
> Note that the feature bit is not set by default on any CPU because it<br>
> may require a configuration register setting to enable the enhanced<br>
> unaligned behavior.<br>
><br>
><br>
> Sanjay requested in [1] that we should merge this to the 3.6 branch.<br>
> I'm not qualified to determine if this is safe or not. Is there somone<br>
> else with X86 expertise that would like to chime in? Nadav, you're<br>
> listed as the code owner here.<br>
><br>
> Thanks,<br>
> Hans<br>
><br>
> 1. <a href="http://llvm.org/bugs/show_bug.cgi?id=22374#c3" target="_blank">http://llvm.org/bugs/show_bug.cgi?id=22374#c3</a><br>
><br>
><br>
---------------------------------------------------------------------<br>
Intel Israel (74) Limited<br>
<br>
This e-mail and any attachments may contain confidential material for<br>
the sole use of the intended recipient(s). Any review or distribution<br>
by others is strictly prohibited. If you are not the intended<br>
recipient, please contact the sender and delete all copies.<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><u></u><u></u></p>
</div>
<p class="MsoNormal"><br>
<br clear="all">
<u></u><u></u></p>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<p class="MsoNormal">--
<u></u><u></u></p>
<div>
<p class="MsoNormal">~Craig<u></u><u></u></p>
</div>
</div>
</div>
</div>
</div>
<div>
<div>
<p>---------------------------------------------------------------------<br>
Intel Israel (74) Limited<u></u><u></u></p>
<p>This e-mail and any attachments may contain confidential material for<br>
the sole use of the intended recipient(s). Any review or distribution<br>
by others is strictly prohibited. If you are not the intended<br>
recipient, please contact the sender and delete all copies.<u></u><u></u></p>
</div>
</div>
</div>
</div>
<p class="MsoNormal"><br>
<br clear="all">
<u></u><u></u></p>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<p class="MsoNormal">-- <u></u><u></u></p>
<div>
<p class="MsoNormal">~Craig<u></u><u></u></p>
</div>
</div>
</div></div></div><div><div class="h5">
<p>---------------------------------------------------------------------<br>
Intel Israel (74) Limited</p>
<p>This e-mail and any attachments may contain confidential material for<br>
the sole use of the intended recipient(s). Any review or distribution<br>
by others is strictly prohibited. If you are not the intended<br>
recipient, please contact the sender and delete all copies.</p></div></div></div>
</blockquote></div><br></div>