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

[Hans Wennborg via llvm-commits]

On Thu, Aug 3, 2017 at 9:22 AM, Hal Finkel <hfinkel at anl.gov> wrote:
>
> On 08/03/2017 11:10 AM, Matthias Braun via llvm-commits wrote:
>>
>> Looks good from my side.
>
>
> LGTM too.

Merged them both as r309957. Thanks everyone!


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