[llvm] r328748 - [MemorySSA] Consider callsite args for hashing and equality.
Tom Stellard via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 9 21:38:31 PDT 2018
On 04/09/2018 08:16 PM, George Burgess IV wrote:
> Thank you very much!
>
> Can you please pick r328755, as well? It's a tiny but necessary bugfix for this. :)
>
r329670.
-Tom
> Thanks again,
> George
>
> On Mon, Apr 9, 2018 at 7:42 PM, Tom Stellard <tstellar at redhat.com <mailto:tstellar at redhat.com>> wrote:
>
> On 04/09/2018 07:13 PM, George Burgess IV wrote:
> > Thanks, Hal!
> >
> > Is this OK with you, too, Tom?
> >
>
> Yes, I've merged this: r329663.
>
> -Tom
>
> > George
> >
> > On Fri, Mar 30, 2018 at 1:33 PM, Hal Finkel <hfinkel at anl.gov <mailto:hfinkel at anl.gov> <mailto:hfinkel at anl.gov <mailto:hfinkel at anl.gov>>> wrote:
> >
> >
> > 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> <mailto: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> <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> <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> <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> <mailto: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> <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
> >
> >
>
>
More information about the llvm-commits
mailing list