<div dir="ltr">Thanks, Hal!<div><br></div><div>Is this OK with you, too, Tom?</div><div><br></div><div>George</div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Mar 30, 2018 at 1:33 PM, Hal Finkel <span dir="ltr"><<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div text="#000000" bgcolor="#FFFFFF"><span>
<p><br>
</p>
<div class="m_881687128802447687m_-7606933222359996600m_7342415150583503276moz-cite-prefix">On 03/30/2018 03:04 PM, George Burgess
IV wrote:<br>
</div>
<blockquote type="cite">
<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>
</blockquote>
<br></span>
I agree. Pulling this (along with r328755) into the release branch
is appropriate.<br>
<br>
-Hal<div><div class="m_881687128802447687m_-7606933222359996600h5"><br>
<br>
<blockquote type="cite">
<div dir="ltr">
<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>
</blockquote>
<br>
</div></div><span class="m_881687128802447687m_-7606933222359996600HOEnZb"><font color="#888888"><pre class="m_881687128802447687m_-7606933222359996600m_7342415150583503276moz-signature" cols="72">--
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory</pre>
</font></span></div>
</blockquote></div><br></div></div>