[PATCH] D80766: [mlir][gpu] Fix logic error in D79508 computing number of private attributions.

[Stephan Herhut via Phabricator via llvm-commits]

herhut requested changes to this revision.
herhut added inline comments.
This revision now requires changes to proceed.


================
Comment at: mlir/lib/Dialect/GPU/IR/GPUDialect.cpp:696
 
+  if (getWorkgroupAttributions().size() != getNumWorkgroupAttributions())
+    return emitOpError() << "number of workgroup attributions do not match";
----------------
I don't think this should be in a verifier. `getWorkgroupAttributions()` is implemented by means of `getNumWorkgroupAttributions`, so this is not an invariant we need to check for each operation.

If we want to prevent this from regressing, we would need to write a unit test that shows the wrong behavior. You could specify the illegal case by using the generic syntax, which does not go through the parser but has to explicitly specify attributes. That should allow you to construct also illegal cases.

You can use `mlir-opt -mlir-print-op-generic` to look at the generic syntax.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80766/new/

https://reviews.llvm.org/D80766