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

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

> 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.

To be explicit here: The issue is not whether the addition happens before or after the comparison, in fact you can freely interchange these instructions, as they have no sequencing constraint. I expect that if you swap x.3 and x.4 in your test case, then your fix will no longer work, even though dropping nuw is equally necessary in that case. The problematic part is the new user of the postinc IV.

I would expect the patch for this to look closer to something like this: https://gist.github.com/nikic/7e1301a81542d95953a44243c8ee5b5e That is, drop the poison flags when SCEV expansion reuses a postinc value in some form, on the assumption that the new user might have UB-on-poison semantics.

The programUndefinedIfPoison() check there is supposed to avoid dropping poison flags if there is an existing UB-on-poison user, in particular if the loop is already in postinc form. Unfortunately this doesn't actually work right now, because getGuaranteedNonPoisonOps() doesn't take into account branch instructions :/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91562