[PATCH] SCEV incorrectly marks certain expressions as nsw

Sanjoy Das sanjoy at playingwithpointers.com
Mon Feb 16 22:49:47 PST 2015


> The condition you're checking is a stated pre-condition of the function. Is this the only dependence on that pre-condition?


Yes, as far as I can tell.  The three checks explicitly test that `PreStart + Step` does not overflow without depending on the no-wrap-ness of `AR`.  The other SCEV functions this calls out to may potentially do a better job if `AR` is in fact `nsw` but that is not required for correctness except in the case this patch fixes.  This is why I think the right fix is to simply relax the precondition (I've changed the comment to reflect that).

Clang bootstrap (Release+Asserts) fails to find a failure.  I don't find that surprising -- we'd essentially have to have a non-nsw `AR` such that `sext(AR->getStart()) + BECount * zext(AR->getStep())` does not overflow, while `sext(AR->getStart()) + BECount * sext(AR->getStep())` does (this part is easy) and `PreStart` is an scev add expression that does not overflow (this part is hard if we want the first property).  The only obvious way to get the second property is to make `PreAR` of the form `C1 + C2 * Var` (with some constraints on `C1` and `C2`), but having `PreStart` in that form makes it difficult for scev to prove that `sext(AR->getStart()) + BECount * zext(AR->getStep())` does not overflow.  Frankly, it is possible that the bug this patch fixes is completely dormant at this time in SCEV.


http://reviews.llvm.org/D7640

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list