[PATCH] D77272: Clean up usages of asserting vector getters in Type

Christopher Tetreault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 10 09:19:55 PDT 2020


ctetreau marked an inline comment as done.
ctetreau added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/BasicTTIImpl.h:85
   unsigned getBroadcastShuffleOverhead(Type *Ty) {
-    assert(Ty->isVectorTy() && "Can only shuffle vectors");
+    auto *VTy = dyn_cast<VectorType>(Ty);
+    assert(VTy && "Can only shuffle vectors");
----------------
sdesmalen wrote:
> Can we just use `cast<>` and get rid of the separate assert below? We'd lose the message, but I'm not sure how useful that is, given that the context makes it quite obvious.
I spoke to this a bit in D77259.

Realistically, for this example, in debug you'll get a slightly more readable assert message. In release, you'll get a null dereference in getVectorInstrCost vs (I assume) a template error in the cast. From a debugging perspective, the second situation is almost certainly preferable, but just doing the cast will be better for performance. I'll fix it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77272





More information about the llvm-commits mailing list