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

via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 26 14:30:27 PDT 2016

On 2016-10-26 15:39, Daniel Berlin via llvm-commits wrote:
> 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 [1], @mcrosier wrote:
>>> In https://reviews.llvm.org/D25969#579967 [2], @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

Right.  LICM doesn't explicitly check for the lifetime intrinsics.  
Rather, the lifetime intrinsics cause the AliasSetTracker to collapse 
sets, which indirectly changes the behavior of LICM with uses ASTs.

>> 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?

I assumed creating stores to the array after the lifetime of the array 
had ended was incorrect, but I guess that's not really true given the 
alloc does not live inside the loop.

> 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).

That sounds reasonable.

> 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

Yes, I agree.  That was the intend of the patch, but I didn't foresee th

>> 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.

Honestly, this is something I saw in passing and I'm not sure I want to 
tackle this problem at th e moment.  However, I'll make sure to create a 
details PR in the morning.  Perhaps, I'll have some free time in the 

Thanks for the feedback Danny.


>> Repository:
>> rL LLVM
>> https://reviews.llvm.org/D25969 [3]
> Links:
> ------
> [1] https://reviews.llvm.org/D25969#580042
> [2] https://reviews.llvm.org/D25969#579967
> [3] https://reviews.llvm.org/D25969
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

More information about the llvm-commits mailing list