[PATCH] D90648: [SCEV] Fix nsw flags for GEP expressions

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:3487
+  // non-negative, we can use nuw.
+  SCEV::NoWrapFlags BaseWrap = GEP->isInBounds() && isKnownNonNegative(Offset)
+      ? SCEV::FlagNUW : SCEV::FlagAnyWrap;
----------------
mkazantsev wrote:
> How about case when gep has no inbounds flag, but we can prove that `base+ offset` does not sign-wrap? Likewise, we could prove that `base + offset` does not unsign-wrap even without non-negative fact for offset in some cases. Why don't we use `isKnownPredicate(slt/ult, base, base + offset)` (at least in affine case)?
If we can prove that, the fact wouldn't follow from the presence of `inbounds` in the GEP -- in fact, we could use this method to prove additional nowrap flags on arbitrary adds. As such, I don't think that this code is the right place to perform such a check.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:3488
+  SCEV::NoWrapFlags BaseWrap = GEP->isInBounds() && isKnownNonNegative(Offset)
+      ? SCEV::FlagNUW : SCEV::FlagAnyWrap;
+  return getAddExpr(BaseExpr, Offset, BaseWrap);
----------------
mkazantsev wrote:
> Isn't inbounds gep always `nuw`? Do we really need the non-negative check?
As the offset is interpreted as a signed number, no. For a negative offset, `sub %base, -1 * %offset` would be nuw, but we don't have a way to express that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90648/new/

https://reviews.llvm.org/D90648