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

[Richard Barton via llvm-commits]

richard.barton.arm added a comment.

Hi all - I was puzzling over how to write a new unit test that would catch this error without just copying the structure of the target parser itself. The best I could come up with is to define a for each architecture extension, a set of all architectures that must enable it by default. You could then iterate over all the extensions and for each check all the architectures and test that those in the set have the enum set and those not in the set do not.

So like:

  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.


https://reviews.llvm.org/D23500