[utils/TableGen/X86CompressEVEXTablesEmitter.cpp] Make sure the tablegen output for the checkPredicate function is deterministic

[akyrtzi]

The output for the checkPredicate function was depending on a std::map iteration that was non-deterministic from run to run, because the keys were pointer values.

Make a change so that the keys are StringRefs so the ordering is stable.

[llvmbot]

@llvm/pr-subscribers-backend-x86

Author: Argyrios Kyrtzidis (akyrtzi)

Changes

The output for the checkPredicate function was depending on a std::map iteration that was not deterministic from run to run, so the contents of X86GenCompressEVEXTables.inc were not deterministic.

Make a change to sort the entries before iterating over them.

---

Full diff: https://github.com/llvm/llvm-project/pull/84533.diff

1 Files Affected:

• (modified) llvm/utils/TableGen/X86CompressEVEXTablesEmitter.cpp (+15-3)

diff --git a/llvm/utils/TableGen/X86CompressEVEXTablesEmitter.cpp b/llvm/utils/TableGen/X86CompressEVEXTablesEmitter.cpp
index b96d16b9797cf3..326a9dfbcfefaa 100644
--- a/llvm/utils/TableGen/X86CompressEVEXTablesEmitter.cpp
+++ b/llvm/utils/TableGen/X86CompressEVEXTablesEmitter.cpp
@@ -82,15 +82,27 @@ void X86CompressEVEXTablesEmitter::printTable(const std::vector<Entry> &Table,
 
 void X86CompressEVEXTablesEmitter::printCheckPredicate(
     const PredicateInstMap &PredicateInsts, raw_ostream &OS) {
+  // Create a sorted list of the map entries so that the tablegen output is
+  // deterministic.
+  typedef std::pair<StringRef, const std::vector<const CodeGenInstruction *> *>
+      PredicateInstsTy;
+  std::vector<PredicateInstsTy> SortedPredicateInsts;
+  for (const auto &[Key, Val] : PredicateInsts) {
+    SortedPredicateInsts.push_back({Key->getValueAsString("CondString"), &Val});
+  }
+  llvm::sort(SortedPredicateInsts,
+             [](const PredicateInstsTy &LHS, const PredicateInstsTy &RHS) {
+               return LHS.first < RHS.first;
+             });
 
   OS << "static bool checkPredicate(unsigned Opc, const X86Subtarget "
         "*Subtarget) {\n"
      << "  switch (Opc) {\n"
      << "  default: return true;\n";
-  for (const auto &[Key, Val] : PredicateInsts) {
-    for (const auto &Inst : Val)
+  for (const auto &[Key, Val] : SortedPredicateInsts) {
+    for (const auto &Inst : *Val)
       OS << "  case X86::" << Inst->TheDef->getName() << ":\n";
-    OS << "    return " << Key->getValueAsString("CondString") << ";\n";
+    OS << "    return " << Key << ";\n";
   }
 
   OS << "  }\n";

[KanRobert]

Not understand. I believe std::map is a red-black tree and the iterations on it is deterministic.

[akyrtzi]

It's non-deterministic because the keys are pointer values, so ordering for them is not stable between runs.

[KanRobert]

It's non-deterministic because the keys are pointer values, so ordering for them is not stable between runs.

I see. Could we change the type of PredicateInstMap to

std::map<StringRef, std::vector<const CodeGenInstruction *>>
      PredicateInstMap;

where the StringRef is the CondString of the predicate. So that we don't have to resort it after the map is constructed.

[akyrtzi]

Could we change the type of PredicateInstMap

I like this idea, see updated commit.

[KanRobert]

LGTM, thanks!

[KanRobert]

Friendly remind: Need to update the description.

[akyrtzi]

Updated 👍