<div dir="ltr">Thank you very much!<div><br></div><div>Can you please pick r328755, as well? It's a tiny but necessary bugfix for this. :)</div><div><br></div><div>Thanks again,</div><div>George</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Apr 9, 2018 at 7:42 PM, Tom Stellard <span dir="ltr"><<a href="mailto:tstellar@redhat.com" target="_blank">tstellar@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 04/09/2018 07:13 PM, George Burgess IV wrote:<br>
> Thanks, Hal!<br>
><br>
> Is this OK with you, too, Tom?<br>
><br>
<br>
</span>Yes, I've merged this: r329663.<br>
<br>
-Tom<br>
<br>
> George<br>
<span class="">><br>
> On Fri, Mar 30, 2018 at 1:33 PM, Hal Finkel <<a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a> <mailto:<a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>>> wrote:<br>
><br>
><br>
> On 03/30/2018 03:04 PM, George Burgess IV wrote:<br>
>> Hello Tom and Hal,<br>
>><br>
>> I would like approval to pick this (and its follow-up, which I'll ping you both about in a sec :) ) into the 6.0 release branch. It fixes a misoptimization that dates back to 5.0, but since 5.0.2rc2 was just cut, I think it's best to leave that as-is.<br>
><br>
> I agree. Pulling this (along with r328755) into the release branch is appropriate.<br>
><br>
> -Hal<br>
><br>
><br>
>><br>
>> Thanks,<br>
>> George<br>
>><br>
</span><span class="">>> On Wed, Mar 28, 2018 at 5:54 PM, George Burgess IV via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a> <mailto:<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.<wbr>llvm.org</a>>> wrote:<br>
>><br>
>> Author: gbiv<br>
>> Date: Wed Mar 28 17:54:39 2018<br>
>> New Revision: 328748<br>
>><br>
</span>>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=328748&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=328748&view=rev</a> <<a href="http://llvm.org/viewvc/llvm-project?rev=328748&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=328748&view=rev</a>><br>
<span class="">>> Log:<br>
>> [MemorySSA] Consider callsite args for hashing and equality.<br>
>><br>
>> We use a `DenseMap<MemoryLocOrCall, MemlocStackInfo>` to keep track of<br>
>> prior work when optimizing uses in MemorySSA. Because we weren't<br>
>> accounting for callsite arguments in either the hash code or equality<br>
>> tests for `MemoryLocOrCall`s, we optimized uses too aggressively in<br>
>> some rare cases.<br>
>><br>
>> Fix by Daniel Berlin.<br>
>><br>
>> Should fix PR36883.<br>
>><br>
>> Added:<br>
>> llvm/trunk/test/Analysis/<wbr>MemorySSA/pr36883.ll<br>
>> Modified:<br>
>> llvm/trunk/lib/Analysis/<wbr>MemorySSA.cpp<br>
>><br>
>> Modified: llvm/trunk/lib/Analysis/<wbr>MemorySSA.cpp<br>
</span>>> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/MemorySSA.cpp?rev=328748&r1=328747&r2=328748&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/lib/<wbr>Analysis/MemorySSA.cpp?rev=<wbr>328748&r1=328747&r2=328748&<wbr>view=diff</a> <<a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/MemorySSA.cpp?rev=328748&r1=328747&r2=328748&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/lib/<wbr>Analysis/MemorySSA.cpp?rev=<wbr>328748&r1=328747&r2=328748&<wbr>view=diff</a>><br>
<div><div class="h5">>> ==============================<wbr>==============================<wbr>==================<br>
>> --- llvm/trunk/lib/Analysis/<wbr>MemorySSA.cpp (original)<br>
>> +++ llvm/trunk/lib/Analysis/<wbr>MemorySSA.cpp Wed Mar 28 17:54:39 2018<br>
>> @@ -153,9 +153,14 @@ public:<br>
>> if (IsCall != Other.IsCall)<br>
>> return false;<br>
>><br>
>> - if (IsCall)<br>
>> - return CS.getCalledValue() == Other.CS.getCalledValue();<br>
>> - return Loc == Other.Loc;<br>
>> + if (!IsCall)<br>
>> + return Loc == Other.Loc;<br>
>> +<br>
>> + if (CS.getCalledValue() != Other.CS.getCalledValue())<br>
>> + return false;<br>
>> +<br>
>> + assert(CS.arg_size() == Other.CS.arg_size());<br>
>> + return std::equal(CS.arg_begin(), CS.arg_end(), Other.CS.arg_begin());<br>
>> }<br>
>><br>
>> private:<br>
>> @@ -179,12 +184,18 @@ template <> struct DenseMapInfo<MemoryLo<br>
>> }<br>
>><br>
>> static unsigned getHashValue(const MemoryLocOrCall &MLOC) {<br>
>> - if (MLOC.IsCall)<br>
>> - return hash_combine(MLOC.IsCall,<br>
>> - DenseMapInfo<const Value *>::getHashValue(<br>
>> - MLOC.getCS().getCalledValue())<wbr>);<br>
>> - return hash_combine(<br>
>> - MLOC.IsCall, DenseMapInfo<MemoryLocation>::<wbr>getHashValue(MLOC.getLoc()));<br>
>> + if (!MLOC.IsCall)<br>
>> + return hash_combine(<br>
>> + MLOC.IsCall,<br>
>> + DenseMapInfo<MemoryLocation>::<wbr>getHashValue(MLOC.getLoc()));<br>
>> +<br>
>> + hash_code hash =<br>
>> + hash_combine(MLOC.IsCall, DenseMapInfo<const Value *>::getHashValue(<br>
>> + MLOC.getCS().getCalledValue())<wbr>);<br>
>> +<br>
>> + for (const Value *Arg : MLOC.getCS().args())<br>
>> + hash = hash_combine(hash, DenseMapInfo<const Value *>::getHashValue(Arg));<br>
>> + return hash;<br>
>> }<br>
>><br>
>> static bool isEqual(const MemoryLocOrCall &LHS, const MemoryLocOrCall &RHS) {<br>
>><br>
>> Added: llvm/trunk/test/Analysis/<wbr>MemorySSA/pr36883.ll<br>
</div></div>>> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/MemorySSA/pr36883.ll?rev=328748&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/test/<wbr>Analysis/MemorySSA/pr36883.ll?<wbr>rev=328748&view=auto</a> <<a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/MemorySSA/pr36883.ll?rev=328748&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/test/<wbr>Analysis/MemorySSA/pr36883.ll?<wbr>rev=328748&view=auto</a>><br>
<div><div class="h5">>> ==============================<wbr>==============================<wbr>==================<br>
>> --- llvm/trunk/test/Analysis/<wbr>MemorySSA/pr36883.ll (added)<br>
>> +++ llvm/trunk/test/Analysis/<wbr>MemorySSA/pr36883.ll Wed Mar 28 17:54:39 2018<br>
>> @@ -0,0 +1,38 @@<br>
>> +; RUN: opt -basicaa -memoryssa -analyze < %s 2>&1 -S | FileCheck %s<br>
>> +; RUN: opt -aa-pipeline=basic-aa -passes='print<memoryssa>,<wbr>verify<memoryssa>' -S < %s 2>&1 | FileCheck %s<br>
>> +;<br>
>> +; We weren't properly considering the args in callsites in equality or hashing.<br>
>> +<br>
>> +target triple = "armv7-dcg-linux-gnueabi"<br>
>> +<br>
>> +; CHECK-LABEL: define <8 x i16> @vpx_idct32_32_neon<br>
>> +define <8 x i16> @vpx_idct32_32_neon(i8* %p, <8 x i16> %v) {<br>
>> +entry:<br>
>> +; CHECK: MemoryUse(liveOnEntry)<br>
>> + %load1 = call <8 x i16> @llvm.arm.neon.vld1.v8i16.<wbr>p0i8(i8* %p, i32 2) #4 ; load CSE replacement<br>
>> +<br>
>> +; CHECK: 1 = MemoryDef(liveOnEntry)<br>
>> + call void @llvm.arm.neon.vst1.p0i8.<wbr>v8i16(i8* %p, <8 x i16> %v, i32 2) #4 ; clobber<br>
>> +<br>
>> + %p_next = getelementptr inbounds i8, i8* %p, i32 16<br>
>> +; CHECK: MemoryUse(liveOnEntry)<br>
>> + %load2 = call <8 x i16> @llvm.arm.neon.vld1.v8i16.<wbr>p0i8(i8* %p_next, i32 2) #4 ; non-aliasing load needed to trigger bug<br>
>> +<br>
>> +; CHECK: MemoryUse(1)<br>
>> + %load3 = call <8 x i16> @llvm.arm.neon.vld1.v8i16.<wbr>p0i8(i8* %p, i32 2) #4 ; load CSE removed<br>
>> +<br>
>> + %add = add <8 x i16> %load1, %load2<br>
>> + %ret = add <8 x i16> %add, %load3<br>
>> + ret <8 x i16> %ret<br>
>> +}<br>
>> +<br>
>> +; Function Attrs: argmemonly nounwind readonly<br>
>> +declare <8 x i16> @llvm.arm.neon.vld1.v8i16.<wbr>p0i8(i8*, i32) #2<br>
>> +<br>
>> +; Function Attrs: argmemonly nounwind<br>
>> +declare void @llvm.arm.neon.vst1.p0i8.<wbr>v8i16(i8*, <8 x i16>, i32) #1<br>
>> +<br>
>> +attributes #1 = { argmemonly nounwind }<br>
>> +attributes #2 = { argmemonly nounwind readonly }<br>
>> +attributes #3 = { nounwind readnone }<br>
>> +attributes #4 = { nounwind }<br>
>><br>
>><br>
>> ______________________________<wbr>_________________<br>
>> llvm-commits mailing list<br>
</div></div>>> <a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a> <mailto:<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.<wbr>llvm.org</a>><br>
>> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a> <<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-<wbr>bin/mailman/listinfo/llvm-<wbr>commits</a>><br>
<div class="HOEnZb"><div class="h5">>><br>
>><br>
><br>
> --<br>
> Hal Finkel<br>
> Lead, Compiler Technology and Programming Languages<br>
> Leadership Computing Facility<br>
> Argonne National Laboratory<br>
><br>
><br>
<br>
</div></div></blockquote></div><br></div>