[PATCH] D37460: [GVN] Prevent LoadPRE from hoisting across instructions that don't pass control flow to successors
Philip Reames via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 3 09:57:23 PDT 2017
reames added inline comments.
================
Comment at: lib/Transforms/Scalar/GVN.cpp:2189
+ // Make sure that there are no instructions with implicit control flow that
+ // could prevent us from reaching our instruction.
----------------
mkazantsev wrote:
> mkazantsev wrote:
> > reames wrote:
> > > This is overly conservative for any instruction which can not ever fault. For instance, PRE a load over a throwing call is perfectly sound. Note that preserving the nsw/nuw flags is questionable in the same case, but that's a hard discussion. :)
> > >
> > > I'd prefer to see the scalarPRE part separate into it's own patch. I know I'm saying the exact opposite of Eli here, but I think this needs more discussion and trying to combine them will delay the loadPRE patch unnecessarily.
> > I disagree that PRE over a throwing call is invalid. Imagine the following situation:
> >
> > int arr[LEN];
> >
> > void foo(index) {
> > ...
> > call rangeCheck(index, LEN);
> > %x = load %arr, %index
> > print(%x);
> > }
> >
> > void rangeCheck(int index, int len) {
> > if (index < 0 || index >= len)
> > throw exception;
> > }
> >
> > We cannot PRE across the throwing call here.
> >> I disagree that prohibiting PRE over a throwing call is invalid
>
> :)
Sorry, I should have said "PRE of a *known safe to speculate* load over a throwing call is perfectly sound". My sloppy wording here was really confusing, sorry!
https://reviews.llvm.org/D37460
More information about the llvm-commits
mailing list