[llvm] Fix non-determinism in debuginfo (PR #68332)

[Paul Kirth via llvm-commits]

https://github.com/ilovepi updated https://github.com/llvm/llvm-project/pull/68332

>From bad7cc5eaa1ca0dfd04f92ecb053ff6ed93daba7 Mon Sep 17 00:00:00 2001
From: Paul Kirth <paulkirth at google.com>
Date: Wed, 4 Oct 2023 16:21:19 -0700
Subject: [PATCH 1/2] Fix non-determinism in debuginfo

Assignment tracking iterates over a SmallSet when adding metadata,
which eventually results in debug metadata being added to the module in
non-deterministic order.

As reported in #63921, we saw some cases where DWARF DebugLoc entries
could have their order reversed, even though there was no functional
difference.

This patch replaces the SmallSet with a SmallVector, and adds the
required DenseMapInfo specialization.

Fixes: #63921
---
 llvm/include/llvm/IR/DebugInfo.h | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/llvm/include/llvm/IR/DebugInfo.h b/llvm/include/llvm/IR/DebugInfo.h
index 26a7cfbbb350234..c5ded71406d885e 100644
--- a/llvm/include/llvm/IR/DebugInfo.h
+++ b/llvm/include/llvm/IR/DebugInfo.h
@@ -16,7 +16,9 @@
 #ifndef LLVM_IR_DEBUGINFO_H
 #define LLVM_IR_DEBUGINFO_H
 
+#include "llvm/ADT/DenseMapInfo.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/SmallVector.h"
@@ -263,7 +265,8 @@ struct VarRecord {
 /// TODO: Backing storage shouldn't be limited to allocas only. Some local
 /// variables have their storage allocated by the calling function (addresses
 /// passed in with sret & byval parameters).
-using StorageToVarsMap = DenseMap<const AllocaInst *, SmallSet<VarRecord, 2>>;
+using StorageToVarsMap =
+    DenseMap<const AllocaInst *, SmallSetVector<VarRecord, 2>>;
 
 /// Track assignments to \p Vars between \p Start and \p End.
 
@@ -314,6 +317,25 @@ class AssignmentTrackingPass : public PassInfoMixin<AssignmentTrackingPass> {
 
 /// Return true if assignment tracking is enabled for module \p M.
 bool isAssignmentTrackingEnabled(const Module &M);
+
+template <> struct DenseMapInfo<at::VarRecord> {
+  static inline at::VarRecord getEmptyKey() {
+    return at::VarRecord{nullptr, nullptr};
+  }
+
+  static inline at::VarRecord getTombstoneKey() {
+    return at::VarRecord{nullptr, nullptr};
+  }
+
+  static unsigned getHashValue(const at::VarRecord &Var) {
+    return hash_combine(Var.Var, Var.DL);
+  }
+
+  static bool isEqual(const at::VarRecord &A, const at::VarRecord &B) {
+    return A == B;
+  }
+};
+
 } // end namespace llvm
 
 #endif // LLVM_IR_DEBUGINFO_H

>From 81ad540477ef1b27b63fe5448400ea1b49c39a4e Mon Sep 17 00:00:00 2001
From: Paul Kirth <paulkirth at google.com>
Date: Thu, 5 Oct 2023 14:05:09 -0700
Subject: [PATCH 2/2] [llvm][debuginfo] Use DenseMapInfo keys from component
 types

The naive implementation would break DenseMap, and yield wrong results.
Instead use the Tobstone and Empty Keys from the field types of
VarRecord.
---
 llvm/include/llvm/IR/DebugInfo.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/llvm/include/llvm/IR/DebugInfo.h b/llvm/include/llvm/IR/DebugInfo.h
index c5ded71406d885e..42bfba1c085b747 100644
--- a/llvm/include/llvm/IR/DebugInfo.h
+++ b/llvm/include/llvm/IR/DebugInfo.h
@@ -320,11 +320,13 @@ bool isAssignmentTrackingEnabled(const Module &M);
 
 template <> struct DenseMapInfo<at::VarRecord> {
   static inline at::VarRecord getEmptyKey() {
-    return at::VarRecord{nullptr, nullptr};
+    return at::VarRecord{DenseMapInfo<DILocalVariable *>::getEmptyKey(),
+                         DenseMapInfo<DILocation *>::getEmptyKey()};
   }
 
   static inline at::VarRecord getTombstoneKey() {
-    return at::VarRecord{nullptr, nullptr};
+    return at::VarRecord{DenseMapInfo<DILocalVariable *>::getTombstoneKey(),
+                         DenseMapInfo<DILocation *>::getTombstoneKey()};
   }
 
   static unsigned getHashValue(const at::VarRecord &Var) {