[PATCH] D25969: [AliasSetTracker] Make AST smarter about intrinsics that don't actually affect memory.

[Daniel Berlin via llvm-commits]

On Wed, Oct 26, 2016 at 12:28 PM, Chad Rosier <mcrosier at codeaurora.org>
wrote:

> mcrosier added a comment.
>
> In https://reviews.llvm.org/D25969#580042, @mcrosier wrote:
>
> > In https://reviews.llvm.org/D25969#579967, @nemanjai wrote:
> >
> > > This caused a failure on all PPC machines I have access to:
> > >  AddressSanitizer-powerpc64le-linux :: TestCases/use-after-scope-
> loop-bug.cc
> >
> >
> > Looking now..
>
>
> I believe I've figured out the underlying problem.  In short, LICM is
> relying on lifetime markers to prevent loads/stores from being moved out of
> a particular scope.
>
>
Can you file more details on a bug somewhere? This seems pretty broken, but
also, without your patch:
dannyb at dannyb-macbookpro3 [12:32:05]
[~/sources/llvm-clean/lib/Transforms/Scalar] [mssa-batch-renamer-2]
-> % grep -i lifetime LICM.cpp





> From the broken test case:
> int main() {
>
>   // Variable goes in and out of scope.
>   for (int i = 0; i < 3; ++i) {
>     int x[3] = {i, i, i};
>     p = x + i;
>   }
>   return *p;  // BOOM
>
> }
>
> LICM will sink the stores to x[3] after the loop, which is obviously
> incorrect.


I don't see why?
First, let's ignore the c-semantics, it's broken as-written in c, since the
variable is dead whether you sink the stores or not.  (ASAN is a separate
issue and should disable moving past lifetime markers or something)

Assuming p is a pointer, sinking the stores to before *p is just as fine as
it is now, as long as you move the lifetime start and end markers to go
with it (or drop them).

I presume the alloca does not occur in the loop, only the lifetime start
and end.
In particular, you should be able to simply drop all lifetime start and
end, and get the same result.

IE lifetime start and end do not serve as a dependence here, only metadata.
If the only thing preventing LICM is the lifetime markers, that's very
wrong since they are non-guaranteed metadata


I've reverted the change in r285227.  Sorry for the breakage.
>
> You should be able to fix this simply by either
1. dropping the lifetime markers (non-ideal) of things you sink.
or
2.  extending the lifetime markers appropriately.

The algorithm for that is that when you eliminate or sink things past them,
you move the end to wherever the lowest point you sunk to is.
You move starts to a block that post-dominates both the new end and the old
end.



>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D25969
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161026/1de86d2e/attachment.html>