[PATCH] D23500: [ARM] Correct ARMv8*-A optional extension definitions in TargetParser

[Renato Golin via llvm-commits]

rengolin added a comment.

In https://reviews.llvm.org/D23500#519404, @richard.barton.arm wrote:

> const char *RASDefaultArches[] = { "armv8.2-a"};
>  ..
>  const struct {AArch64::ArchExtKind Ext, const char *DefaultArches[] } Defaults[] =  {
>
>   {AArch64::AEK_RAS, RASDefaultArches },
>   ...
>
> };
>  for D in Defaults:
>
>   for Ext in D.first():
>     for Arch in AllArches:
>       ASSERT(Arch in D.second() ? Arch.ArchBaseExtensions & Ext : !(Arch.ArchBaseExtensions & Ext))
>
>   Does seem a little contrived though. If you all are happy with the existing testing then I will land the patch as-is. Happy to work on further tests if they are needed. When the ACLE catches up then we can test these properly I think.


This would be the pattern that I was expecting, yes, but for all arches / extensions.

My problem with that technique is that new arches / extensions won't fail the tests, so may get away with not adding tests, but I don't have a good strategy to make it break if a new extension is added... Maybe this could be two tests, or a complicated one...

I'm open to suggestions. I also don't mind this commit going as is and you adding some more generic tests later.

But I think these tests will need reviewing one way or another.


https://reviews.llvm.org/D23500