[PATCH] D93448: [llvm][Arm/AArch64] Format extension flags in CPU test failures

[David Spickett via Phabricator via llvm-commits]

DavidSpickett created this revision.
Herald added subscribers: danielkiss, kristof.beyls.
DavidSpickett requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

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)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93448

Files:
  llvm/unittests/Support/TargetParserTest.cpp


Index: llvm/unittests/Support/TargetParserTest.cpp
===================================================================
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -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 "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include <string>
@@ -33,6 +35,47 @@
     "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,
@@ -67,7 +110,8 @@
   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));
 }
@@ -813,7 +857,8 @@
 
   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));


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D93448.312421.patch
Type: text/x-patch
Size: 3179 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201217/3abfaeef/attachment.bin>