[llvm][Arm/AArch64] Format extension flags in CPU test failures
authorDavid Spickett <david.spickett@linaro.org>
Thu, 17 Dec 2020 10:52:37 +0000 (10:52 +0000)
committerDavid Spickett <david.spickett@linaro.org>
Tue, 22 Dec 2020 11:13:36 +0000 (11:13 +0000)
Previously you just two hex numbers you had to decode manually.
This change adds a predicate formatter for extension flags
to produce failure messages like:
```
[ RUN      ] AArch64CPUTests/AArch64CPUTestFixture.testAArch64CPU/2
<...>llvm/unittests/Support/TargetParserTest.cpp:862:
Failure
Expected extension flags: +fp-armv8, +crc, +crypto (0xe)
     Got extension flags: +fp-armv8, +neon, +crc, +crypto (0x1e)
[  FAILED  ] AArch64CPUTests/AArch64CPUTestFixture.testAArch64CPU/2,
where GetParam() = "cortex-a34", "armv8-a", <...>
```

From there you can take the feature name and map it back
to the enum in ARM/AArch64TargetParser.def.
(which isn't perfect but you've probably got both files
open if you're editing these tests)

Note that AEK_NONE is not meant to be user facing in the compiler
but here it is part of the tests. So failures may show an
extension "none" where the normal target parser wouldn't.

The formatter is implemented as a template on ARM::ISAKind
because the predicate formatters assume all parameters are used
for comparison.
(e.g. PRED_FORMAT3 is for comparing 3 values, not having 3
arguments in general)

Reviewed By: MarkMurrayARM

Differential Revision: https://reviews.llvm.org/D93448

llvm/unittests/Support/TargetParserTest.cpp

index 5208f6a..bc2fd62 100644 (file)
@@ -8,7 +8,9 @@
 
 #include "llvm/Support/TargetParser.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/ARMBuildAttributes.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "gtest/gtest.h"
 #include <string>
 
@@ -31,6 +33,47 @@ const char *ARMArch[] = {
     "armv8m.main", "iwmmxt",       "iwmmxt2",     "xscale",       "armv8.1-m.main",
 };
 
+template <ARM::ISAKind ISAKind>
+std::string FormatExtensionFlags(uint64_t Flags) {
+  std::vector<StringRef> Features;
+
+  if (ISAKind == ARM::ISAKind::AARCH64) {
+    // AEK_NONE is not meant to be shown to the user so the target parser
+    // does not recognise it. It is relevant here though.
+    if (Flags & AArch64::AEK_NONE)
+      Features.push_back("none");
+    AArch64::getExtensionFeatures(Flags, Features);
+  } else {
+    if (Flags & ARM::AEK_NONE)
+      Features.push_back("none");
+    ARM::getExtensionFeatures(Flags, Features);
+  }
+
+  // The target parser also includes every extension you don't have.
+  // E.g. if AEK_CRC is not set then it adds "-crc". Not useful here.
+  Features.erase(std::remove_if(Features.begin(), Features.end(),
+                                [](StringRef extension) {
+                                  return extension.startswith("-");
+                                }),
+                 Features.end());
+
+  return llvm::join(Features, ", ");
+}
+
+template <ARM::ISAKind ISAKind>
+testing::AssertionResult
+AssertSameExtensionFlags(const char *m_expr, const char *n_expr,
+                         uint64_t ExpectedFlags, uint64_t GotFlags) {
+  if (ExpectedFlags == GotFlags)
+    return testing::AssertionSuccess();
+
+  return testing::AssertionFailure() << llvm::formatv(
+             "Expected extension flags: {0} ({1:x})\n"
+             "     Got extension flags: {2} ({3:x})\n",
+             FormatExtensionFlags<ISAKind>(ExpectedFlags), ExpectedFlags,
+             FormatExtensionFlags<ISAKind>(GotFlags), GotFlags);
+}
+
 struct ARMCPUTestParams {
   ARMCPUTestParams(StringRef CPUName, StringRef ExpectedArch,
                    StringRef ExpectedFPU, uint64_t ExpectedFlags,
@@ -65,7 +108,8 @@ TEST_P(ARMCPUTestFixture, ARMCPUTests) {
   EXPECT_EQ(params.ExpectedFPU, ARM::getFPUName(FPUKind));
 
   uint64_t default_extensions = ARM::getDefaultExtensions(params.CPUName, AK);
-  EXPECT_EQ(params.ExpectedFlags, default_extensions);
+  EXPECT_PRED_FORMAT2(AssertSameExtensionFlags<ARM::ISAKind::ARM>,
+                      params.ExpectedFlags, default_extensions);
 
   EXPECT_EQ(params.CPUAttr, ARM::getCPUAttr(AK));
 }
@@ -816,7 +860,8 @@ TEST_P(AArch64CPUTestFixture, testAArch64CPU) {
 
   uint64_t default_extensions =
       AArch64::getDefaultExtensions(params.CPUName, AK);
-  EXPECT_EQ(params.ExpectedFlags, default_extensions);
+  EXPECT_PRED_FORMAT2(AssertSameExtensionFlags<ARM::ISAKind::AARCH64>,
+                      params.ExpectedFlags, default_extensions);
 
   unsigned FPUKind = AArch64::getDefaultFPU(params.CPUName, AK);
   EXPECT_EQ(params.ExpectedFPU, ARM::getFPUName(FPUKind));