[llvm] r309651 - [StackColoring] Update AliasAnalysis information in stack coloring pass

[Matthias Braun via llvm-commits]

Looks good from my side.

> On Aug 3, 2017, at 9:08 AM, Hans Wennborg via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Matthias, Hal: Is this (r309651+r309849) good for merging to 5.0?
> 
> On Wed, Aug 2, 2017 at 11:20 AM, Hiroshi 7 Inoue <INOUEHRS at jp.ibm.com> wrote:
>> Hal, Matthias,
>> 
>> Committed in https://reviews.llvm.org/rL309849.
>> Thank you so much for your reviews.
>> 
>>> In the future, please don't commit a non-
>>> trivial change like this without an explicit approval.
>> Sure.
>> 
>> -----
>> Hiroshi Inoue <inouehrs at jp.ibm.com>
>> IBM Research - Tokyo
>> 
>> 
>> Hal Finkel <hfinkel at anl.gov> wrote on 2017/08/02 20:53:12:
>> 
>>> From: Hal Finkel <hfinkel at anl.gov>
>>> To: Hiroshi 7 Inoue <INOUEHRS at jp.ibm.com>, Hans Wennborg
>>> <hans at chromium.org>
>>> Cc: <hwennborg at google.com>, llvm-commits <llvm-
>>> commits at lists.llvm.org>, Matthias Braun <matze at braunis.de>
>>> Date: 2017/08/02 21:04
>>> Subject: Re: [llvm] r309651 - [StackColoring] Update AliasAnalysis
>>> information in stack coloring pass
>>> 
>> 
>>> On 08/02/2017 03:16 AM, Hiroshi 7 Inoue wrote:
>> 
>>> Hi,
>>> 
>>> I updated my patch in Phabricator based on comments from Hal and Matthias.
>>> https://reviews.llvm.org/D35907
>>> 
>>> I will commit the new patch if it looks ok for everyone.
>>> 
>>> Please commit the update. In the future, please don't commit a non-
>>> trivial change like this without an explicit approval.
>>> 
>>> Thanks again,
>>> Hal
>> 
>>> Thank you.
>>> 
>>> Hiroshi
>>> 
>>> -----
>>> Hiroshi Inoue <inouehrs at jp.ibm.com>
>>> IBM Research - Tokyo
>>> 
>>> 
>>> hwennborg at google.com wrote on 2017/08/02 08:41:33:
>>> 
>>>> From: Hans Wennborg <hans at chromium.org>
>>>> To: Hal Finkel <hfinkel at anl.gov>
>>>> Cc: Matthias Braun <matze at braunis.de>, Hiroshi Inoue
>>>> <inouehrs at jp.ibm.com>, llvm-commits <llvm-commits at lists.llvm.org>
>>>> Date: 2017/08/02 08:41
>>>> Subject: Re: [llvm] r309651 - [StackColoring] Update AliasAnalysis
>>>> information in stack coloring pass
>>>> Sent by: hwennborg at google.com
>>>> 
>>>> On Tue, Aug 1, 2017 at 10:51 AM, Hal Finkel <hfinkel at anl.gov> wrote:
>>>>> 
>>>>> On 08/01/2017 12:38 PM, Matthias Braun wrote:
>>>>> 
>>>>> This is going a little fast, given that Hal had raised concerns and
>>>>> the
>>>>> patch went in before he responded (at least it looks like that
>>> in phab). I'd
>>>>> like to hear his opinion first, given that my experience with
>>> alias analysis
>>>>> in LLVM is limited.
>>>>> 
>>>>> 
>>>>> I'd like to see this in 5.0, however, I just provided a post-commit
>>>>> review
>>>>> and it looks like it will need a small update before merging.
>>>> 
>>>> Sounds good. Thanks!
>>>> 
>>>> 
>>>> 
>>>>> On Aug 1, 2017, at 10:15 AM, Hans Wennborg via llvm-commits
>>>>> <llvm-commits at lists.llvm.org> wrote:
>>>>> 
>>>>> Matthias, the PR for this was marked as a release blocker for 5.0.
>>>>> What do you think about merging this?
>>>>> 
>>>>> On Mon, Jul 31, 2017 at 8:32 PM, Hiroshi Inoue via llvm-commits
>>>>> <llvm-commits at lists.llvm.org> wrote:
>>>>> 
>>>>> Author: inouehrs
>>>>> Date: Mon Jul 31 20:32:15 2017
>>>>> New Revision: 309651
>>>>> 
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=309651&view=rev
>>>>> Log:
>>>>> [StackColoring] Update AliasAnalysis information in stack coloring
>>>>> pass
>>>>> 
>>>>> Stack coloring pass need to maintain AliasAnalysis information
>>> when merging
>>>>> stack slots of different types.
>>>>> Actually, there is a FIXME comment in StackColoring.cpp
>>>>> 
>>>>> // FIXME: In order to enable the use of TBAA when using AA in CodeGen,
>>>>> // we'll also need to update the TBAA nodes in MMOs with values
>>>>> // derived from the merged allocas.
>>>>> 
>>>>> But, TBAA has been already enabled in CodeGen without fixing this
>>>>> pass.
>>>>> The incorrect TBAA metadata results in recent failures in
>>> bootstrap test on
>>>>> ppc64le (PR33928) by allowing unsafe instruction scheduling.
>>>>> Although we observed the problem on ppc64le, this is a platform
>>>>> neutral
>>>>> issue.
>>>>> 
>>>>> This patch makes the stack coloring pass maintains AliasAnalysis
>>> information
>>>>> when merging multiple stack slots.
>>>>> 
>>>>> 
>>>>> (This is https://reviews.llvm.org/D35907)
>>>>> _______________________________________________
>>>>> llvm-commits mailing list
>>>>> llvm-commits at lists.llvm.org
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>> 
>>>>> 
>>>>> 
>>>>> --
>>>>> Hal Finkel
>>>>> Lead, Compiler Technology and Programming Languages
>>>>> Leadership Computing Facility
>>>>> Argonne National Laboratory
>>>> 
>> 
>>> 
>>> --
>>> Hal Finkel
>>> Lead, Compiler Technology and Programming Languages
>>> Leadership Computing Facility
>>> Argonne National Laboratory
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits