[PATCH] D96079: Move implementation of isAssumeLikeIntrinsic into IntrinsicInst

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith added a comment.

Not sure I'm the best person to review this, but I have a couple of drive-by questions.



================
Comment at: llvm/include/llvm/IR/IntrinsicInst.h:104
+    }
+    return false;
+  }
----------------
Usually we'd have a `default: break;` or `default: return false;` if the switch isn't totally covered... is there a reason you removed it when moving the code?


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:526
-      default: break;
-      // FIXME: This list is repeated from NoTTI::getIntrinsicCost.
-      case Intrinsic::assume:
----------------
Should this FIXME be kept?


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

https://reviews.llvm.org/D96079