[PATCH] D58809: [LICM] Infer proper alignment from loads during scalar promotion

Philip Reames via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 28 19:24:31 PST 2019


reames created this revision.
reames added reviewers: asbirlea, sanjoy, skatkov, anna, jfb.
Herald added subscribers: jdoerfert, bollu, mcrosier.

This patch fixes an issue where we would compute an unnecessarily small alignment during scalar promotion when no store is not to be guaranteed to execute, but we've proven load speculation safety.  Since speculating a load requires proving the existing alignment is valid at the new location (see Loads.cpp), we can use the alignment fact from the load.

For non-atomics, this is a performance problem.  For atomics, this is a correctness issue, though an *incredibly* rare one to see in practice.  For atomics, we might not be able to lower an improperly aligned load or store (i.e. i32 align 1).  If such an instruction makes it all the way to codegen, we *may* fail to codegen the operation, or we may simply generate a slow call to a library function.  The part that makes this super hard to see in practice is that the memory location actually *is* well aligned, and instcombine knows that.  So, to see a failure, you have to have a) hit the bug in LICM, b) somehow hit a depth limit in InstCombine/ValueTracking to avoid fixing the alignment, and c) then have generated an instruction which fails codegen rather than simply emitting a slow libcall.  All around, pretty hard to hit.


https://reviews.llvm.org/D58809

Files:
  lib/Transforms/Scalar/LICM.cpp
  test/Transforms/LICM/promote-tls.ll
  test/Transforms/LICM/scalar-promote-unwind.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D58809.188840.patch
Type: text/x-patch
Size: 5139 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190301/f449c0bf/attachment.bin>


More information about the llvm-commits mailing list