[PATCH] D76010: [ValueTracking] Let isGuaranteedNotToBeUndefOrPoison look into more constants/instructions

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In D76010#1938205 <https://reviews.llvm.org/D76010#1938205>, @jdoerfert wrote:

> @spatel does this look better?


Yes - I added a few more inline nits. I don't have a good understanding of the casting part of the patch, but other than that LGTM.



================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:4639
 
-  if (auto II = dyn_cast<ICmpInst>(V)) {
-    if (llvm::all_of(II->operands(), [](const Value *V) {
-          return isGuaranteedNotToBeUndefOrPoison(V);
-        }))
+  if (auto GEPI = dyn_cast<GetElementPtrInst>(V))
+    if (!GEPI->isInBounds() && llvm::all_of(GEPI->operands(), OpCheck))
----------------
nit: "auto *"


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:4643
+
+  if (auto FI = dyn_cast<FCmpInst>(V))
+    if (FI->getFastMathFlags().none() && llvm::all_of(FI->operands(), OpCheck))
----------------
nit: "auto *"


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:4647
 
   if (auto I = dyn_cast<Instruction>(V)) {
+    if ((isa<BitCastInst>(I) || isa<PHINode>(V) || isa<ICmpInst>(V)) &&
----------------
nit: "auto *"


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:4648
   if (auto I = dyn_cast<Instruction>(V)) {
+    if ((isa<BitCastInst>(I) || isa<PHINode>(V) || isa<ICmpInst>(V)) &&
+        llvm::all_of(I->operands(), OpCheck))
----------------
Use "I" in all of these clauses for consistency.
If the intent is to grow this list, might want to use a "switch(I->getOpcode())" already so later patches are minimal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76010