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

</body></html>