[PATCH] D119529: [BasicTTI] Set scalarization cost of getCommonMaskedMemoryOpCost to Invalid.

Sander de Smalen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 15 03:05:50 PST 2022


sdesmalen added a comment.

In D119529#3321740 <https://reviews.llvm.org/D119529#3321740>, @Jim wrote:

> In D119529#3313680 <https://reviews.llvm.org/D119529#3313680>, @sdesmalen wrote:
>
>> Can you add a test for this change?
>
> The crash only triggered target is given (specify -mtriple=riscv64) and BasicTTIImplBase::getCommonMaskedMemoryOpCost would be called.
> If no target given, NoTTIImpl is used as TTI, and TargetTransformInfoImpl.h::getMemoryOpCost simply returns cost 1 without any crash.
>
> I don't have plan to implement hook getMaskedMemoryOpCost for RISCV. 
> Do you have any suggest for this kind situation?

This function assumes fixed-width vectors and cannot be used for scalable vectors. I'm not sure what returning 'Invalid' here would really fix, other than the compiler not crashing for a use-case that should not have occurred in the first place, because an overloaded cost function should have been implemented. Whether the compiler falls into the assert from `cast<FixedVectorType>`, or whether it returns an Invalid cost, in either case you'll need to implement a memory-op-cost function for your target. And because you'll need to implement a cost-function anyway, returning Invalid doesn't really make a difference, because then this code will never be hit. Otherwise, you would have been able to write a test-case for it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119529



More information about the llvm-commits mailing list