[llvm] r328748 - [MemorySSA] Consider callsite args for hashing and equality.
Hal Finkel via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 30 13:33:49 PDT 2018
On 03/30/2018 03:04 PM, George Burgess IV wrote:
> Hello Tom and Hal,
>
> 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.
I agree. Pulling this (along with r328755) into the release branch is
appropriate.
-Hal
>
> Thanks,
> George
>
> On Wed, Mar 28, 2018 at 5:54 PM, George Burgess IV via llvm-commits
> <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
>
> Author: gbiv
> Date: Wed Mar 28 17:54:39 2018
> New Revision: 328748
>
> URL: http://llvm.org/viewvc/llvm-project?rev=328748&view=rev
> <http://llvm.org/viewvc/llvm-project?rev=328748&view=rev>
> Log:
> [MemorySSA] Consider callsite args for hashing and equality.
>
> We use a `DenseMap<MemoryLocOrCall, MemlocStackInfo>` to keep track of
> prior work when optimizing uses in MemorySSA. Because we weren't
> accounting for callsite arguments in either the hash code or equality
> tests for `MemoryLocOrCall`s, we optimized uses too aggressively in
> some rare cases.
>
> Fix by Daniel Berlin.
>
> Should fix PR36883.
>
> Added:
> llvm/trunk/test/Analysis/MemorySSA/pr36883.ll
> Modified:
> llvm/trunk/lib/Analysis/MemorySSA.cpp
>
> Modified: llvm/trunk/lib/Analysis/MemorySSA.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/MemorySSA.cpp?rev=328748&r1=328747&r2=328748&view=diff
> <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/MemorySSA.cpp?rev=328748&r1=328747&r2=328748&view=diff>
> ==============================================================================
> --- llvm/trunk/lib/Analysis/MemorySSA.cpp (original)
> +++ llvm/trunk/lib/Analysis/MemorySSA.cpp Wed Mar 28 17:54:39 2018
> @@ -153,9 +153,14 @@ public:
> if (IsCall != Other.IsCall)
> return false;
>
> - if (IsCall)
> - return CS.getCalledValue() == Other.CS.getCalledValue();
> - return Loc == Other.Loc;
> + if (!IsCall)
> + return Loc == Other.Loc;
> +
> + if (CS.getCalledValue() != Other.CS.getCalledValue())
> + return false;
> +
> + assert(CS.arg_size() == Other.CS.arg_size());
> + return std::equal(CS.arg_begin(), CS.arg_end(),
> Other.CS.arg_begin());
> }
>
> private:
> @@ -179,12 +184,18 @@ template <> struct DenseMapInfo<MemoryLo
> }
>
> static unsigned getHashValue(const MemoryLocOrCall &MLOC) {
> - if (MLOC.IsCall)
> - return hash_combine(MLOC.IsCall,
> - DenseMapInfo<const Value *>::getHashValue(
> - MLOC.getCS().getCalledValue()));
> - return hash_combine(
> - MLOC.IsCall,
> DenseMapInfo<MemoryLocation>::getHashValue(MLOC.getLoc()));
> + if (!MLOC.IsCall)
> + return hash_combine(
> + MLOC.IsCall,
> + DenseMapInfo<MemoryLocation>::getHashValue(MLOC.getLoc()));
> +
> + hash_code hash =
> + hash_combine(MLOC.IsCall, DenseMapInfo<const Value
> *>::getHashValue(
> +
> MLOC.getCS().getCalledValue()));
> +
> + for (const Value *Arg : MLOC.getCS().args())
> + hash = hash_combine(hash, DenseMapInfo<const Value
> *>::getHashValue(Arg));
> + return hash;
> }
>
> static bool isEqual(const MemoryLocOrCall &LHS, const
> MemoryLocOrCall &RHS) {
>
> Added: llvm/trunk/test/Analysis/MemorySSA/pr36883.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/MemorySSA/pr36883.ll?rev=328748&view=auto
> <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/MemorySSA/pr36883.ll?rev=328748&view=auto>
> ==============================================================================
> --- llvm/trunk/test/Analysis/MemorySSA/pr36883.ll (added)
> +++ llvm/trunk/test/Analysis/MemorySSA/pr36883.ll Wed Mar 28
> 17:54:39 2018
> @@ -0,0 +1,38 @@
> +; RUN: opt -basicaa -memoryssa -analyze < %s 2>&1 -S | FileCheck %s
> +; RUN: opt -aa-pipeline=basic-aa
> -passes='print<memoryssa>,verify<memoryssa>' -S < %s 2>&1 |
> FileCheck %s
> +;
> +; We weren't properly considering the args in callsites in
> equality or hashing.
> +
> +target triple = "armv7-dcg-linux-gnueabi"
> +
> +; CHECK-LABEL: define <8 x i16> @vpx_idct32_32_neon
> +define <8 x i16> @vpx_idct32_32_neon(i8* %p, <8 x i16> %v) {
> +entry:
> +; CHECK: MemoryUse(liveOnEntry)
> + %load1 = call <8 x i16> @llvm.arm.neon.vld1.v8i16.p0i8(i8* %p,
> i32 2) #4 ; load CSE replacement
> +
> +; CHECK: 1 = MemoryDef(liveOnEntry)
> + call void @llvm.arm.neon.vst1.p0i8.v8i16(i8* %p, <8 x i16> %v,
> i32 2) #4 ; clobber
> +
> + %p_next = getelementptr inbounds i8, i8* %p, i32 16
> +; CHECK: MemoryUse(liveOnEntry)
> + %load2 = call <8 x i16> @llvm.arm.neon.vld1.v8i16.p0i8(i8*
> %p_next, i32 2) #4 ; non-aliasing load needed to trigger bug
> +
> +; CHECK: MemoryUse(1)
> + %load3 = call <8 x i16> @llvm.arm.neon.vld1.v8i16.p0i8(i8* %p,
> i32 2) #4 ; load CSE removed
> +
> + %add = add <8 x i16> %load1, %load2
> + %ret = add <8 x i16> %add, %load3
> + ret <8 x i16> %ret
> +}
> +
> +; Function Attrs: argmemonly nounwind readonly
> +declare <8 x i16> @llvm.arm.neon.vld1.v8i16.p0i8(i8*, i32) #2
> +
> +; Function Attrs: argmemonly nounwind
> +declare void @llvm.arm.neon.vst1.p0i8.v8i16(i8*, <8 x i16>, i32) #1
> +
> +attributes #1 = { argmemonly nounwind }
> +attributes #2 = { argmemonly nounwind readonly }
> +attributes #3 = { nounwind readnone }
> +attributes #4 = { nounwind }
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
>
>
--
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180330/81f04591/attachment.html>
More information about the llvm-commits
mailing list