[llvm-dev] [RFC] [X86] Emit unaligned vector moves on avx machine with option control.

Philip Reames via llvm-dev llvm-dev at lists.llvm.org
Thu Apr 15 08:36:48 PDT 2021


IMO, no.  We should encourage sanitizers instead.

 From experience, any code base where porting trips across this probably 
also has a bunch of other undefined behavior which is causing less 
obvious miscompiles, and also need found and fixed. That's why we have 
sanitizers.

Philip

On 4/15/21 7:07 AM, Luo, Yuanke via llvm-dev wrote:
>
> Yes, replacing aligned move instruction with unaligned move 
> instruction doesn’t solve all the issue that happens in optimization 
> pipeline, but it doesn’t make things worse. One advantage for 
> unaligned move is that it makes the behavior the same no matter the 
> mov instruction is folded or not. Do you think it is worth to support 
> this feature if compiler can help users avoid changing their complex 
> legacy code?
>
> Thanks
>
> Yuanke
>
> *From:* James Y Knight <jyknight at google.com>
> *Sent:* Thursday, April 15, 2021 9:09 PM
> *To:* Liu, Chen3 <chen3.liu at intel.com>
> *Cc:* llvm-dev at lists.llvm.org; Luo, Yuanke <yuanke.luo at intel.com>; 
> Maslov, Sergey V <sergey.v.maslov at intel.com>
> *Subject:* Re: [llvm-dev] [RFC] [X86] Emit unaligned vector moves on 
> avx machine with option control.
>
> On Thu, Apr 15, 2021 at 4:43 AM Liu, Chen3 <chen3.liu at intel.com 
> <mailto:chen3.liu at intel.com>> wrote:
>
>     Hi, James Y Knight.
>
>     I'm not sure if you misunderstood this patch. This patch won’t
>     change any alignment information in IR and MI, which means
>     ‘load…align 32’ will always keep the alignment information but
>     select ‘vmovups’ instead of ‘vmovaps’ during ISEL. It can be
>     simply considered that the only thing this patch does is to
>     replace the aligned-move mnemonic with the unaligned-move mnemonic
>     (in fact, we shouldn’t call it replace but emit unaligned). I
>     think there is no impact on optimization or code layout.
>
> Yes -- I understood that, and that is exactly why this patch is not 
> OK. Giving LLVM incorrect information about the alignment of 
> objects causes problems other than just the emission of movaps 
> instructions -- that alignment information is correct gets relied upon 
> throughout the optimization pipeline.
>
> So, a command-line option to "fix" only that one instruction is not 
> something which we can reasonably provide, because it will not 
> reliably fix users' problems. A program which is being "mis"-compiled 
> due to the use of misaligned objects might still be miscompiled by 
> LLVM when using your proposed patch. ("mis" in quotes, since the 
> compiler is correctly compiling the code according to the standard, 
> even if not according to the user's expectations).
>
> The second paragraph of my original email describes an alternative 
> patch that you could write, which /would/ reliably fix such 
> miscompilation -- effectively creating a variant of C where creating 
> and accessing misaligned objects has fully defined behavior. (And, 
> just to reiterate, my initial feeling is that creating such an option 
> is not a worthwhile endeavor, but I could be persuaded otherwise.)
>
>     After discussion, we think this option more like changing the
>     behavior when process with unaligned memory: raising exception or
>     accepting performance degradation.  Maybe the option is more like
>     “no-exception-on-unalginedmem”. We do have some users want this
>     feature. They can accept “run slow” but do not want exception.
>
>     Thanks.
>
>     Chen Liu.
>
>     *From:* Philip Reames <listmail at philipreames.com
>     <mailto:listmail at philipreames.com>>
>     *Sent:* Thursday, April 15, 2021 6:44 AM
>     *To:* James Y Knight <jyknight at google.com
>     <mailto:jyknight at google.com>>; Liu, Chen3 <chen3.liu at intel.com
>     <mailto:chen3.liu at intel.com>>
>     *Cc:* llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>;
>     Luo, Yuanke <yuanke.luo at intel.com <mailto:yuanke.luo at intel.com>>;
>     Maslov, Sergey V <sergey.v.maslov at intel.com
>     <mailto:sergey.v.maslov at intel.com>>
>     *Subject:* Re: [llvm-dev] [RFC] [X86] Emit unaligned vector moves
>     on avx machine with option control.
>
>     +1 to what James said.  My reaction to the original proposal is a
>     strong -1, and James did a good job of explaining why.
>
>     Philip
>
>     On 4/14/21 11:57 AM, James Y Knight via llvm-dev wrote:
>
>         This is not a principled change -- it avoids a problem arising
>         from /one/ use of alignment information, but there are other
>         uses of alignment in LLVM, and those will still cause
>         problems, potentially less clearly. So, I think that this will
>         not be a useful option to provide to users, in this form.
>
>         What I suspect you /actually/ want here is an option to tell
>         Clang not to infer load/store alignments based on object types
>         or alignment attributes -- instead treating everything as
>         being potentially aligned to 1 unless the allocation is seen
>         (e.g. global/local variables). Clang would still need to use
>         the usual alignment computation for variable definitions and
>         structure layout, but not memory operations. If clang emits
>         "load ... align 1" instructions in LLVM IR, the right thing
>         would then happen in the X86 backend automatically.
>
>         My initial inclination is that this feature is also not
>         particularly worthwhile to implement, but I'm open to being
>         convinced that this is indeed valuable enough to be
>         worthwhile. It should actually work reliably, and is somewhat
>         in line with other such "not-quite-C" flags we provide (e.g.
>         -fno-delete-null-pointer-checks). Of course, even with such an
>         implementation, you can still have a problem with user code
>         depending on alignof() returning a reliable answer (e.g.,
>         llvm::PointerUnion). Not much can be done about that.
>
>         On Wed, Apr 14, 2021 at 2:07 PM Liu, Chen3 via llvm-dev
>         <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
>
>             Hi all.
>
>             We want to make a patch to always emit unaligned vector
>             move instructions on AVX machine with option control. We
>             do this for the following reason:
>
>              1. With AVX the performance for aligned vector move and
>                 unaligned vector move on X86 are the same if the
>                 address is aligned. In this case we prefer to use
>                 unaligned move because it can avoid some run time
>                 exceptions;
>              2. This fixes an inconsistency in optimization: suppose a
>                 load operation was merged into another instruction
>                 (e.g., load and add becomes `add [memop]'). If a
>                 misaligned pointer is passed to the two-instruction
>                 sequence, it will
>
>             raise an exception. If the same pointer is passed to the
>             memop instruction, it will work. Thus, the behavior of
>             misalignment depends upon what optimization levels and
>             passes are applied, and small source changes could cause
>
>             issues to appear and disappear. It's better for the user
>             to consistently use unaligned load/store to improve the
>             debug experience;
>
>              3. Makes good use of HW that is capable of handling
>                 misaligned data gracefully. It is not necessarily a
>                 bug in users code but a third-part library. For
>                 example it would allow using a library built in old
>                 ages where stack alignment was 4-byte only.
>              4. Compatible with ICC so that users can easily use llvm;
>
>             Roman Lebedev is worried that this patch will hide UB. In
>             our opinions, UB doesn't have to mean raise an exception.
>             The example code(https://godbolt.org/z/43bYPraoa
>             <https://godbolt.org/z/43bYPraoa>) does have UB behavior
>             but it is still valid (and reasonable) to interpret that
>             UB as `go slower',
>
>             instead of `raise exception'. Besides, as default we still
>             emit aligned instructions as before,  but we provide an
>             option for users with this need.
>
>             We have two patches discussing this issue, one of which
>             has been abandoned:
>
>             https://reviews.llvm.org/D88396
>             <https://reviews.llvm.org/D88396> (abandoned)
>
>             https://reviews.llvm.org/D99565
>             <https://reviews.llvm.org/D99565> (in review)
>
>             Thanks.
>
>             Chen Liu.
>
>             _______________________________________________
>             LLVM Developers mailing list
>             llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>
>             https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>             <https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
>
>         _______________________________________________
>
>         LLVM Developers mailing list
>
>         llvm-dev at lists.llvm.org  <mailto:llvm-dev at lists.llvm.org>
>
>         https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev  <https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210415/63a70683/attachment-0001.html>


More information about the llvm-dev mailing list