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

Juneyoung Lee via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Jul 19 00:04:21 PDT 2020


aqjune marked 4 inline comments as done.
aqjune added inline comments.


================
Comment at: llvm/include/llvm/Analysis/ValueTracking.h:607
+  /// poison from non-undef & non-poison operands.
+  bool canCreateUndef(const Operator *Op, bool ConsiderPoisonAsUndef);
 
----------------
nikic wrote:
> jdoerfert wrote:
> > Drive by: Now that I see this, I would prefer the public API to have two functions. I'm fine if others disagree but for the users it seems more natural to have `canCreateUndef` and `canCreateUndefOrPoison`, (and maybe `canCreatePoison`).
> Same here. I would prefer there to only be canCreatePoison and canCreatePoisonOrUndef. Just canCreateUndef by itself would be a footgun, because poison relaxes to undef. I don't see an immediate use for a "can create undef excluding undef that was originally poison" query.
Leaving canCreateUndefOrPoison and canCreatePoison only, since it turns out that canCreateUndef could not be used to resolve the instsimplify problem at D83360


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84007





More information about the llvm-commits mailing list