[PATCH] D38415: [SimplifyIndVar] Replace IVUsers with loop invariant if possible
Hongbin Zheng via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 2 15:17:04 PDT 2017
etherzhhb added inline comments.
================
Comment at: lib/Transforms/Utils/SimplifyIndVar.cpp:552
- if (!C)
+ if (!SE->isLoopInvariant(S, L))
return false;
----------------
efriedma wrote:
> etherzhhb wrote:
> > efriedma wrote:
> > > etherzhhb wrote:
> > > > We may only want to do the replace if S is invariant of the IV's Loop, which may be different from "L" which is the innermost loop that contains the IV user
> > > Yes; otherwise, the transform isn't doing anything useful.
> > >
> > > We might need to call isHighCostExpansion here, so we don't generate something ridiculous?
> > Yes, this make sense.
> >
> > I did some experiments and we may want to actually write:
> >
> > ```
> > if (!SE->isLoopInvariant(S, IV's Loop) || !SE->isLoopInvariant(S, IV User's Loop))
> > ```
> >
> > If IV's Loop do not contains IV User's Loop
> Given a function in LCSSA form, I don't think this relevant? But feel free to prove me wrong. :)
Not sure if we assume LCSSA (or SCEV could see through it), I hit some crash when I just test 'if (!SE->isLoopInvariant(S, IV's Loop))' and it turned out that IV's Loop do not contain IV's user. I will find those cases
https://reviews.llvm.org/D38415
More information about the llvm-commits
mailing list