[PATCH] D84007: [ValueTracking] Change canCreatePoison to take Operator, add canCreateUndef

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:4783
+
+  return false;
 }
----------------
nikic wrote:
> jdoerfert wrote:
> > nikic wrote:
> > > jdoerfert wrote:
> > > > This seems wrong. A load of an uninitialized location should produce `undef` for example, shouldn't it? I fear there are way more things that can produce `undef`. The default should be true here.
> > > Returning true here would make this function always return true :) If we drop the "undef only" mode, then returning false should be fine, as this function only needs to deal with cases where undef is possible but poison is not.
> > What about my load example? poison is not possible but undef is, returning false still seems wrong, or maybe I'm just confused..
> Loads can also return poison, and canCreatePoison() will return true for them. (Having load/store implicitly filter poison would prevent mem2reg/SROA.)
Loads *can* return poison. Some are known *not to* but still *can* return `undef`. Are we sure this implicit link between the two procedures is wise? I mean, even if the situation I describe doesn't happen now, how do you prevent someone from making `canCreatePoison` smarter and introducing a bug here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84007