On Feb 7, 2014, at 11:29 AM, Chris Lattner <clattner at apple.com> wrote:> > On Feb 1, 2014, at 4:33 AM, Chandler Carruth <chandlerc at gmail.com> wrote: > >> So, there are two primary ideas behind SSA form management in the loop optimizers of LLVM: >> >> - Require LCSSA form input, leverage its (very powerful) guarantees to simplify maintaining SSA form, and also maintain LCSSA form. >> >> - Don't bother with LCSSA form input, assume the worst, and use powerful incremental SSA formation utilities built on SSAUpdater to form SSA on demand when needed. >> >> (Note, there are plenty of places where SSAUpdater makes sense, so this isn't really about doing away with it at all.) > > It’s worth noting that LCSSA predates SSAUpdater. If I went back in time and knew what I knew now, I wouldn’t have gone with LCSSA. > > My gripes are three fold: 1) SSAUpdater can handle anything that LCSSA simplifies, 2) that LCSSA is annoying to keep up to date, 3) LCSSA burns compile time optimistically rewriting loop values, which are then later collapsed away even if nothing cares about those values. > > My personal preference would be to get rid of LCSSA completely, but I don’t know how to stage that.Until recently I felt exactly the same way. I didn’t want LCSSA just as another mechanism for updating SSA. I’m warming up to having it run during the early loop passes if it - significantly simplifies the LICM logic - greatly speeds up SSAUpdater within loops (maybe little net compile-time increase on average?) - compartmentalizes loop transforms and SSA update so we can debug loop opts one loop at a time I still don’t particularly like that we force all LLVM clients to perform LCSSA when all they end up doing is rotating and simplifying loops (no LICM/unroll). So it is a tradeoff. -Andy
On Feb 7, 2014, at 12:05 PM, Andrew Trick <atrick at apple.com> wrote:>>> (Note, there are plenty of places where SSAUpdater makes sense, so this isn't really about doing away with it at all.) >> >> It’s worth noting that LCSSA predates SSAUpdater. If I went back in time and knew what I knew now, I wouldn’t have gone with LCSSA. >> >> My gripes are three fold: 1) SSAUpdater can handle anything that LCSSA simplifies, 2) that LCSSA is annoying to keep up to date, 3) LCSSA burns compile time optimistically rewriting loop values, which are then later collapsed away even if nothing cares about those values. >> >> My personal preference would be to get rid of LCSSA completely, but I don’t know how to stage that. > > Until recently I felt exactly the same way. I didn’t want LCSSA just as another mechanism for updating SSA. > > I’m warming up to having it run during the early loop passes if it > - significantly simplifies the LICM logic > - greatly speeds up SSAUpdater within loops (maybe little net compile-time increase on average?) > - compartmentalizes loop transforms and SSA update so we can debug loop opts one loop at a time > > I still don’t particularly like that we force all LLVM clients to perform LCSSA when all they end up doing is rotating and simplifying loops (no LICM/unroll). So it is a tradeoff.I don't have a strong opinion here, just throwing out some thoughts. You've been working on the loop passes much more recently, so if you think it is worth holding on to (or worth using just for the early passes?) then go for it. -Chris
On Sat, Feb 8, 2014 at 2:36 PM, Chris Lattner <clattner at apple.com> wrote:> On Feb 7, 2014, at 12:05 PM, Andrew Trick <atrick at apple.com> wrote: > >>> (Note, there are plenty of places where SSAUpdater makes sense, so > this isn't really about doing away with it at all.) > >> > >> It’s worth noting that LCSSA predates SSAUpdater. If I went back in > time and knew what I knew now, I wouldn’t have gone with LCSSA. > >> > >> My gripes are three fold: 1) SSAUpdater can handle anything that LCSSA > simplifies, 2) that LCSSA is annoying to keep up to date, 3) LCSSA burns > compile time optimistically rewriting loop values, which are then later > collapsed away even if nothing cares about those values. > >> > >> My personal preference would be to get rid of LCSSA completely, but I > don’t know how to stage that. > > > > Until recently I felt exactly the same way. I didn’t want LCSSA just as > another mechanism for updating SSA. > > > > I’m warming up to having it run during the early loop passes if it > > - significantly simplifies the LICM logic >I've committed this in r201148 in order to fix several PRs. We can of course back it out and go down a different path if that's the end decision, but it seems better to not have asserts in the interim. =] You can see the simplification to 'sink' that falls out of this though. This is, IMO, not a-typical.> > - greatly speeds up SSAUpdater within loops (maybe little net > compile-time increase on average?) > > - compartmentalizes loop transforms and SSA update so we can debug loop > opts one loop at a time > > > > I still don’t particularly like that we force all LLVM clients to > perform LCSSA when all they end up doing is rotating and simplifying loops > (no LICM/unroll). So it is a tradeoff. >Again, LoopSimplify does not require LCSSA today. Nothing to do there. If we folded rotate into simplify (which we should probably do) then we would be done.> >I don't have a strong opinion here, just throwing out some thoughts.> You've been working on the loop passes much more recently, so if you think > it is worth holding on to (or worth using just for the early passes?) then > go for it. >It's worth noting that getting rid of LCSSA would be a non-trivial amount of work and would have to be done somewhat carefully to preserve invariants of other passes in the LPM. Not saying we can't do it, but there is a reason to keep piling onto LCSSA until there is a clear decision to rip it out, and only then to rip all of it out at once. Anyways, I'm still somewhat preferring to keep it in place for the simplifications. The cost appears to be quite small now that we don't invalidate all AA. ;] -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140211/e55f8d48/attachment.html>