[llvm-exegesis] Finish plumbing the Config field.

Summary:
Right now there are no snippet generators that emit the `Config` Field,
but I plan to add it to investigate LEA operands for PR32326.

What was broken was:
 - `Config` Was not propagated up until the BenchmarkResult::Key.
 - Clustering should really consider different configs as measuring
 different things, so we should stabilize on (Opcode, Config) instead of
 just Opcode.

Reviewers: gchatelet

Subscribers: tschuett, llvm-commits, lebedev.ri

Tags: #llvm

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

llvm-svn: 374031

test/tools/llvm-exegesis/X86/analysis-cluster-stabilization-config.test

@@ -4,8 +4,7 @@
 # have different configs, so they should not be placed in the same cluster by
 # stabilization.
 
-# CHECK-UNSTABLE: SQRTSSr
+# CHECK-UNSTABLE-NOT: SQRTSSr
-# CHECK-UNSTABLE: SQRTSSr
 
 ---
 mode:            latency

tools/llvm-exegesis/lib/BenchmarkCode.h

@@ -9,7 +9,7 @@
 #ifndef LLVM_TOOLS_LLVM_EXEGESIS_BENCHMARKCODE_H
 #define LLVM_TOOLS_LLVM_EXEGESIS_BENCHMARKCODE_H
 
-#include "RegisterValue.h"
+#include "BenchmarkResult.h"
 #include "llvm/MC/MCInst.h"
 #include <string>
 #include <vector>
