[PATCH] D95286: [LSR] Drop potentially invalid nowrap flags when switching to post-inc IV (PR46943)

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp:1447
+    if (isa<OverflowingBinaryOperator>(Result)) {
+      if (auto *I = dyn_cast<Instruction>(Result)) {
+        if (!S->hasNoUnsignedWrap())
----------------
nikic wrote:
> fhahn wrote:
> > nikic wrote:
> > > reames wrote:
> > > > if (auto *op = dyn_cast<OBO>...)
> > > > 
> > > > e.g. you don't need the instruction, you can use the accessors on OBO.  
> > > OBO can generally also be a constant expression, which is why it's not allowed to set nowrap flags directly on it. We need to go through Instruction to modify the flags.
> > But can `Result` be a constant expression here? `PN` must be an AddRec for L, then the incoming value from the loop should be non-constant, otherwise it wouldn't be an AddRec?
> Yes, it can't be a constant expression here, but it can be in general, thus the OBO API does not support setting nowrap flags. (This is why I was originally using BinaryOperator here.)
> 
> Or do you mean that this code should be using `cast<>` rather than `dyn_cast<>` for Instruction? I can change that.
> Or do you mean that this code should be using cast<> rather than dyn_cast<> for Instruction? I can change that.

Yep, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95286