From b6d54a1ba3f73aab629f73be1844674abc604294 Mon Sep 17 00:00:00 2001
From: Jacques Pienaar <jpienaar@google.com>
Date: Thu, 19 Dec 2019 16:43:35 -0800
Subject: [PATCH] Unique trait list during ODS Operator trait construction

Concatting lists in TableGen is easy, creating unique lists less so. There is no reason for duplicated op traits so we could throw an error instead but duplicates could occur due to concatting different list of traits in ODS (e.g., for convenience reasons), so just dedup them during Operator trait construction instead.

PiperOrigin-RevId: 286488423
---
 mlir/include/mlir/IR/OpBase.td   |  1 +
 mlir/lib/TableGen/Operator.cpp   | 19 +++++++++++++------
 mlir/test/mlir-tblgen/op-decl.td |  4 +++-
 3 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/mlir/include/mlir/IR/OpBase.td b/mlir/include/mlir/IR/OpBase.td
index 8f6770f297ed..24e48b329d58 100644
--- a/mlir/include/mlir/IR/OpBase.td
+++ b/mlir/include/mlir/IR/OpBase.td
@@ -1586,6 +1586,7 @@ class Op<Dialect dialect, string mnemonic, list<OpTrait> props = []> {
   bit hasFolder = 0;
 
   // Op traits.
+  // Note: The list of traits will be uniqued by ODS.
   list<OpTrait> traits = props;
 
   // Additional code that will be added to the public part of the generated
diff --git a/mlir/lib/TableGen/Operator.cpp b/mlir/lib/TableGen/Operator.cpp
index 4529208a39c6..3825363bec00 100644
--- a/mlir/lib/TableGen/Operator.cpp
+++ b/mlir/lib/TableGen/Operator.cpp
@@ -23,6 +23,7 @@
 #include "mlir/TableGen/OpTrait.h"
 #include "mlir/TableGen/Predicate.h"
 #include "mlir/TableGen/Type.h"
+#include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/Record.h"
@@ -293,12 +294,18 @@ void tblgen::Operator::populateOpStructure() {
     results.push_back({name, TypeConstraint(resultDef)});
   }
 
-  auto traitListInit = def.getValueAsListInit("traits");
-  if (!traitListInit)
-    return;
-  traits.reserve(traitListInit->size());
-  for (auto traitInit : *traitListInit)
-    traits.push_back(OpTrait::create(traitInit));
+  // Create list of traits, skipping over duplicates: appending to lists in
+  // tablegen is easy, making them unique less so, so dedupe here.
+  if (auto traitList = def.getValueAsListInit("traits")) {
+    // This is uniquing based on pointers of the trait.
+    SmallPtrSet<const llvm::Init *, 32> traitSet;
+    traits.reserve(traitSet.size());
+    for (auto traitInit : *traitList) {
+      // Keep traits in the same order while skipping over duplicates.
+      if (traitSet.insert(traitInit).second)
+        traits.push_back(OpTrait::create(traitInit));
+    }
+  }
 
   // Handle regions
   auto *regionsDag = def.getValueAsDag("regions");
diff --git a/mlir/test/mlir-tblgen/op-decl.td b/mlir/test/mlir-tblgen/op-decl.td
index c0420cb19c1c..a217a139848b 100644
--- a/mlir/test/mlir-tblgen/op-decl.td
+++ b/mlir/test/mlir-tblgen/op-decl.td
@@ -9,7 +9,9 @@ def Test_Dialect : Dialect {
 class NS_Op<string mnemonic, list<OpTrait> traits> :
     Op<Test_Dialect, mnemonic, traits>;
 
-def NS_AOp : NS_Op<"a_op", [NoSideEffect]> {
+// NoSideEffect trait is included twice to ensure it gets uniqued during
+// emission.
+def NS_AOp : NS_Op<"a_op", [NoSideEffect, NoSideEffect]> {
   let arguments = (ins
     I32:$a,
     Variadic<F32>:$b,