<div dir="ltr">Hello Tom and Hal,<div><br></div><div>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.</div><div><br></div><div>Thanks,</div><div>George</div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Mar 28, 2018 at 5:54 PM, George Burgess IV via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Author: gbiv<br>
Date: Wed Mar 28 17:54:39 2018<br>
New Revision: 328748<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=328748&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject?rev=328748&view=rev</a><br>
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/Memor<wbr>ySSA/pr36883.ll<br>
Modified:<br>
llvm/trunk/lib/Analysis/Memory<wbr>SSA.cpp<br>
<br>
Modified: llvm/trunk/lib/Analysis/Memory<wbr>SSA.cpp<br>
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-pr<wbr>oject/llvm/trunk/lib/Analysis/<wbr>MemorySSA.cpp?rev=328748&r1=32<wbr>8747&r2=328748&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/lib/Analysis/Memory<wbr>SSA.cpp (original)<br>
+++ llvm/trunk/lib/Analysis/Memory<wbr>SSA.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/Memor<wbr>ySSA/pr36883.ll<br>
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-pr<wbr>oject/llvm/trunk/test/Analysis<wbr>/MemorySSA/pr36883.ll?rev=3287<wbr>48&view=auto</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/test/Analysis/Memor<wbr>ySSA/pr36883.ll (added)<br>
+++ llvm/trunk/test/Analysis/Memor<wbr>ySSA/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>,veri<wbr>fy<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.p0i8<wbr>(i8* %p, i32 2) #4 ; load CSE replacement<br>
+<br>
+; CHECK: 1 = MemoryDef(liveOnEntry)<br>
+ call void @llvm.arm.neon.vst1.p0i8.v8i16<wbr>(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.p0i8<wbr>(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.p0i8<wbr>(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.p0i8<wbr>(i8*, i32) #2<br>
+<br>
+; Function Attrs: argmemonly nounwind<br>
+declare void @llvm.arm.neon.vst1.p0i8.v8i16<wbr>(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>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.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><br>
</blockquote></div><br></div></div>