Thanks for the fix. I agree this is the right behaviour FWIW. I held off approving it in case there's another idiom that's preferred when comparing canonical types.

In D77056#2487754, @rsmith wrote:

In D77056#2465936, @rsandifo-arm wrote:

Either way, I realise this isn't great style. It just seems like a practical compromise between the rock of classes having a constant size and the hard place of vectors having a variable length.

I don't think this is just a question of style; it's a severe limitation to language functionality. In a very broad sense, you would not be able to use these types with templates. For example, even if you define an operator< between svint8_t, you can't use std::min between two svint8_t's. And people are going to try to work around that by defining the operators first, then #includeing the algorithms in question, which will lead to ODR issues, will break when C++ modules is enabled, and so on.

Ping. Also, please let me know if the response above doesn't answer the concerns and if you'd like more info/justification.

Thanks for the reviews.

Ping.

Sorry for reviving this after so long, but I'd like to bump it from an RFC to an RFA. As the new covering message says, the spec is now part of the ACLE specification.

Rebase onto current trunk, and also on top of:
https://reviews.llvm.org/D92222

Was superceded by later revisions, but I forgot to close this one.

Was superceded by later revisions, but I forgot to close this one.

The tests look good to me FWIW. The Sema side is already covered by Sema/attr-arm-sve-vector-bits.c and the patch seems to test for the important ABI bits.

In D86065#2241146, @david-arm wrote:

Hi @ctetreau, ok for now I'm going to completely remove the operators and revert the code using those operators to how it was before. I'm not sure what you mean about the predicate functions so I've left those for now, since they aren't needed for this patch. The purpose of this patch was originally supposed to be mechanical anyway - just making members private. I only added the operators as an after-thought really, just to be consistent with how TypeSize dealt with the identical problem. For what it's worth, I believe that GCC solved this exact same problem by adding two types of comparison functions - one set that absolutely wanted an answer to ">,<,>=,<=" and asserted if it wasn't known at compile time, and another set of comparison functions that returned an additional boolean value indicating whether the answer was known or not. Perhaps my knowledge is out of date, but I believe this was the accepted solution and seemed to work well.

LGTM from a spec point of view, but I don't think I should be the one to approve.

I'm not qualified to review the CodeGen stuff (or accept the patch, obvs. :-)) but FWIW, here are some comments on the doc and Sema side.

LGTM, but I don't think I should be the one to accept.

Thanks the update, the idea looks good in general. Two questions though:

• Is it worth having two functions? I thought it might be simpler to have just getCustomEHPadPreservedMask(), with nullptr meaning “no clobbers”.
• It looks from the tests like the function isn't saving or restoring z8-z23 and p4-p15, is that right? That's fine for landing pads that can't return directly to the callee (e.g. because they rethrow). But in these tests the landing pad does return to the caller. Since the extra callee-saved registers are clobbered on the EH edge, we need to save and restore those registers ourselves on control paths that include an EH edge and a return.

In D84044#2176118, @sdesmalen wrote:

Yes, great spot! The caller should assume that all SVE registers are clobbered by the unwinder if the function _may_ throw an exception. I think I'll create a separate patch to fix this, as it's not necessarily something to do with how LLVM emits the unwind info, but rather a bug where the caller doesn't honour the right CC.

Thanks for doing this. FWIW, the patch LGTM from a spec point of view. I just saw one minor spelling nit.

Put two other related warnings under the new option

In D79244#2019027, @efriedma wrote:

Should we stick ext_excess_initializers_in_char_array_initializer and ext_initializer_string_for_char_array_too_long in the same warning group?

OK, thanks! Reclaiming and reopening the revision in that case.

OK, I'll leave the file as-is in that case.

I'd like to promote this from an RFC to an RFA. The associated cfe-dev discussion was:

In D76689#1939834, @efriedma wrote:

Semantically, this makes sense.

I'm not really happy with the use of the term "scalar initializer" to refer to initializing a vector. Seems likely to confuse users.

Don't refer to the sizeless types as "scalars"

Require operators to be defined as static or inside a namespace

In D77056#1950358, @efriedma wrote:

I'm concerned we're going to run into trouble if two people define different SVE "libraries", and you try to link both of them into the same program. If you're not careful, you end up with ODR violations. The scope of this is sort of restricted in normal C++: class and enumerated types sort of have an informal "owner", and people tend to respect that. I mean, you could say it's the user's own fault if they break ODR, but we're essentially making a trap here. Maybe it would make sense to require that SVE operators are defined in a namespace? That would make the user think about the issue.

In D76694#1939863, @efriedma wrote:

Is it not legal to cast an SVE type to any type other than itself?

In D76693#1939866, @efriedma wrote:

Do we want to allow stuff like x ? (svint8_t)0 : (signed char)0

In D76084#1920103, @efriedma wrote:

I'm not sure we actually want to reject this; let's discuss on the review for D76086, since subscripting is sort of a special case of arithmetic.

Allow pointers to sizeless types to be caught.

In D76086#1920558, @efriedma wrote:

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.