[Mlir-commits] [mlir] [mlir][gpu] Guard negative workgroup_attributions in GPU ops (PR #174409)

[Fabian Mora]

================
@@ -886,12 +886,20 @@ LogicalResult LaunchOp::verify() {
 }
 
 LogicalResult LaunchOp::verifyRegions() {
+  int64_t numWorkgroupAttributions = 0;
+  if (auto attr = (*this)->getAttrOfType<IntegerAttr>(
+          getNumWorkgroupAttributionsAttrName())) {
+    numWorkgroupAttributions = attr.getInt();
+    if (numWorkgroupAttributions < 0)
+      return emitOpError() << "expected non-negative workgroup_attributions";
+  }
+
   // Kernel launch takes kNumConfigOperands leading operands for grid/block
   // sizes and transforms them into kNumConfigRegionAttributes region arguments
   // for block/thread identifiers and grid/block sizes.
   if (!getBody().empty()) {
     if (getBody().getNumArguments() <
-        kNumConfigRegionAttributes + getNumWorkgroupAttributions())
+        kNumConfigRegionAttributes + numWorkgroupAttributions)
----------------
fabianmcg wrote:

The thing here is that the num private and workgroup attributions are not really user attributes, but part of the internal state of the op. They shouldn't be set by the user, the only way to set them is by either parsing an op in generic form, or by using `mlir::Operaiton` APIs and setting the attribute directly.
As such, we shouldn't waste verification logic on something that it's clearly an user going out of their way to create an invalid op state. Thus, this should be an assertion.

Seeing the actual code of  `getNumWorkgroupAttributions`, the solution is to have an assert inside the function `getNumWorkgroupAttributions` checking the `i64` attribute is non-negative and less than `numeric_limits<uint32_t>::max`.

https://github.com/llvm/llvm-project/pull/174409