[PATCH] D128159: [DAG] Enable scalable vectors handling in computeKnownBits

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added a comment.

Reading through the comments, has anything changed since my first comment?  The main response seems to be "but hay it works".  I'm sure that's true, but we've had several such moments when adding scalable support and each time we've been told to implement things properly.  Hence the new vector type, element count, type size and instruction cost.  I'm not hearing good reasons why demanded elements is any different.

To call out specific cases I'm worried about:

1. Target's that implement fixed length operations via the scalable types. It seems reasonable to be able to track the real elements within their container.
2. Ensuring operations on `<vscale x 1 x` types do not get misrepresented as being scalar like.

Whilst I'm sure it's possible to show these changes function correctly, it's my belief that such validation is based on either today's limited implementation or plain luck, rather than the design being solid.

Perhaps others have a different view? To be honest this is an area that I've largely ignored so I'll need to dig deeper but I just don't see a safe implementation without either a new type or new set of functions that are designed specifically for the limited use case you want to handle.


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

https://reviews.llvm.org/D128159