[PATCH] D30446: [IndVars] Do not branch on poison

Sanjoy Das via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 15 12:56:50 PDT 2017


sanjoy added a subscriber: atrick.
sanjoy added a comment.

In https://reviews.llvm.org/D30446#702025, @efriedma wrote:

> I guess first off, I'd like to take a step back and ask: do we need to do this transform at all?  We're using some very complicated logic to conditionally rewrite an induction variable in terms of another induction variable... and then LSR will rewrite all the induction variables from scratch.  This also seems difficult to test effectively: if we incorrectly rewrite an induction variable to something which could be poison, it'll almost always work, except in obscure cases.


I see this transform as a "canonicalization" type transform (supposed to make later loop opts easier), but I did not add it.  Maybe @atrick can shed some light here on the initial motivation?  I'm most certainly in favor of deleting code where possible.

Having said that, I don't think this is the only place in LLVM where the "it'll almost always work, except in obscure cases" adage applies, so that alone can't be the reason for excising this.


https://reviews.llvm.org/D30446





More information about the llvm-commits mailing list