[PATCH] D25542: GVN-hoist: avoid calling MemoryLocation::get() on a call (PR30499)

Daniel Berlin via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 12 20:41:00 PDT 2016


Yup.
Thanks!


On Wed, Oct 12, 2016 at 8:30 PM, Sebastian Pop <sebpop at gmail.com> wrote:

> On Wed, Oct 12, 2016 at 10:26 PM, Daniel Berlin <dberlin at dberlin.org>
> wrote:
> > You can get rid of UseInst too, MemoryLocOrCall will take a MemoryUse
> > directly :)
>
> Yes if I add the "const MU" to the class, like so:
>
> diff --git a/lib/Transforms/Utils/MemorySSA.cpp
> b/lib/Transforms/Utils/MemorySSA.cpp
> index 03faffd..b02a3f5 100644
> --- a/lib/Transforms/Utils/MemorySSA.cpp
> +++ b/lib/Transforms/Utils/MemorySSA.cpp
> @@ -105,6 +105,8 @@ public:
>    MemoryLocOrCall() : IsCall(false) {}
>    MemoryLocOrCall(MemoryUseOrDef *MUD)
>        : MemoryLocOrCall(MUD->getMemoryInst()) {}
> +  MemoryLocOrCall(const MemoryUseOrDef *MUD)
> +      : MemoryLocOrCall(MUD->getMemoryInst()) {}
>
>    MemoryLocOrCall(Instruction *Inst) {
>      if (ImmutableCallSite(Inst)) {
> @@ -269,8 +271,7 @@ static bool instructionClobbersQuery(MemoryDef
> *MD, const MemoryUseOrDef *MU,
>  // Return true when MD may alias MU, return false otherwise.
>  bool defClobbersUseOrDef(MemoryDef *MD, const MemoryUseOrDef *MU,
>                           AliasAnalysis &AA) {
> -  return instructionClobbersQuery(MD, MU, MemoryLocOrCall(MU->
> getMemoryInst()),
> -                                  AA);
> +  return instructionClobbersQuery(MD, MU, MemoryLocOrCall(MU), AA);
>  }
>  }
>
>
>
> >
> >
> > On Wed, Oct 12, 2016 at 8:20 PM, Sebastian Pop <sebpop at gmail.com> wrote:
> >>
> >> On Wed, Oct 12, 2016 at 10:05 PM, Daniel Berlin <dberlin at dberlin.org>
> >> wrote:
> >> > There is actually a much simpler fix you sholdl use.
> >> >
> >> > return instructionClobbersQuery(MD, MU MemoryLocOrCall(MU), AA);
> >> >
> >> > You need to move that version of instructionClobbersQuery up, but it
> was
> >> > built to always do the right thing.
> >> >
> >> > Also note it will properly handle a case you are not: Fences do not
> have
> >> > memorylocations, and calling get on them will also crash.
> >> >
> >>
> >> Right. I will commit the attached patch after testing.
> >>
> >> Thanks,
> >> Sebastian
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161012/4d1dd6dc/attachment.html>


More information about the llvm-commits mailing list