[PATCH] D111643: [ValueTracking] Let propgatesPoison consider single poison operand.

Philip Reames via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 12 09:03:08 PDT 2021


reames added a comment.

This looks pretty reasonable to me overall.  A couple high level suggestions:

- I second the suggestion of taking the new operand as a Use.
- Framing wise, I suggest making it clear that we're restricting the set of operands known to be poison.  And that the nullptr value has no restriction.  I found your comment and patch framing difficult to decipher.  I had to work back to what you meant from the code.
- We could generalize this slightly by taking a arrayref with a subset of the operands of the instruction which can be assumed poison.  Why?  Because we can use that combined with non-poison proven operand information in inscombine to drop freezes.  As an example, freeze(udiv non-poison, X) can drop the freeze as it can only produce poison in the immediate UB case.  I'm not asking you to implement this BTW, just to make the interface obvious that it could be.  (Hm, all of the examples I can think of for this involve instructions with two operands, so maybe we don't need this.  Left for your consideration.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111643



More information about the llvm-commits mailing list