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

Sander de Smalen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 2 09:30:13 PST 2020


sdesmalen added a comment.

In D75297#1896735 <https://reviews.llvm.org/D75297#1896735>, @ctetreau wrote:

> how pervasive is this issue? I worry that adding this stopgap today will allow a bunch of dangerous code to get committed, and will make future efforts to correct the issue more difficult.


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.

> Can the places where this currently blows up just call getKnownMinSize()?

I'd rather not change all places to use `getKnownMinSize()` as this suggests the code is successfully ported.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75297





More information about the llvm-commits mailing list