@@ -19,12 +19,7 @@ namespace exegesis {
 
 // A collection of instructions that are to be assembled, executed and measured.
 struct BenchmarkCode {
-  // The sequence of instructions that are to be repeated.
+  InstructionBenchmarkKey Key;
-  std::vector<llvm::MCInst> Instructions;
-
-  // Before the code is executed some instructions are added to setup the
-  // registers initial values.
-  std::vector<RegisterValue> RegisterInitialValues;
 
   // We also need to provide the registers that are live on entry for the
   // assembler to generate proper prologue/epilogue.

tools/llvm-exegesis/lib/BenchmarkResult.h

@@ -15,8 +15,8 @@
 #ifndef LLVM_TOOLS_LLVM_EXEGESIS_BENCHMARKRESULT_H
 #define LLVM_TOOLS_LLVM_EXEGESIS_BENCHMARKRESULT_H
 
-#include "BenchmarkCode.h"
 #include "LlvmState.h"
+#include "RegisterValue.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/MC/MCInst.h"

tools/llvm-exegesis/lib/BenchmarkRunner.cpp

@@ -31,7 +31,6 @@ BenchmarkRunner::BenchmarkRunner(const LLVMState &State,
 
 BenchmarkRunner::~BenchmarkRunner() = default;
 
-
 namespace {
 class FunctionExecutorImpl : public BenchmarkRunner::FunctionExecutor {
 public:
@@ -92,10 +91,9 @@ InstructionBenchmark BenchmarkRunner::runConfiguration(
   InstrBenchmark.NumRepetitions = NumRepetitions;
   InstrBenchmark.Info = BC.Info;
 
-  const std::vector<llvm::MCInst> &Instructions = BC.Instructions;
+  const std::vector<llvm::MCInst> &Instructions = BC.Key.Instructions;
 
-  InstrBenchmark.Key.Instructions = Instructions;
+  InstrBenchmark.Key = BC.Key;
-  InstrBenchmark.Key.RegisterInitialValues = BC.RegisterInitialValues;
 
   // Assemble at least kMinInstructionsForSnippet instructions by repeating the
   // snippet for debug/analysis. This is so that the user clearly understands
@@ -104,10 +102,10 @@ InstructionBenchmark BenchmarkRunner::runConfiguration(
   {
     llvm::SmallString<0> Buffer;
     llvm::raw_svector_ostream OS(Buffer);
-    assembleToStream(
+    assembleToStream(State.getExegesisTarget(), State.createTargetMachine(),
-        State.getExegesisTarget(), State.createTargetMachine(), BC.LiveIns,
+                     BC.LiveIns, BC.Key.RegisterInitialValues,
-        BC.RegisterInitialValues,
+                     Repetitor.Repeat(Instructions, kMinInstructionsForSnippet),
-        Repetitor.Repeat(BC.Instructions, kMinInstructionsForSnippet), OS);
+                     OS);
     const ExecutableFunction EF(State.createTargetMachine(),
                                 getObjectFromBuffer(OS.str()));
     const auto FnBytes = EF.getFunctionBytes();
@@ -117,7 +115,7 @@ InstructionBenchmark BenchmarkRunner::runConfiguration(
   // Assemble NumRepetitions instructions repetitions of the snippet for
   // measurements.
   const auto Filler =
-      Repetitor.Repeat(BC.Instructions, InstrBenchmark.NumRepetitions);
+      Repetitor.Repeat(Instructions, InstrBenchmark.NumRepetitions);
 
   llvm::object::OwningBinary<llvm::object::ObjectFile> ObjectFile;
   if (DumpObjectToDisk) {
@@ -133,7 +131,7 @@ InstructionBenchmark BenchmarkRunner::runConfiguration(
     llvm::SmallString<0> Buffer;
     llvm::raw_svector_ostream OS(Buffer);
     assembleToStream(State.getExegesisTarget(), State.createTargetMachine(),
-                     BC.LiveIns, BC.RegisterInitialValues, Filler, OS);
+                     BC.LiveIns, BC.Key.RegisterInitialValues, Filler, OS);
     ObjectFile = getObjectFromBuffer(OS.str());
   }
 
@@ -150,7 +148,7 @@ InstructionBenchmark BenchmarkRunner::runConfiguration(
     // Scale the measurements by instruction.
     BM.PerInstructionValue /= InstrBenchmark.NumRepetitions;
     // Scale the measurements by snippet.
-    BM.PerSnippetValue *= static_cast<double>(BC.Instructions.size()) /
+    BM.PerSnippetValue *= static_cast<double>(Instructions.size()) /
                           InstrBenchmark.NumRepetitions;
   }
 
@@ -167,7 +165,7 @@ BenchmarkRunner::writeObjectFile(const BenchmarkCode &BC,
     return std::move(E);
   llvm::raw_fd_ostream OFS(ResultFD, true /*ShouldClose*/);
   assembleToStream(State.getExegesisTarget(), State.createTargetMachine(),
-                   BC.LiveIns, BC.RegisterInitialValues, FillFunction, OFS);
+                   BC.LiveIns, BC.Key.RegisterInitialValues, FillFunction, OFS);
   return ResultPath.str();
 }
 

tools/llvm-exegesis/lib/Clustering.cpp

@@ -237,39 +237,40 @@ void InstructionBenchmarkClustering::clusterizeNaive(unsigned NumOpcodes) {
 // We shall find every opcode with benchmarks not in just one cluster, and move
 // *all* the benchmarks of said Opcode into one new unstable cluster per Opcode.
 void InstructionBenchmarkClustering::stabilize(unsigned NumOpcodes) {
-  // Given an instruction Opcode, in which clusters do benchmarks of this
+  // Given an instruction Opcode and Config, in which clusters do benchmarks of
-  // instruction lie? Normally, they all should be in the same cluster.
+  // this instruction lie? Normally, they all should be in the same cluster.
-  std::vector<llvm::SmallSet<ClusterId, 1>> OpcodeToClusterIDs;
+  struct OpcodeAndConfig {
-  OpcodeToClusterIDs.resize(NumOpcodes);
+    explicit OpcodeAndConfig(const InstructionBenchmark &IB)
-  // The list of opcodes that have more than one cluster.
+        : Opcode(IB.keyInstruction().getOpcode()), Config(&IB.Key.Config) {}
-  llvm::SetVector<size_t> UnstableOpcodes;
+    unsigned Opcode;
-  // Populate OpcodeToClusterIDs and UnstableOpcodes data structures.
+    const std::string *Config;
+
+    auto Tie() const -> auto { return std::tie(Opcode, *Config); }
+
+    bool operator<(const OpcodeAndConfig &O) const { return Tie() < O.Tie(); }
+    bool operator!=(const OpcodeAndConfig &O) const { return Tie() != O.Tie(); }
+  };
+  std::map<OpcodeAndConfig, llvm::SmallSet<ClusterId, 1>>
+      OpcodeConfigToClusterIDs;
+  // Populate OpcodeConfigToClusterIDs and UnstableOpcodes data structures.
   assert(ClusterIdForPoint_.size() == Points_.size() && "size mismatch");
   for (const auto &Point : zip(Points_, ClusterIdForPoint_)) {
     const ClusterId &ClusterIdOfPoint = std::get<1>(Point);
     if (!ClusterIdOfPoint.isValid())
       continue; // Only process fully valid clusters.
-    const unsigned Opcode = std::get<0>(Point).keyInstruction().getOpcode();
+    const OpcodeAndConfig Key(std::get<0>(Point));
-    assert(Opcode < NumOpcodes && "NumOpcodes is incorrect (too small)");
     llvm::SmallSet<ClusterId, 1> &ClusterIDsOfOpcode =
-        OpcodeToClusterIDs[Opcode];
+        OpcodeConfigToClusterIDs[Key];
     ClusterIDsOfOpcode.insert(ClusterIdOfPoint);
-    // Is there more than one ClusterID for this opcode?.
-    if (ClusterIDsOfOpcode.size() < 2)
-      continue; // If not, then at this moment this Opcode is stable.
-    // Else let's record this unstable opcode for future use.
-    UnstableOpcodes.insert(Opcode);
   }
-  assert(OpcodeToClusterIDs.size() == NumOpcodes && "sanity check");
 
-  // We know with how many [new] clusters we will end up with.
+  for (const auto &OpcodeConfigToClusterID : OpcodeConfigToClusterIDs) {
-  const auto NewTotalClusterCount = Clusters_.size() + UnstableOpcodes.size();
-  Clusters_.reserve(NewTotalClusterCount);
-  for (const size_t UnstableOpcode : UnstableOpcodes.getArrayRef()) {
     const llvm::SmallSet<ClusterId, 1> &ClusterIDs =
-        OpcodeToClusterIDs[UnstableOpcode];
+        OpcodeConfigToClusterID.second;
-    assert(ClusterIDs.size() > 1 &&
+    const OpcodeAndConfig &Key = OpcodeConfigToClusterID.first;
-           "Should only have Opcodes with more than one cluster.");
+    // We only care about unstable instructions.
+    if (ClusterIDs.size() < 2)
+      continue;
 
     // Create a new unstable cluster, one per Opcode.
     Clusters_.emplace_back(ClusterId::makeValidUnstable(Clusters_.size()));
@@ -290,8 +291,8 @@ void InstructionBenchmarkClustering::stabilize(unsigned NumOpcodes) {
       // and the rest of the points is for the UnstableOpcode.
       const auto it = std::stable_partition(
           OldCluster.PointIndices.begin(), OldCluster.PointIndices.end(),
-          [this, UnstableOpcode](size_t P) {
+          [this, &Key](size_t P) {
-            return Points_[P].keyInstruction().getOpcode() != UnstableOpcode;
+            return OpcodeAndConfig(Points_[P]) != Key;
           });
       assert(std::distance(it, OldCluster.PointIndices.end()) > 0 &&
              "Should have found at least one bad point");
@@ -314,7 +315,6 @@ void InstructionBenchmarkClustering::stabilize(unsigned NumOpcodes) {
            "New unstable cluster should end up with no less points than there "
            "was clusters");
   }
-  assert(Clusters_.size() == NewTotalClusterCount && "sanity check");
 }
 
 llvm::Expected<InstructionBenchmarkClustering>

tools/llvm-exegesis/lib/CodeTemplate.h

@@ -115,6 +115,8 @@ struct CodeTemplate {
   CodeTemplate &operator=(const CodeTemplate &) = delete;
 
   ExecutionMode Execution = ExecutionMode::UNKNOWN;
+  // See InstructionBenchmarkKey.::Config.
+  std::string Config;
   // Some information about how this template has been created.
   std::string Info;
   // The list of the instructions for this template.

tools/llvm-exegesis/lib/SnippetFile.cpp

@@ -36,7 +36,7 @@ public:
   // instructions.
   void EmitInstruction(const MCInst &Instruction,
                        const MCSubtargetInfo &STI) override {
-    Result->Instructions.push_back(Instruction);
+    Result->Key.Instructions.push_back(Instruction);
   }
 
   // Implementation of the AsmCommentConsumer.
@@ -65,7 +65,7 @@ public:
       const StringRef HexValue = Parts[1].trim();
       RegVal.Value = APInt(
           /* each hex digit is 4 bits */ HexValue.size() * 4, HexValue, 16);
-      Result->RegisterInitialValues.push_back(std::move(RegVal));
+      Result->Key.RegisterInitialValues.push_back(std::move(RegVal));
       return;
     }
     if (CommentText.consume_front("LIVEIN")) {

tools/llvm-exegesis/lib/SnippetGenerator.cpp

@@ -73,12 +73,13 @@ SnippetGenerator::generateConfigurations(
         BC.Info = CT.Info;
         for (InstructionTemplate &IT : CT.Instructions) {
           randomizeUnsetVariables(State.getExegesisTarget(), ForbiddenRegs, IT);
-          BC.Instructions.push_back(IT.build());
+          BC.Key.Instructions.push_back(IT.build());
         }
         if (CT.ScratchSpacePointerInReg)
           BC.LiveIns.push_back(CT.ScratchSpacePointerInReg);
-        BC.RegisterInitialValues =
+        BC.Key.RegisterInitialValues =
             computeRegisterInitialValues(CT.Instructions);
+        BC.Key.Config = CT.Config;
         Output.push_back(std::move(BC));
       }
     }

unittests/tools/llvm-exegesis/X86/SnippetFileTest.cpp

@@ -78,8 +78,8 @@ TEST_F(X86SnippetFileTest, Works) {
   EXPECT_FALSE((bool)Snippets.takeError());
   ASSERT_THAT(*Snippets, SizeIs(1));
   const auto &Snippet = (*Snippets)[0];
-  ASSERT_THAT(Snippet.Instructions, ElementsAre(HasOpcode(X86::INC64r)));
+  ASSERT_THAT(Snippet.Key.Instructions, ElementsAre(HasOpcode(X86::INC64r)));
-  ASSERT_THAT(Snippet.RegisterInitialValues,
+  ASSERT_THAT(Snippet.Key.RegisterInitialValues,
               ElementsAre(RegisterInitialValueIs(X86::RAX, 15),
                           RegisterInitialValueIs(X86::SIL, 0)));
   ASSERT_THAT(Snippet.LiveIns, ElementsAre(X86::RDI, X86::DL));