[PATCH] D82892: Added comparision for all types in haveSameSpecialState() of Instruction.cpp

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/IR/Instruction.cpp:475
+    else
+      return cast<ReturnInst>(I2)->getReturnValue() == nullptr;
+  }
----------------
I don't think this belongs here, as it is comparison operands. That's handled separately, e.g. in isSameOperationAs.


================
Comment at: llvm/lib/IR/Instruction.cpp:478
+  if (const BranchInst *BI = dyn_cast<BranchInst>(I1))
+    return BI->isConditional() == cast<BranchInst>(I2)->isConditional();
+  if (const SwitchInst *SI = dyn_cast<SwitchInst>(I1))
----------------
This can be an assertion, because operand count equality implies it.


================
Comment at: llvm/lib/IR/Instruction.cpp:481
+    return SI->getCondition()->getType() ==
+           cast<SwitchInst>(I2)->getCondition()->getType();
+  if (const IndirectBrInst *IBI = dyn_cast<IndirectBrInst>(I1))
----------------
Similar to the return case, I don't think this belongs, as it's checking operands.


================
Comment at: llvm/lib/IR/Instruction.cpp:486
+    return RI->getValue()->getType() ==
+           cast<ResumeInst>(I2)->getValue()->getType();
+  if (const CatchSwitchInst *CSI = dyn_cast<CatchSwitchInst>(I1))
----------------
Same here.


================
Comment at: llvm/lib/IR/Instruction.cpp:502
+  if (const GetElementPtrInst *GEP1 = dyn_cast<GetElementPtrInst>(I1))
+    return GEP1->isInBounds() == cast<GetElementPtrInst>(I2)->isInBounds();
+  if (const CastInst *CI = dyn_cast<CastInst>(I1))
----------------
inbounds is subclass optional data, it should not be compared here. (It is legal to drop inbounds.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82892