Change behavior when converting to tpp #128
Conversation
- Generate parallel loops as default. - Check static shape in `checkOperandForTpp` - Drop duplicate code - Revert to original behavior where we have two separate passes: 1 for detection and 1 for mapping (`populateMapLinalgToTppPatterns` is not removed from `populateConvertLinalgToTppPatterns`.
|
|
||
| // ----- | ||
|
|
||
| #map5 = affine_map<(d0, d1, d2) -> (d0, d1, d2)> |
There was a problem hiding this comment.
Why was this test dropped? Do we not recognise this any more as tpp.relu?
There was a problem hiding this comment.
Actually, this looks like a repeat of the test above, is that the reason?
There was a problem hiding this comment.
No simply because we don't run the detection in the conversion pass anymore. We revert to original behavior where we first need to run -map-linalg-to-tpp and then convert-linalg-to-tpp. It is better to keep these as separate passes.
There was a problem hiding this comment.
Is there a similar test elsewhere, then?
There was a problem hiding this comment.
Yes, the one above. Was the same without the tpp.relu annotation.
There was a problem hiding this comment.
Sorry, no. (0, x) can be normalised to (x, 0) and we're good.
This test seems to look at all "recogniseable" linalg ops that have the TPP shape (or annotation). IIUC, that's done by the mapping pass.
Do we have tests for the mapping pass, to make sure that test that you removed will continue to be recognised in the future?
Here, we only have hand-coded IR that already has the annotation, I'm wondering if we have tests that make sure we keep adding annotation to the right constructs.
There was a problem hiding this comment.
And the mapping is quite not right at the moment as we only check to have a maxf
So, if we have a maxf(x, 2) it will map to RELU?
There was a problem hiding this comment.
We don't have much: https://github.com/plaidml/tpp-sandbox/blob/main/test/TPP/simple-match.mlir and yes at the moment maxf(x, 2) will get annotated with a tpp.relu
There was a problem hiding this comment.
We also need to make the relu single operand see: #129
There was a problem hiding this comment.
We don't have much: https://github.com/plaidml/tpp-sandbox/blob/main/test/TPP/simple-match.mlir
That should be fine for the time being.
and yes at the moment
maxf(x, 2)will get annotated with a tpp.relu
checkOperandForTpppopulateMapLinalgToTppPatternsis not removed frompopulateConvertLinalgToTppPatterns.