[PATCH] D83926: [ValueTracking] Let isGuaranteedNotToBeUndefOrPoison use Operator, look into more ops

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

While you're here, I'd suggest to go through all of `Instructions.def` and add an entry in the switch for all instructions that have a return value, so we at least know what we're missing. This is what I see:

- udiv, sdiv, urem, srem: If `exact` not set, recursive check operands. (Division by zero etc is undefined behavior, not poison.)
- shl, lshr, ashr: Apart from `exact`, these also have the bitwidth case, which could be checked for constant shamt.
- alloca: never undef/poison. (The pointer, not the content...)
- load: We don't now. (Might want `!noundef` metadata in the future, if that's useful)
- atomicrmw, cmpxchg: No idea, no familiarity.
- trunc, sext, zext: Check operands, no special cases.
- fptoui, fptosi: These poison on overflow, can't infer anything useful.
- uitofp, sitofp: These stopped poisoning on overflow at some point, just check operands.
- fpext, fptrunc: Presumably just check operand, but I'm not familiar.
- ptrtoint, inttoptr: Check operand.
- addrspacecast: I would assume this is also just "check operand", but with addrspacecasts you never know :)
- call/invoke: Check noundef attribute on return value.
- select: Check operands and FMF.
- extractelement, insertelement, extractvalue, insertvalue: Check operands.
- shufflevalue: Have to take undef lanes into account.

It might make sense to add some of the straightforward cases (like trunc/sext/zext) to this review and leave the rest as TODOs.



================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:4797
+
+    if (auto *GEPOp = dyn_cast<GEPOperator>(V)) {
+      if (!GEPOp->isInBounds() && llvm::all_of(GEPOp->operands(), OpCheck))
----------------
Move this case into the switch? Similar to how you handle FPMathOperator.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:4810
     case Instruction::FCmp: {
-      auto *FI = dyn_cast<FCmpInst>(I);
-      if (FI->getFastMathFlags().none() &&
-          llvm::all_of(FI->operands(), OpCheck))
+      auto *FPOp = dyn_cast<FPMathOperator>(Opr);
+      if (FPOp->getFastMathFlags().none() &&
----------------
Should be `cast<>` rather than `dyn_cast<>`.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:4818
+      if (isGuaranteedNotToBeUndefOrPoison(Opr->getOperand(0), CtxI, DT,
+                                           Depth + 1))
+        return true;
----------------
Use `OpCheck(Opr->getOperand(0))` here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83926