[PATCH] IndVarSimplify: Don't let LFTR compare against a poison value
atrick at apple.com
Wed Sep 3 16:33:32 PDT 2014
I'll say LGTM because fixing a bug is an improvement. This is unfortunate though because LFTR does not care about overflow and we don't want to pessimize code just because SCEV was able to infer NSW. The problem is that we can't express that NSW applies to some uses an not others.
The fix is also not totally robust because SCEV could, in theory, drop NSW flags even though they exist in IR. The induction variable was not necessarily produced by SCEVExpander. I think making it totally robust would involve getting a new expression for CmpIndVar by applying non-NSW AddExpr's (similar to genLoopLimit). That is of course much more complexity, hence risk, so maybe best as a FIXME.
Out of curiosity, can you explain how we came to generate bad code for this case (so I don't have to debug the test case)? Was there an optimization after LFTR that assumed NSW at the compare? Do we end up widening the loop test and failing to exit?
FWIW: The check against FlagAnyWrap was a little confusing to me, as opposed to simply:
if (maskFlags(Expr->getNoWrapFlags(), SCEV::NSW | SCEV::NUW))
More information about the llvm-commits