[PATCH] D35317: [EarlyCSE] Handle calls with no MemorySSA info.

[Daniel Berlin via llvm-commits]

Hey Vedant,
You are right that this is a reasonable alternative.
It would save some hash lookups, but not sure it's worth it one way or the
other.

I'll approve the original patch and leave it up to Geoff to decide

On Thu, Jul 13, 2017 at 6:59 PM, Vedant Kumar <vsk at apple.com> wrote:

> I'm a newbie to this part of llvm, but it seems like isSameMemGeneration
> isn't expected to be passed instructions which don't have corresponding
> memory accesses. One alternative approach is to check that the parsed
> mem-inst for the candidate instruction is valid, before querying
> isSameMemGeneration. That could save some time.
>
> E.g:
>
>
>
>
> best,
> vedant
>
>
> > On Jul 12, 2017, at 11:45 AM, Geoff Berry via Phabricator via
> llvm-commits <llvm-commits at lists.llvm.org> wrote:
> >
> > gberry created this revision.
> > Herald added subscribers: Prazek, mcrosier.
> >
> > When checking for memory dependencies between calls using MemorySSA,
> > handle cases where the calls have no MemoryAccess associated with them
> > because the AA analysis being used has determined that the call does not
> > read/write memory.
> >
> > Fixes PR33756
> >
> >
> > https://reviews.llvm.org/D35317
> >
> > Files:
> >  lib/Transforms/Scalar/EarlyCSE.cpp
> >  test/Transforms/EarlyCSE/globalsaa-memoryssa.ll
> >
> >
> > Index: test/Transforms/EarlyCSE/globalsaa-memoryssa.ll
> > ===================================================================
> > --- /dev/null
> > +++ test/Transforms/EarlyCSE/globalsaa-memoryssa.ll
> > @@ -0,0 +1,23 @@
> > +; RUN: opt < %s -S -globals-aa -early-cse-memssa | FileCheck %s
> > +
> > +define i16 @f1() readonly {
> > +  ret i16 0
> > +}
> > +
> > +declare void @f2()
> > +
> > +; Check that EarlyCSE correctly handles function calls that don't have
> > +; a MemoryAccess.  In this case the calls to @f1 have no
> > +; MemoryAccesses since globals-aa determines that @f1 doesn't
> > +; read/write memory at all.
> > +
> > +; CHECK-LABEL: @f3(
> > +; CHECK-NEXT: %call1 = call i16 @f1()
> > +; CHECK-NEXT: call void @f2()
> > +; CHECK-NEXT  ret void
> > +define void @f3() {
> > +  %call1 = call i16 @f1()
> > +  call void @f2()
> > +  %call2 = call i16 @f1()
> > +  ret void
> > +}
> > Index: lib/Transforms/Scalar/EarlyCSE.cpp
> > ===================================================================
> > --- lib/Transforms/Scalar/EarlyCSE.cpp
> > +++ lib/Transforms/Scalar/EarlyCSE.cpp
> > @@ -562,13 +562,20 @@
> >   if (!MSSA)
> >     return false;
> >
> > +  // If MemorySSA has determined that one of EarlierInst or LaterInst
> does not
> > +  // read/write memory, then we can safely return true here.
> > +  MemoryAccess *EarlierMA = MSSA->getMemoryAccess(EarlierInst);
> > +  MemoryAccess *LaterMA = MSSA->getMemoryAccess(LaterInst);
> > +  if (!EarlierMA || !LaterMA)
> > +    return true;
> > +
> >   // Since we know LaterDef dominates LaterInst and EarlierInst dominates
> >   // LaterInst, if LaterDef dominates EarlierInst then it can't occur
> between
> >   // EarlierInst and LaterInst and neither can any other write that
> potentially
> >   // clobbers LaterInst.
> >   MemoryAccess *LaterDef =
> >       MSSA->getWalker()->getClobberingMemoryAccess(LaterInst);
> > -  return MSSA->dominates(LaterDef, MSSA->getMemoryAccess(EarlierInst));
> > +  return MSSA->dominates(LaterDef, EarlierMA);
> > }
> >
> > bool EarlyCSE::processNode(DomTreeNode *Node) {
> >
> >
> > <D35317.106266.patch>_______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170714/325f051a/attachment.html>