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

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen accepted this revision.
sdesmalen added a comment.
This revision is now accepted and ready to land.

LGTM!



================
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");
----------------
ctetreau wrote:
> 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.
Thanks! In this case the only difference for a release build would be that `VTy` would be `nullptr` when using `dyn_cast`, and would be `Ty` when using `cast`. I don't think the use of `cast` gives a template error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77272