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

[whchung]

Summary

Fix logic error in D79508. The old logic would make the first check in
GPUFuncOp::verifyBody always pass.

[herhut]

mlir/lib/Dialect/GPU/IR/GPUDialect.cpp

696

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.

[whchung]

mlir/lib/Dialect/GPU/IR/GPUDialect.cpp

696

Thanks for the hint. Will revise the patch.

[whchung]

Revise the fix and added positive and negative tests.

[herhut]

mlir/lib/Dialect/GPU/IR/GPUDialect.cpp

681–682

This produces error messages that are not overly helpful. I would verify this is follows. We need at least getNumArguments() + getNumWorkgroupAttributions() many operands, as otherwise there are not enough. The getNumPrivateAttributions() does not need to be verified, as it is computed as the remaining operands that are left over.

So the only error case is when we do not have enough operands to satisfy function arguments and workgroup attributions.

[whchung]

Address code review comments.

[herhut]

Thanks! Please excuse the long delay due to me travelling.

[whchung]

This patch was landed in commit 603b974cf7103766a0e5e4a0320fedb7c4b570f9.