[PATCH] D134648: [LangRef] Update text for vscale to be more flexible but maintain original intent.

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D134648#3817518 <https://reviews.llvm.org/D134648#3817518>, @aemerson wrote:

> In D134648#3815329 <https://reviews.llvm.org/D134648#3815329>, @nikic wrote:
>
>> I don't think we can allow this as long as vscale constant expressions are supported (these are currently represented via gep of scalable type). I am rather strongly opposed to having constants that are not in fact constant, so this change should only happen after a migration towards `llvm.vscale` intrinsics.
>
> I haven't been following the scalable types story in a lot of detail, but doesn't disallowing GEP of scalable types imply that pointer calculations need to be done via decomposed integer arithmetic? Does this break for non-integral pointer types?

Arbitrary arithmetic on pointers is represented via `getelementptr i8, ptr %p, i64 %offset`, it's not necessary (and highly undesirable) to drop down to ptrtoint + add + inttoptr.

For vscale in particular, it's not necessary to go that far:

  %gep = getelementptr <vscale x N x %T>, ptr %p, i64 %index
  ; Is equivalent to
  %vscale = call i64 @llvm.vscale.i64()
  %scaled.index = mul i64 %index, %vscale
  %gep = getelementptr <N x %T>, ptr %p, i64 %scaled.index

Though for the purposes of this change, I'm mainly concerned about the constant expression case, which would no longer be well-defined if vscale is non-constant. I //think// constant scalable GEPs are mostly used to represent vscale itself (using something like `ptrtoint (ptr getelementptr (<vscale x 1 x i8>, ptr null, i64 1) to i64)`).

Removing scalable GEPs entirely would also be beneficial, because our current support for scalable GEPs tends to be of the "just don't crash" kind and adding proper support for them is associated with a lot of additional complexity. On the other hand, a `llvm.vscale`-based representation "just works", e.g. BasicAA would be able to reason about it without further changes. But that's kind of orthogonal to this particular change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134648