[PATCH] D91562: [LSR] Drop poison flags for post-inc IVs (PR46943)

[Felix S Klock II via Phabricator via llvm-commits]

pnkfelix added a comment.

In D91562#2400598 <https://reviews.llvm.org/D91562#2400598>, @nikic wrote:

> The approach here doesn't look right to me. Poison flags have no relation to the position where a certain instruction is executed. What's relevant is where the instruction is used.
>
> This transform adds a new poison-is-UB user to the post-inc IV. Whereever that code is, should be the place that drops poison flags. The code is pretty convoluted though, so I didn't immediately spot where that happens.

The main instance I know of of a new poison-is-UB user is the `TermBr` itself that will immediately succeed `Cond` after the transformation. Based on what the transformation is doing, I guess *all* users of the `Cond` are potentially poison-is-UB users.

That is, for the specific bug that I'm recreating: (1.) the addition now happens before the Cond instead of after (and still overflows; it always overflowed), (2.) the poison value flows into the `Cond`, yielding poison, and (3.) the poison from the `Cond` flows into the `TermBr`, yielding UB.

I considered starting from the `TermBr`, and tracing backwards to find every operation that flows into its computation. But I was concerned that would be overly conservative and potentially introduce regressions. I considered the change I posted to be the simplest change I could make that would still fix the bug.

I suppose I could try to trace the value-flow backwards from the `TermBr`, but only remove the nowrap flags from the instructions that are *dominated* by the `Cond`... would that be preferable to you, @nikic ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91562