[PATCH] D145395: [SCEV] Also strengthen flags for post-increment.

Mon Apr 17 02:51:44 PDT 2023

fhahn marked an inline comment as done.
fhahn added inline comments.

================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:5673
   if (auto *AR = dyn_cast<SCEVAddRecExpr>(PHISCEV)) {
-    setNoWrapFlags(const_cast<SCEVAddRecExpr *>(AR),
-                   (SCEV::NoWrapFlags)(AR->getNoWrapFlags() |
-                                       proveNoWrapViaConstantRanges(AR)));
+    Flags = (SCEV::NoWrapFlags)(Flags | proveNoWrapViaConstantRanges(AR));
+    setNoWrapFlags(const_cast<SCEVAddRecExpr *>(AR), Flags);
----------------
nikic wrote:
> It's not really obvious to me why this is correct. Can't the extra addition of the post-inc addrec overflow, even if the pre-inc addrec can't? I believe the current reasoning for setting flags on the post-inc addrec is specifically about nowrap flags present in IR that would result in UB if violated. I don't think this would hold up for flags inferred from the pre-inc addrec range.
It's also not really obvious to me as mentioned here: https://reviews.llvm.org/D144050#inline-1404195

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145395