[PATCH] D75297: [TypeSize] Allow returning scalable size in implicit conversion to uint64_t

Mon Mar 2 12:29:25 PST 2020

ctetreau added a comment.

In D75297#1901266 <https://reviews.llvm.org/D75297#1901266>, @sdesmalen wrote:

> We should probably move to a different approach soon (as we discussed in our last sync-up call), but until then I think there is little down-side to removing this assert. Removing the assert would allow us to use the SVE ACLE intrinsics which don't rely on common optimisations.
>  Until we get auto-vec support there are currently no other uses of scalable types. Moving to auto-vec will require a systematic approach to upgrading the code-base to distinguish scalable types, so we may have to fix up a couple more cases for new code that's added until we do so, but even with the assert nothing would break unless someone adds new code and/or tests that uses scalable types.

I have concerns about introducing technical debt that we plan to pay off "soon". Life happens and things get deemed "good enough". If this patch is necessary for work to proceed on ACLE, perhaps those working on it should locally cherry pick this patch and do their work?

Currently, this implicit conversion is an alias for getFixedSize(). Lots of code is written using this assumption, and changing the meaning of the conversion is only going to add bugs. In my opinion, the conversion should be removed; if this had been done previously then we wouldn't be in this position today. Failing that, we should fix the breakages, not hide them.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75297