[TableGen] Warn on redundant intrinsic properties

[jayfoad]

Warn on explicit intrinsic properties that are already implied by the
use of DefaultAttrsIntrinsic.

[llvmbot]

@llvm/pr-subscribers-tablegen

Author: Jay Foad (jayfoad)

Changes

Warn on explicit intrinsic properties that are already implied by the
use of DefaultAttrsIntrinsic.

---

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

3 Files Affected:

• (added) llvm/test/TableGen/warn-default-property.td (+7)
• (modified) llvm/utils/TableGen/Basic/CodeGenIntrinsics.cpp (+12-13)
• (modified) llvm/utils/TableGen/Basic/CodeGenIntrinsics.h (-4)

diff --git a/llvm/test/TableGen/warn-default-property.td b/llvm/test/TableGen/warn-default-property.td
new file mode 100644
index 0000000000000..eba75e4b5728e
--- /dev/null
+++ b/llvm/test/TableGen/warn-default-property.td
@@ -0,0 +1,7 @@
+// RUN: llvm-tblgen -gen-intrinsic-impl -I %p/../../include %s -DTEST_INTRINSICS_SUPPRESS_DEFS -o /dev/null 2>&1 | FileCheck %s
+
+include "llvm/IR/Intrinsics.td"
+
+// CHECK: warning: property 'IntrWillReturn' is already enabled by default
+// CHECK: warning: property 'IntrNoCallback' is already enabled by default
+def int_foo : DefaultAttrsIntrinsic<[], [], [IntrWillReturn, IntrNoMem, IntrNoCallback]>;
diff --git a/llvm/utils/TableGen/Basic/CodeGenIntrinsics.cpp b/llvm/utils/TableGen/Basic/CodeGenIntrinsics.cpp
index bc42efa3b2e9c..7f3429c569e26 100644
--- a/llvm/utils/TableGen/Basic/CodeGenIntrinsics.cpp
+++ b/llvm/utils/TableGen/Basic/CodeGenIntrinsics.cpp
@@ -324,6 +324,13 @@ CodeGenIntrinsic::CodeGenIntrinsic(const Record *R,
   for (unsigned E = TypeList->size(); Idx < E; ++Idx)
     IS.ParamTys.push_back(TypeList->getElementAsRecord(Idx));
 
+  // Apply default properties, unless they are disabled.
+  ArrayRef<const Record *> DefaultProperties(Ctx.DefaultProperties);
+  if (TheDef->getValueAsBit("DisableDefaultAttributes"))
+    DefaultProperties = {};
+  for (const Record *Property : DefaultProperties)
+    setProperty(Property);
+
   // Parse the intrinsic properties.
   const ListInit *PropList = R->getValueAsListInit("IntrProperties");
   for (unsigned i = 0, e = PropList->size(); i != e; ++i) {
@@ -331,12 +338,14 @@ CodeGenIntrinsic::CodeGenIntrinsic(const Record *R,
     assert(Property->isSubClassOf("IntrinsicProperty") &&
            "Expected a property!");
 
+    if (is_contained(DefaultProperties, Property)) {
+      PrintWarning(TheDef->getLoc(), "property '" + Property->getName() +
+                                         "' is already enabled by default");
+    }
+
     setProperty(Property);
   }
 
-  // Set default properties to true.
-  setDefaultProperties(Ctx.DefaultProperties);
-
   // Also record the SDPatternOperator Properties.
   Properties = parseSDPatternOperatorProperties(R);
 
@@ -345,16 +354,6 @@ CodeGenIntrinsic::CodeGenIntrinsic(const Record *R,
     llvm::sort(Attrs);
 }
 
-void CodeGenIntrinsic::setDefaultProperties(
-    ArrayRef<const Record *> DefaultProperties) {
-  // opt-out of using default attributes.
-  if (TheDef->getValueAsBit("DisableDefaultAttributes"))
-    return;
-
-  for (const Record *Rec : DefaultProperties)
-    setProperty(Rec);
-}
-
 void CodeGenIntrinsic::setProperty(const Record *R) {
   if (R->getName() == "IntrNoMem")
     ME = MemoryEffects::none();
diff --git a/llvm/utils/TableGen/Basic/CodeGenIntrinsics.h b/llvm/utils/TableGen/Basic/CodeGenIntrinsics.h
index 676f575b2749d..1cd687756cccd 100644
--- a/llvm/utils/TableGen/Basic/CodeGenIntrinsics.h
+++ b/llvm/utils/TableGen/Basic/CodeGenIntrinsics.h
@@ -154,10 +154,6 @@ struct CodeGenIntrinsic {
 
   bool hasProperty(enum SDNP Prop) const { return Properties & (1 << Prop); }
 
-  /// Goes through all IntrProperties that have IsDefault value set and sets
-  /// the property.
-  void setDefaultProperties(ArrayRef<const Record *> DefaultProperties);
-
   /// Helper function to set property \p Name to true.
   void setProperty(const Record *R);
 

[jayfoad]

I'm not sure how useful this is generally, but it's (a cleaned up version of) what I used to prepare #140923.