Page MenuHomePhabricator

[Sema][SVE] Reject arithmetic on pointers to sizeless types
ClosedPublic

Authored by rsandifo-arm on Mar 12 2020, 11:44 AM.

Details

Summary

This patch completes a trio of changes related to arrays of
sizeless types. It rejects various forms of arithmetic on
pointers to sizeless types, in the same way as for other
incomplete types.

Diff Detail

Event Timeline

rsandifo-arm created this revision.Mar 12 2020, 11:44 AM

Do we really want to forbid this? The expected semantics are clear, and the obvious lowering to getelementptr just works.

Do we really want to forbid this? The expected semantics are clear, and the obvious lowering to getelementptr just works.

The problem is that pointer arithmetic is really a form of sizeof() operation. I.e. it only makes sense to move pointers past N objects if you know how big they are.

One corner-case that behaves strangely without the patch is:

void f() {
  int x;
  constexpr int y = (&x + 1) - &x;
}

void g() {
  __SVInt8_t x;
  constexpr int y = (&x + 1) - &x;
}

f() seems to be valid code, but g() produces:

warning: subtraction of pointers to type '__SVInt8_t' of zero size has undefined behavior [-Wpointer-arith]

So if we were to allow pointer arithmetic, I think in practice we'd need some rules to make sizeof effectively variable, but limited to that context.

In practice I don't think a more relaxed rule wil be useful because it isn't possible to create arrays of sizeless type. So the simplest approach seems to be to inherit the normal rules for incomplete types.

efriedma accepted this revision.Mar 12 2020, 3:42 PM

LGTM

People are going to want to iterate over arrays in memory to write simple intrinsic loops. If we don't allow p + i to work, people will be forced to write (__SVInt8_t*)((char*)p + i * svcntb()). Which is the same thing, just a lot uglier. And it wouldn't really be that crazy to support sizeof(__SVInt8_t). I mean, it's effectively just a different way of spelling svcntb(). I guess we can always relax it later, though.

This revision is now accepted and ready to land.Mar 12 2020, 3:42 PM

People are going to want to iterate over arrays in memory to write simple intrinsic loops. If we don't allow p + i to work, people will be forced to write (__SVInt8_t*)((char*)p + i * svcntb()). Which is the same thing, just a lot uglier. And it wouldn't really be that crazy to support sizeof(__SVInt8_t). I mean, it's effectively just a different way of spelling svcntb(). I guess we can always relax it later, though.

__SVInt8_t is only really supposed to be used for short-term working data rather than storage. Sizelessness is one thing that makes it awkward to use as the target of a pointer iterator. But another is that the underlying ABI type has an alignment of 16 bytes regardless of how big the vector actually is. Also, for length-agnostic code, it would be unusual to have two __SVInt8_ts side-by-side in memory, since the code doesn't know ahead of time how big each one is.

So for length-agnostic code the expectation is that loops would iterate on int8_t * instead of __SVInt8_t *. It's then possible to operate on pointers that aren't 16-byte aligned. The pointer can also be incremented by svcntb() without casting. The same goes for other combinations, e.g. int32_t * and svcntw().

For length-specific code we're working on another solution that allows SVE vector types to be treated like normal fixed-length types. I hope we'll be able to share that soon.

I don't think there's really any reason to force the user into this sort of thing... but again, we can relax this later, so I'm not that concerned about it at the moment.

This revision was automatically updated to reflect the committed changes.