<div dir="ltr">Will the folding tables in X86InstrInfo somewhat mitigate this? I'm sure it won't be perfect, but maybe offers some protection.</div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Feb 5, 2015 at 1:29 PM, Hans Wennborg <span dir="ltr"><<a href="mailto:hans@chromium.org" target="_blank">hans@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Is it a miscompile (i.e. it's now folding to something illegal) or a<br>
performance (failing to fold to something that would be better) problem?<br>
<br>
- Hans<br>
<br>
On Thu, Feb 5, 2015 at 1:12 PM, Demikhovsky, Elena<br>
<div class="HOEnZb"><div class="h5"><<a href="mailto:elena.demikhovsky@intel.com">elena.demikhovsky@intel.com</a>> wrote:<br>
> It broke folding unaligned loads in instructions.<br>
><br>
> - Elena<br>
><br>
><br>
> -----Original Message-----<br>
> From: <a href="mailto:hwennborg@google.com">hwennborg@google.com</a> [mailto:<a href="mailto:hwennborg@google.com">hwennborg@google.com</a>] On Behalf Of Hans Wennborg<br>
> Sent: Thursday, February 05, 2015 22:52<br>
> To: Demikhovsky, Elena<br>
> Cc: Sanjay Patel; Craig Topper; llvm-commits; Adam Nemet (<a href="mailto:anemet@apple.com">anemet@apple.com</a>); Kuperstein, Michael M; Nadav Rotem<br>
> Subject: Re: [llvm] r227983 - Fix program crashes due to alignment exceptions generated for SSE memop instructions (PR22371).<br>
><br>
> I don't have enough expertise in this area of the code to quite understand what's broken. As far as I understand, Sanjay's patch fixed most of PR22374. What aspect of AVX-512 did it break?<br>
><br>
> On Thu, Feb 5, 2015 at 12:03 PM, Demikhovsky, Elena <<a href="mailto:elena.demikhovsky@intel.com">elena.demikhovsky@intel.com</a>> wrote:<br>
>> I know that the AVX-512 will be incorrect with this patch. I can add a<br>
>> test that will fail, but I can’t rewrite in one day all AVX-512 instructions.<br>
>><br>
>><br>
>><br>
>> - Elena<br>
>><br>
>><br>
>><br>
>> From: Sanjay Patel [mailto:<a href="mailto:spatel@rotateright.com">spatel@rotateright.com</a>]<br>
>> Sent: Thursday, February 05, 2015 18:03<br>
>> To: Demikhovsky, Elena<br>
>> Cc: Craig Topper; llvm-commits; Adam Nemet (<a href="mailto:anemet@apple.com">anemet@apple.com</a>);<br>
>> Kuperstein, Michael M; Nadav Rotem; Hans W<br>
>><br>
>><br>
>> Subject: Re: [llvm] r227983 - Fix program crashes due to alignment<br>
>> exceptions generated for SSE memop instructions (PR22371).<br>
>><br>
>><br>
>><br>
>> The intent of r227983 is simply to revert the wrong behavior<br>
>> introduced by<br>
>> r224330 for *SSE* codegen.<br>
>> If r224330 was supposed to change behavior of AVX512, it should have<br>
>> included test cases to validate that, right? These should be separate<br>
>> patches.<br>
>><br>
>>> You removed FeatureVectorUAMem form HSW, but you did not add<br>
>>> FeatureSSEUnalignedMem<br>
>><br>
>> Correct - AFAIK, Haswell doesn't have this feature. Perhaps we need to<br>
>> change 'memop' to 'sse_memop' to make it clearer that memops are only<br>
>> for use with SSE...or just modify the 'load' defs to include this<br>
>> predicate<br>
>> 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<br>
>> subtarget // allows unaligned accesses, match any load, though this<br>
>> may require // 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>
>><br>
>><br>
>> On Thu, Feb 5, 2015 at 1:11 AM, Demikhovsky, Elena<br>
>> <<a href="mailto:elena.demikhovsky@intel.com">elena.demikhovsky@intel.com</a>> wrote:<br>
>><br>
>> It can use load, I agree.<br>
>><br>
>> But AVX512 code should be changed before committing the patch.<br>
>><br>
>><br>
>><br>
>> - Elena<br>
>><br>
>><br>
>><br>
>> From: Craig Topper [mailto:<a href="mailto:craig.topper@gmail.com">craig.topper@gmail.com</a>]<br>
>> Sent: Thursday, February 05, 2015 10:04<br>
>><br>
>><br>
>> To: Demikhovsky, Elena<br>
>> Cc: Sanjay Patel; llvm-commits<br>
>> Subject: Re: [llvm] r227983 - Fix program crashes due to alignment<br>
>> exceptions generated for SSE memop instructions (PR22371).<br>
>><br>
>><br>
>><br>
>> Why is AVX512 not using load?<br>
>><br>
>><br>
>><br>
>> On Wed, Feb 4, 2015 at 11:59 PM, Demikhovsky, Elena<br>
>> <<a href="mailto:elena.demikhovsky@intel.com">elena.demikhovsky@intel.com</a>> wrote:<br>
>><br>
>> We use it in AVX512.<br>
>><br>
>><br>
>><br>
>> - Elena<br>
>><br>
>><br>
>><br>
>> From: Craig Topper [mailto:<a href="mailto:craig.topper@gmail.com">craig.topper@gmail.com</a>]<br>
>> Sent: Thursday, February 05, 2015 09:52<br>
>> To: Demikhovsky, Elena<br>
>> Cc: Sanjay Patel; llvm-commits<br>
>><br>
>><br>
>> Subject: Re: [llvm] r227983 - Fix program crashes due to alignment<br>
>> exceptions generated for SSE memop instructions (PR22371).<br>
>><br>
>><br>
>><br>
>> I thought we had converted all of AVX/AVX2 to use 'loadXXX' instead of<br>
>> 'memopXXX'?<br>
>><br>
>><br>
>><br>
>> On Wed, Feb 4, 2015 at 11:44 PM, Demikhovsky, Elena<br>
>> <<a href="mailto:elena.demikhovsky@intel.com">elena.demikhovsky@intel.com</a>> wrote:<br>
>><br>
>> Hi,<br>
>> You removed FeatureVectorUAMem form HSW, but you did not add<br>
>> 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>
>> - 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<br>
>> exceptions generated for SSE memop instructions (PR22371).<br>
>><br>
>> I understand this on roughly the same level as Nadav does (looks<br>
>> correct, and should be safer than the current situation), but Elena<br>
>> 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">hwennborg@google.com</a> [mailto:<a href="mailto:hwennborg@google.com">hwennborg@google.com</a>] On Behalf Of<br>
>> Hans Wennborg<br>
>> Sent: Wednesday, February 04, 2015 20:53<br>
>> To: Nadav Rotem<br>
>> Cc: Sanjay Patel; llvm-commits; Demikhovsky, Elena; Kuperstein,<br>
>> Michael M<br>
>> Subject: Re: [llvm] r227983 - Fix program crashes due to alignment<br>
>> 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">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">hans@chromium.org</a>> wrote:<br>
>>><br>
>>> On Tue, Feb 3, 2015 at 9:13 AM, Sanjay Patel <<a href="mailto:spatel@rotateright.com">spatel@rotateright.com</a>><br>
>>> 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<br>
>>> this feature flag but without enabling AVX codegen. Another mistake<br>
>>> in<br>
>>> r224330 was not adding the feature flag to all AVX CPUs; the AMD<br>
>>> chips 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<br>
>>> somone else with X86 expertise that would like to chime in? Nadav,<br>
>>> you're 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">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><br>
>><br>
>><br>
>><br>
>><br>
>><br>
>> --<br>
>><br>
>> ~Craig<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>
>><br>
>><br>
>><br>
>> --<br>
>><br>
>> ~Craig<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>
>><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>
> 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>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature">~Craig</div>
</div>