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

[Hal Finkel via llvm-commits]

On 08/03/2017 11:10 AM, Matthias Braun via llvm-commits wrote:
> Looks good from my side.

LGTM too.

  -Hal

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