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

[Wen-Heng (Jack) Chung via Phabricator via llvm-commits]

whchung added inline comments.


================
Comment at: mlir/lib/Dialect/GPU/IR/GPUDialect.cpp:696
 
+  if (getWorkgroupAttributions().size() != getNumWorkgroupAttributions())
+    return emitOpError() << "number of workgroup attributions do not match";
----------------
herhut wrote:
> 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.
Thanks for the hint. Will revise the patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80766